[PATCH] D116840: [HIP] Fix device only linking for -fgpu-rdc

Yaxun Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 10 07:44:48 PST 2022


yaxunl added inline comments.


================
Comment at: clang/lib/Driver/Driver.cpp:3173
+                  AssociatedOffloadKind);
+        AL.clear();
+        // Offload the host object to the host linker.
----------------
tra wrote:
> 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).
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. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116840/new/

https://reviews.llvm.org/D116840



More information about the cfe-commits mailing list