This is an archive of the discontinued LLVM Phabricator instance.

[HIP] Fix device only linking for -fgpu-rdc
ClosedPublic

Authored by yaxunl on Jan 7 2022, 2:28 PM.

Details

Summary

Currently when -fgpu-rdc is specified, HIP toolchain always does host linking even
if --cuda-device-only is specified.

This patch fixes that. Only device linking is performed when --cuda-device-only
is specified.

Diff Detail

Event Timeline

yaxunl requested review of this revision.Jan 7 2022, 2:28 PM
yaxunl created this revision.
tra added inline comments.Jan 7 2022, 3:09 PM
clang/lib/Driver/Driver.cpp
3173

Doing clear() in a function intended to append looks suspicious.
We may potentially discard whatever was in AL on entry to the function, plus whatever has been added per DeviceLinkerInputs.

I wonder if a better API would be to return a new action list, instead of modifying one.

At the very least I'd like to see a comment explaining what's going on here and, maybe, some assertions (e.g. if we expect AL to be empty on entry).

yaxunl added inline comments.Jan 10 2022, 7:44 AM
clang/lib/Driver/Driver.cpp
3173

Good point.

What we want to do here is that if bundling is disabled, append link actions, otherwise create a bundle action with link actions as input, then append bundle action.

I will save link actions to some temporary action list instead of using AL. Then I can avoid clear previous entries in AL unintentionally.

yaxunl updated this revision to Diff 398662.Jan 10 2022, 8:53 AM

avoid clearing AL

tra accepted this revision.Jan 10 2022, 10:31 AM
This revision is now accepted and ready to land.Jan 10 2022, 10:31 AM
This revision was landed with ongoing or failed builds.Jan 10 2022, 2:38 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJan 10 2022, 2:38 PM