yaxunl marked 4 inline comments as done.
yaxunl added inline comments.
================
Comment at: lib/Driver/Driver.cpp:2221
+ CudaDeviceActions.clear();
+ for (unsigned I = 0, E = GpuArchList.size(); I != E; ++I) {
+ CudaDeviceActions.push_back(UA);
----------------
tra wrote:
> `for(auto Arch: GpuArchList)`
will do
================
Comment at: lib/Driver/Driver.cpp:2265-2272
+ assert(AssociatedOffloadKind == Action::OFK_Cuda ||
AssociatedOffloadKind == Action::OFK_HIP);
+
// We don't need to support CUDA.
- if (!C.hasOffloadToolChain<Action::OFK_Cuda>())
+ if (AssociatedOffloadKind == Action::OFK_Cuda &&
!C.hasOffloadToolChain<Action::OFK_Cuda>())
+ return false;
+
+ // We don't need to support HIP.
----------------
tra wrote:
> Please reformat.
will do
================
Comment at: lib/Driver/Driver.cpp:2330-2332
+ for (CudaArch Arch : GpuArchs) {
GpuArchList.push_back(Arch);
+ }
----------------
tra wrote:
> Single-statement for does not need braces.
will do
================
Comment at: lib/Driver/Driver.cpp:2485-2493
+ // The host only depends on device action in the linking phase, when all
+ // the device images have to be embedded in the host image.
+ if (CurPhase == phases::Link) {
+ DeviceLinkerInputs.resize(CudaDeviceActions.size());
+ auto LI = DeviceLinkerInputs.begin();
+ for (auto *A : CudaDeviceActions) {
+ LI->push_back(A);
----------------
tra wrote:
> 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?
>
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.
https://reviews.llvm.org/D46476
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
