[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc
Artem Belevich via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 11 12:07:59 PDT 2020
tra added a comment.
Looks OK in general. I'm happy to see reduced opt/llc use.
You may want to get someone more familiar with the AMD GPU compilation process to double-check that the compilation pipeline still does the right thing.
================
Comment at: clang/lib/Driver/Driver.cpp:2725-2726
for (unsigned I = 0, E = GpuArchList.size(); I != E; ++I) {
// Create a link action to link device IR with device library
// and generate ISA.
+ CudaDeviceActions[I] = C.getDriver().ConstructPhaseAction(
----------------
The comment about "create link action" should probably be moved down below to where the link action is constructed now.
================
Comment at: clang/lib/Driver/Driver.cpp:2727-2732
+ CudaDeviceActions[I] = C.getDriver().ConstructPhaseAction(
+ C, Args, phases::Backend, CudaDeviceActions[I],
+ AssociatedOffloadKind);
+ CudaDeviceActions[I] = C.getDriver().ConstructPhaseAction(
+ C, Args, phases::Assemble, CudaDeviceActions[I],
+ AssociatedOffloadKind);
----------------
Looks like we're chaining backend/assembly actions here. but it's not easy to spot that we use `CudaDeviceActions[I]` as an intermediate value. At the first glance it looked like a copy/paste error writing to `CudaDeviceActions[I]` multiple times.
It would be easier to see what's going on if the code was structured like this:
```
BackendAction = Construct(... CudaDeviceActions[I]);
AssembleAction = Construct(... BackendAction);
AL.push_back(AssembleAction)
CudaDeviceActions[I] = C.MakeAction<LinkJobAction>(AL);
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81627/new/
https://reviews.llvm.org/D81627
More information about the cfe-commits
mailing list