[PATCH] D46476: [HIP] Add action builder for HIP

[Artem Belevich via Phabricator via cfe-commits]

tra 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);
----------------
`for(auto Arch: GpuArchList)`


================
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.
----------------
Please reformat.


================
Comment at: lib/Driver/Driver.cpp:2330-2332
+      for (CudaArch Arch : GpuArchs) {
         GpuArchList.push_back(Arch);
+      }
----------------
Single-statement for does not need braces.


================
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);
----------------
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?



https://reviews.llvm.org/D46476