This is an archive of the discontinued LLVM Phabricator instance.

[HIP] Add hip offload kind
ClosedPublic

Authored by yaxunl on May 4 2018, 2:06 PM.

Details

Summary

There are quite differences in HIP action builder and action job creation,
which justifies to define a separate offload kind.

Diff Detail

Repository
rL LLVM

Event Timeline

yaxunl created this revision.May 4 2018, 2:06 PM

Otherwise LGTM.

lib/Driver/Compilation.cpp
201 ↗(On Diff #145290)

Mentioning only CUDA in the second clause makes me wonder whether it's *only* okay to abort a CUDA pipeline, not a HIP one. That is presumably not your intent. You could just drop "CUDA" there.

yaxunl marked an inline comment as done.May 7 2018, 8:34 AM
yaxunl added inline comments.
lib/Driver/Compilation.cpp
201 ↗(On Diff #145290)

The second sentence is OK for both CUDA and HIP. Will drop CUDA in the second sentence.

yaxunl updated this revision to Diff 145472.May 7 2018, 8:43 AM
yaxunl marked 2 inline comments as done.

Updated comments.

rjmccall accepted this revision.May 7 2018, 10:38 AM

Thanks, LGTM.

This revision is now accepted and ready to land.May 7 2018, 10:38 AM
tra accepted this revision.May 8 2018, 11:59 AM

Small nit. LGTM otherwise.

lib/Driver/ToolChains/Clang.cpp
133–135 ↗(On Diff #145472)

CUDA and HIP are mutually exclusive, so this should probably be else if

yaxunl marked an inline comment as done.May 8 2018, 1:25 PM
yaxunl added inline comments.
lib/Driver/ToolChains/Clang.cpp
133–135 ↗(On Diff #145472)

Will do when committing.

This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.