This is an archive of the discontinued LLVM Phabricator instance.

[HIP] Add action builder for HIP
ClosedPublic

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

Details

Summary

To support separate compile/link and linking across device IR in different source files,
a new HIP action builder is introduced. Basically it compiles/links host and device
code separately, and embed fat binary in host linking stage through linker script.

Diff Detail

Repository
rC Clang

Event Timeline

yaxunl created this revision.May 4 2018, 2:17 PM
yaxunl retitled this revision from [HIP] Add action builder to [HIP] Add action builder for HIP.May 15 2018, 9:59 AM
yaxunl added reviewers: hfinkel, Hahnfeld.
yaxunl updated this revision to Diff 147156.May 16 2018, 12:37 PM

Skip backend and assemmbler phases for amdgcn since it does not support linking of object files.

yaxunl edited the summary of this revision. (Show Details)May 18 2018, 8:35 AM
yaxunl added reviewers: sfantao, t-tye.
tra added inline comments.May 18 2018, 3:50 PM
lib/Driver/Driver.cpp
2221

for(auto Arch: GpuArchList)

2265–2272

Please reformat.

2330–2332

Single-statement for does not need braces.

2485–2493

I'm not sure I understand what happens here and the comment does not help.
We appear to add each element of CudaDeviceActions to the action list of each linker input.

Does the comment mean that *only in linking mode* do we need to add dependency on device actions?

yaxunl marked 4 inline comments as done.May 22 2018, 10:33 AM
yaxunl added inline comments.
lib/Driver/Driver.cpp
2221

will do

2265–2272

will do

2330–2332

will do

2485–2493

Modified the comment to make it clearer.

We only add dependency on device action at linking phase. HIP embeds device image in host image in host linking phase. Since we need to link all device actions, we cannot create link action here since we have not went through all device actions yet. We just save device actions to DeviceLinkerInputs and create device link action later in appendLinkDependences, where all device actions have been went through.

yaxunl updated this revision to Diff 148051.May 22 2018, 10:38 AM
yaxunl marked 4 inline comments as done.

Revised by Artem's comments.

ping

Any further changes are needed? Thanks.

tra accepted this revision.May 29 2018, 11:45 AM

One nit. LGTM otherwise.

test/Driver/cuda-phases.cu
15

Please wrap long RUN lines in all tests.

This revision is now accepted and ready to land.May 29 2018, 11:45 AM
yaxunl marked an inline comment as done.May 29 2018, 12:08 PM
yaxunl added inline comments.
test/Driver/cuda-phases.cu
15

will do when commit. Thanks!

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