[CUDA][HIP] Fix new driver crashing when using -save-temps in RDC-mode

[jhuber6]

Summary

Previously when using the clang-offload-packager we did not pass the
active offloading kinds. Then in Clang when we attempted to detect when
there was host-offloading action that needed to be embedded in the host
we did not find it. This patch adds the active offloading kinds so we
know when there is input to be embedded.

[tra]

clang/lib/Driver/Driver.cpp

4394

getActiveOffloadKinds returns a mask of offload kinds, yet we cast it to OffloadKind in DeviceDependences::add above. This mixing of OffloadKind and sets of them looks questionable to me.

If we are relying on passing sets of OffloadKindnow, then we should make it clear in the code that it is the intent, including a more detailed description that DeviceOffloadKinds is a list of OffloadKind sets, so whoever iterates over elements does not attempt to compare its elements with OffloadKind values. I think it would be helpful to change DeviceOffloadKinds element type to a new class with a helper method hasOffloadKind(OffloadKind) to avoid accidental comparison of a mask with enum value.

[jhuber6]

Mixing the concept of the mask passing via an unsigned and a single enum value was incorrect. Add a new interface that accepts a mask and adds all active values instead.

[jhuber6]

clang/lib/Driver/Driver.cpp

4394

Yeah I think I shouldn't mix the logic here. I updated it.

[tra]

LGTM with a few nits.

clang/include/clang/Driver/Action.h

304

Nit: Add a*n* action

307

Nit: OffloadKindMask?

clang/lib/Driver/Action.cpp

320

It would be good to clear processed bits in Mask and then assert that it's zero after we're done. Otherwise it would be easy to miss handling a new offload kind if/when it gets added.

[jhuber6]

clang/lib/Driver/Action.cpp

320

I was also considering just putting something like this next to the declarations since we already do this for the new driver as well.

constexpr OffloadKind[] DeviceOffloadingKinds = {OFK_OpenMP, OFK_Cuda, OFK_HIP};

clang/include/clang/Driver/Action.h

private:
 
     /// The architectures that should be used with this action.
     BoundArchList DeviceBoundArchs;
 
     /// The offload kind of each dependence.
     OffloadKindList DeviceOffloadKinds;
 
   public:
-    /// Add a action along with the associated toolchain, bound arch, and
+    /// Add an action along with the associated toolchain, bound arch, and
     /// offload kind.
     void add(Action &A, const ToolChain &TC, const char *BoundArch,
              OffloadKind OKind);
 
+    /// Add an action along with the associated toolchain, bound arch, and
+    /// offload kinds.
+    void add(Action &A, const ToolChain &TC, const char *BoundArch,
+             unsigned OffloadKindMask);
+
     /// Get each of the individual arrays.
     const ActionList &getActions() const { return DeviceActions; }
     const ToolChainList &getToolChains() const { return DeviceToolChains; }
     const BoundArchList &getBoundArchs() const { return DeviceBoundArchs; }
     const OffloadKindList &getOffloadKinds() const {
       return DeviceOffloadKinds;
     }
   };

clang/lib/Driver/Action.cpp

void OffloadAction::DeviceDependences::add(Action &A, const ToolChain &TC,
                                            const char *BoundArch,
                                            OffloadKind OKind) {
   DeviceActions.push_back(&A);
   DeviceToolChains.push_back(&TC);
   DeviceBoundArchs.push_back(BoundArch);
   DeviceOffloadKinds.push_back(OKind);
 }
 
+void OffloadAction::DeviceDependences::add(Action &A, const ToolChain &TC,
+                                           const char *BoundArch,
+                                           unsigned OffloadKindMask) {
+  DeviceActions.push_back(&A);
+  DeviceToolChains.push_back(&TC);
+  DeviceBoundArchs.push_back(BoundArch);
+
+  // Add each active offloading kind from a mask.
+  for (OffloadKind OKind : {OFK_OpenMP, OFK_Cuda, OFK_HIP})
+    if (OKind & OffloadKindMask)
+      DeviceOffloadKinds.push_back(OKind);
+}
+
 OffloadAction::HostDependence::HostDependence(Action &A, const ToolChain &TC,
                                               const char *BoundArch,
                                               const DeviceDependences &DDeps)
     : HostAction(A), HostToolChain(TC), HostBoundArch(BoundArch) {
   for (auto K : DDeps.getOffloadKinds())
     HostOffloadKinds |= K;
 }
 

clang/lib/Driver/Driver.cpp

if (C.isOffloadingHostKind(Action::OFK_Cuda) &&
     DDep.add(*FatbinAction, *C.getSingleOffloadToolChain<Action::OFK_HIP>(),
              nullptr, Action::OFK_HIP);
   } else {
     // Package all the offloading actions into a single output that can be
     // embedded in the host and linked.
     Action *PackagerAction =
         C.MakeAction<OffloadPackagerJobAction>(OffloadActions, types::TY_Image);
     DDep.add(*PackagerAction, *C.getSingleOffloadToolChain<Action::OFK_Host>(),
-             nullptr, Action::OFK_None);
+             nullptr, C.getActiveOffloadKinds());
   }
 
   // If we are unable to embed a single device output into the host, we need to
   // add each device output as a host dependency to ensure they are still built.
   bool SingleDeviceOutput = !llvm::any_of(OffloadActions, [](Action *A) {
     return A->getType() == types::TY_Nothing;
   }) && isa<CompileJobAction>(HostAction);
   OffloadAction::HostDependence HDep(

clang/test/Driver/amdgpu-openmp-toolchain.c

 // CHECK-PHASES: 3: input, "[[INPUT]]", c, (device-openmp)
 // CHECK-PHASES: 4: preprocessor, {3}, cpp-output, (device-openmp)
 // CHECK-PHASES: 5: compiler, {4}, ir, (device-openmp)
 // CHECK-PHASES: 6: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, "device-openmp (amdgcn-amd-amdhsa)" {5}, ir
 // CHECK-PHASES: 7: backend, {6}, assembler, (device-openmp)
 // CHECK-PHASES: 8: assembler, {7}, object, (device-openmp)
 // CHECK-PHASES: 9: offload, "device-openmp (amdgcn-amd-amdhsa)" {8}, object
 // CHECK-PHASES: 10: clang-offload-packager, {9}, image
-// CHECK-PHASES: 11: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, " (x86_64-unknown-linux-gnu)" {10}, ir
+// CHECK-PHASES: 11: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, "device-openmp (x86_64-unknown-linux-gnu)" {10}, ir
 // CHECK-PHASES: 12: backend, {11}, assembler, (host-openmp)
 // CHECK-PHASES: 13: assembler, {12}, object, (host-openmp)
 // CHECK-PHASES: 14: clang-linker-wrapper, {13}, image, (host-openmp)
 
 // handling of --libomptarget-amdgpu-bc-path
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 --libomptarget-amdgpu-bc-path=%S/Inputs/hip_dev_lib/libomptarget-amdgpu-gfx803.bc %s 2>&1 | FileCheck %s --check-prefix=CHECK-LIBOMPTARGET
 // CHECK-LIBOMPTARGET: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-target-cpu" "gfx803" "-fcuda-is-device" "-mlink-builtin-bitcode"{{.*}}Inputs/hip_dev_lib/libomptarget-amdgpu-gfx803.bc"{{.*}}
 

clang/test/Driver/cuda-bindings.cu

 // driver.
 // 
 // RUN: %clang -### -target powerpc64le-ibm-linux-gnu --offload-new-driver \
 // RUN:        -fsyntax-only --offload-arch=sm_70 --offload-arch=sm_52 -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix=SYNTAX-ONLY %s
 // SYNTAX-ONLY: "-cc1" "-triple" "nvptx64-nvidia-cuda"{{.*}}"-fsyntax-only"
 // SYNTAX-ONLY: "-cc1" "-triple" "nvptx64-nvidia-cuda"{{.*}}"-fsyntax-only"
 // SYNTAX-ONLY: "-cc1" "-triple" "powerpc64le-ibm-linux-gnu"{{.*}}"-fsyntax-only"
+
+//
+// Check to ensure that we can use '-save-temps' when operating in RDC-mode.
+//
+// RUN: %clang -### -target powerpc64le-ibm-linux-gnu -save-temps --offload-new-driver \
+// RUN:        -fgpu-rdc --offload-arch=sm_70 --offload-arch=sm_52 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix=SAVE-TEMPS %s
+// SAVE-TEMPS: "-cc1" "-triple" "nvptx64-nvidia-cuda"{{.*}}"-target-cpu" "sm_52"
+// SAVE-TEMPS: "-cc1" "-triple" "nvptx64-nvidia-cuda"{{.*}}"-target-cpu" "sm_70"
+// SAVE-TEMPS: "-cc1" "-triple" "powerpc64le-ibm-linux-gnu"

clang/test/Driver/cuda-phases.cu

 // NEW-DRIVER-RDC-NEXT: 7: assembler, {6}, object, (device-cuda, sm_52)
 // NEW-DRIVER-RDC-NEXT: 8: offload, "device-cuda (nvptx64-nvidia-cuda:sm_52)" {7}, object
 // NEW-DRIVER-RDC-NEXT: 9: input, "[[INPUT]]", cuda, (device-cuda, sm_70)
 // NEW-DRIVER-RDC-NEXT: 10: preprocessor, {9}, cuda-cpp-output, (device-cuda, sm_70)
 // NEW-DRIVER-RDC-NEXT: 11: compiler, {10}, ir, (device-cuda, sm_70)
 // NEW-DRIVER-RDC-NEXT: 12: backend, {11}, assembler, (device-cuda, sm_70)
 // NEW-DRIVER-RDC-NEXT: 13: assembler, {12}, object, (device-cuda, sm_70)
 // NEW-DRIVER-RDC-NEXT: 14: offload, "device-cuda (nvptx64-nvidia-cuda:sm_70)" {13}, object
-// NEW-DRIVER-RDC-NEXT: 15: clang-offload-packager, {8, 14}, image
-// NEW-DRIVER-RDC-NEXT: 16: offload, " (powerpc64le-ibm-linux-gnu)" {2}, " (powerpc64le-ibm-linux-gnu)" {15}, ir
+// NEW-DRIVER-RDC-NEXT: 15: clang-offload-packager, {8, 14}, image, (device-cuda)
+// NEW-DRIVER-RDC-NEXT: 16: offload, "host-cuda (powerpc64le-ibm-linux-gnu)" {2}, "device-cuda (powerpc64le-ibm-linux-gnu)" {15}, ir
 // NEW-DRIVER-RDC-NEXT: 17: backend, {16}, assembler, (host-cuda)
 // NEW-DRIVER-RDC-NEXT: 18: assembler, {17}, object, (host-cuda)
 // NEW-DRIVER-RDC-NEXT: 19: clang-linker-wrapper, {18}, image, (host-cuda)
 
 // RUN: %clang -### -target powerpc64le-ibm-linux-gnu -ccc-print-phases --offload-new-driver -fgpu-rdc \
 // RUN:   --offload-arch=sm_52 --offload-arch=sm_70 %s 2>&1 | FileCheck --check-prefix=NEW-DRIVER %s
 //      NEW-DRIVER: 0: input, "[[INPUT:.+]]", cuda
 // NEW-DRIVER-NEXT: 1: preprocessor, {0}, cuda-cpp-output
 // NEW-DRIVER-NEXT: 2: compiler, {1}, ir
 // NEW-DRIVER-NEXT: 3: input, "[[INPUT]]", cuda, (device-cuda, sm_52)
 // NEW-DRIVER-NEXT: 4: preprocessor, {3}, cuda-cpp-output, (device-cuda, sm_52)
 // NEW-DRIVER-NEXT: 5: compiler, {4}, ir, (device-cuda, sm_52)
 // NEW-DRIVER-NEXT: 6: backend, {5}, assembler, (device-cuda, sm_52)
 // NEW-DRIVER-NEXT: 7: assembler, {6}, object, (device-cuda, sm_52)
 // NEW-DRIVER-NEXT: 8: offload, "device-cuda (nvptx64-nvidia-cuda:sm_52)" {7}, object
 // NEW-DRIVER-NEXT: 9: input, "[[INPUT]]", cuda, (device-cuda, sm_70)
 // NEW-DRIVER-NEXT: 10: preprocessor, {9}, cuda-cpp-output, (device-cuda, sm_70)
 // NEW-DRIVER-NEXT: 11: compiler, {10}, ir, (device-cuda, sm_70)
 // NEW-DRIVER-NEXT: 12: backend, {11}, assembler, (device-cuda, sm_70)
 // NEW-DRIVER-NEXT: 13: assembler, {12}, object, (device-cuda, sm_70)
 // NEW-DRIVER-NEXT: 14: offload, "device-cuda (nvptx64-nvidia-cuda:sm_70)" {13}, object
 // NEW-DRIVER-NEXT: 15: clang-offload-packager, {8, 14}, image
-// NEW-DRIVER-NEXT: 16: offload, " (powerpc64le-ibm-linux-gnu)" {2}, " (powerpc64le-ibm-linux-gnu)" {15}, ir
+// NEW-DRIVER-NEXT: 16: offload, "host-cuda (powerpc64le-ibm-linux-gnu)" {2}, "device-cuda (powerpc64le-ibm-linux-gnu)" {15}, ir
 // NEW-DRIVER-NEXT: 17: backend, {16}, assembler, (host-cuda)
 // NEW-DRIVER-NEXT: 18: assembler, {17}, object, (host-cuda)
 // NEW-DRIVER-NEXT: 19: clang-linker-wrapper, {18}, image, (host-cuda)
 
 // RUN: %clang -### --target=powerpc64le-ibm-linux-gnu -ccc-print-phases --offload-new-driver \
 // RUN:   --offload-arch=sm_52 --offload-arch=sm_70 %s %S/Inputs/empty.cpp 2>&1 | FileCheck --check-prefix=NON-CUDA-INPUT %s
 //      NON-CUDA-INPUT: 0: input, "[[CUDA:.+]]", cuda, (host-cuda)
 // NON-CUDA-INPUT-NEXT: 1: preprocessor, {0}, cuda-cpp-output, (host-cuda)

clang/test/Driver/openmp-offload-gpu.c

 // CHECK-PHASES: 3: input, "[[INPUT]]", c, (device-openmp)
 // CHECK-PHASES: 4: preprocessor, {3}, cpp-output, (device-openmp)
 // CHECK-PHASES: 5: compiler, {4}, ir, (device-openmp)
 // CHECK-PHASES: 6: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, "device-openmp (nvptx64-nvidia-cuda)" {5}, ir
 // CHECK-PHASES: 7: backend, {6}, assembler, (device-openmp)
 // CHECK-PHASES: 8: assembler, {7}, object, (device-openmp)
 // CHECK-PHASES: 9: offload, "device-openmp (nvptx64-nvidia-cuda)" {8}, object
 // CHECK-PHASES: 10: clang-offload-packager, {9}, image
-// CHECK-PHASES: 11: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, " (x86_64-unknown-linux-gnu)" {10}, ir
+// CHECK-PHASES: 11: offload, "host-openmp (x86_64-unknown-linux-gnu)" {2}, "device-openmp (x86_64-unknown-linux-gnu)" {10}, ir
 // CHECK-PHASES: 12: backend, {11}, assembler, (host-openmp)
 // CHECK-PHASES: 13: assembler, {12}, object, (host-openmp)
 // CHECK-PHASES: 14: clang-linker-wrapper, {13}, image, (host-openmp)
 
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_52 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-BINDINGS
 // CHECK-BINDINGS: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[HOST_BC:.+]]"
 // CHECK-BINDINGS: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[DEVICE_BC:.+]]"
 // CHECK-BINDINGS: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[DEVICE_BC]]"], output: "[[DEVICE_OBJ:.+]]"

clang/test/Driver/openmp-offload.c

 // CHK-PHASES-NEXT: 2: compiler, {1}, ir, (host-openmp)
 // CHK-PHASES-NEXT: 3: input, "[[INPUT]]", c, (device-openmp)
 // CHK-PHASES-NEXT: 4: preprocessor, {3}, cpp-output, (device-openmp)
 // CHK-PHASES-NEXT: 5: compiler, {4}, ir, (device-openmp)
 // CHK-PHASES-NEXT: 6: offload, "host-openmp (powerpc64-ibm-linux-gnu)" {2}, "device-openmp (powerpc64-ibm-linux-gnu)" {5}, ir
 // CHK-PHASES-NEXT: 7: backend, {6}, assembler, (device-openmp)
 // CHK-PHASES-NEXT: 8: assembler, {7}, object, (device-openmp)
 // CHK-PHASES-NEXT: 9: offload, "device-openmp (powerpc64-ibm-linux-gnu)" {8}, object
-// CHK-PHASES-NEXT: 10: clang-offload-packager, {9}, image
-// CHK-PHASES-NEXT: 11: offload, "host-openmp (powerpc64-ibm-linux-gnu)" {2}, " (powerpc64-ibm-linux-gnu)" {10}, ir
+// CHK-PHASES-NEXT: 10: clang-offload-packager, {9}, image, (device-openmp)
+// CHK-PHASES-NEXT: 11: offload, "host-openmp (powerpc64-ibm-linux-gnu)" {2}, "device-openmp (powerpc64-ibm-linux-gnu)" {10}, ir
 // CHK-PHASES-NEXT: 12: backend, {11}, assembler, (host-openmp)
 // CHK-PHASES-NEXT: 13: assembler, {12}, object, (host-openmp)
 // CHK-PHASES-NEXT: 14: clang-linker-wrapper, {13}, image, (host-openmp)
 
 /// ###########################################################################
 
 /// Check the phases when using multiple targets and multiple source files
 // RUN: %clang -ccc-print-phases -lsomelib -fopenmp=libomp --target=powerpc64-ibm-linux-gnu \

 // CHK-PHASES-FILES-NEXT: 10: offload, "device-openmp (x86_64-pc-linux-gnu)" {9}, object
 // CHK-PHASES-FILES-NEXT: 11: input, "[[INPUT]]", c, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 12: preprocessor, {11}, cpp-output, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 13: compiler, {12}, ir, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 14: offload, "host-openmp (powerpc64-ibm-linux-gnu)" {3}, "device-openmp (powerpc64-ibm-linux-gnu)" {13}, ir
 // CHK-PHASES-FILES-NEXT: 15: backend, {14}, assembler, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 16: assembler, {15}, object, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 17: offload, "device-openmp (powerpc64-ibm-linux-gnu)" {16}, object
-// CHK-PHASES-FILES-NEXT: 18: clang-offload-packager, {10, 17}, image
-// CHK-PHASES-FILES-NEXT: 19: offload, "host-openmp (powerpc64-ibm-linux-gnu)" {3}, " (powerpc64-ibm-linux-gnu)" {18}, ir
+// CHK-PHASES-FILES-NEXT: 18: clang-offload-packager, {10, 17}, image, (device-openmp)
+// CHK-PHASES-FILES-NEXT: 19: offload, "host-openmp (powerpc64-ibm-linux-gnu)" {3}, "device-openmp (powerpc64-ibm-linux-gnu)" {18}, ir
 // CHK-PHASES-FILES-NEXT: 20: backend, {19}, assembler, (host-openmp)
 // CHK-PHASES-FILES-NEXT: 21: assembler, {20}, object, (host-openmp)
 // CHK-PHASES-FILES-NEXT: 22: input, "[[INPUT]]", c, (host-openmp)
 // CHK-PHASES-FILES-NEXT: 23: preprocessor, {22}, cpp-output, (host-openmp)
 // CHK-PHASES-FILES-NEXT: 24: compiler, {23}, ir, (host-openmp)
 // CHK-PHASES-FILES-NEXT: 25: input, "[[INPUT]]", c, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 26: preprocessor, {25}, cpp-output, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 27: compiler, {26}, ir, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 28: offload, "host-openmp (powerpc64-ibm-linux-gnu)" {24}, "device-openmp (x86_64-pc-linux-gnu)" {27}, ir
 // CHK-PHASES-FILES-NEXT: 29: backend, {28}, assembler, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 30: assembler, {29}, object, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 31: offload, "device-openmp (x86_64-pc-linux-gnu)" {30}, object
 // CHK-PHASES-FILES-NEXT: 32: input, "[[INPUT]]", c, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 33: preprocessor, {32}, cpp-output, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 34: compiler, {33}, ir, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 35: offload, "host-openmp (powerpc64-ibm-linux-gnu)" {24}, "device-openmp (powerpc64-ibm-linux-gnu)" {34}, ir
 // CHK-PHASES-FILES-NEXT: 36: backend, {35}, assembler, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 37: assembler, {36}, object, (device-openmp)
 // CHK-PHASES-FILES-NEXT: 38: offload, "device-openmp (powerpc64-ibm-linux-gnu)" {37}, object
-// CHK-PHASES-FILES-NEXT: 39: clang-offload-packager, {31, 38}, image
-// CHK-PHASES-FILES-NEXT: 40: offload, "host-openmp (powerpc64-ibm-linux-gnu)" {24}, " (powerpc64-ibm-linux-gnu)" {39}, ir
+// CHK-PHASES-FILES-NEXT: 39: clang-offload-packager, {31, 38}, image, (device-openmp)
+// CHK-PHASES-FILES-NEXT: 40: offload, "host-openmp (powerpc64-ibm-linux-gnu)" {24}, "device-openmp (powerpc64-ibm-linux-gnu)" {39}, ir
 // CHK-PHASES-FILES-NEXT: 41: backend, {40}, assembler, (host-openmp)
 // CHK-PHASES-FILES-NEXT: 42: assembler, {41}, object, (host-openmp)
 // CHK-PHASES-FILES-NEXT: 43: clang-linker-wrapper, {0, 21, 42}, image, (host-openmp)
 
 /// Check -fopenmp-is-device is passed when compiling for the device.
 // RUN:   %clang -### --target=powerpc64le-linux -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-FOPENMP-IS-DEVICE %s