OpenMP: Fix object clobbering issue when using save-temps

[pdhaliwal]

Summary

There are two preconditions to reproduce the issue,

1. Use -save-temps option
2. Provide the -o option with name equal to the input file name without the file extension. For e.g. clang a.c -o a

With the -o specified, the AssembleJobAction after OffloadWrapperJobAction
will produce the object file with same name as host code object file.
Due to this clash, the OffloadWrapperAction overwrites the initial host
object file, which results in lld error. This also fixes the multiple definition of __dummy.omp_offloading.entry' issue in D96769 .

[pdhaliwal]

Here's a bit of background,
OffloadingPrefix was not getting properly set in the dependent actions of OffloadWrapperJobAction (which are backend [11] and assemble [12]). Since backend [11] and assemble [12] host-wrapper actions have same logic to the other host actions (3 & 4), those will overwrite the previous generated files from host-only actions.

For e.g. following were the names generated for output files previously (marked as bold). (clang -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -save-temps -ccc-print-bindings helloworld.c -o helloworld)

1. "x86_64-unknown-linux-gnu" - "clang", inputs: ["helloworld.c"], output: "helloworld-host-x86_64-unknown-linux-gnu.i"
2. "x86_64-unknown-linux-gnu" - "clang", inputs: ["helloworld-host-x86_64-unknown-linux-gnu.i"], output: "helloworld-host-x86_64-unknown-linux-gnu.bc"
3. "x86_64-unknown-linux-gnu" - "clang", inputs: ["helloworld-host-x86_64-unknown-linux-gnu.bc"], output: "helloworld-host-x86_64-unknown-linux-gnu.s"
4. "x86_64-unknown-linux-gnu" - "clang::as", inputs: ["helloworld-host-x86_64-unknown-linux-gnu.s"], output: "helloworld-host-x86_64-unknown-linux-gnu.o"
5. "nvptx64-nvidia-cuda" - "clang", inputs: ["helloworld.c"], output: "helloworld-openmp-nvptx64-nvidia-cuda.i"
6. "nvptx64-nvidia-cuda" - "clang", inputs: ["helloworld-openmp-nvptx64-nvidia-cuda.i", "helloworld-host-x86_64-unknown-linux-gnu.bc"], output: "helloworld-openmp-nvptx64-nvidia-cuda.bc"
7. "nvptx64-nvidia-cuda" - "clang", inputs: ["helloworld-openmp-nvptx64-nvidia-cuda.bc"], output: "helloworld-openmp-nvptx64-nvidia-cuda.s"
8. "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["helloworld-openmp-nvptx64-nvidia-cuda.s"], output: "helloworld-openmp-nvptx64-nvidia-cuda.o"
9. "nvptx64-nvidia-cuda" - "NVPTX::OpenMPLinker", inputs: ["helloworld-openmp-nvptx64-nvidia-cuda.o"], output: "a.out-openmp-nvptx64-nvidia-cuda"
10. "x86_64-unknown-linux-gnu" - "offload wrapper", inputs: ["a.out-openmp-nvptx64-nvidia-cuda"], output: "helloworld-host-x86_64-unknown-linux-gnu-wrapper.bc"
11. "x86_64-unknown-linux-gnu" - "clang", inputs: ["helloworld-host-x86_64-unknown-linux-gnu-wrapper.bc"], output: "helloworld-host-x86_64-unknown-linux-gnu.s"
12. "x86_64-unknown-linux-gnu" - "clang::as", inputs: ["helloworld-host-x86_64-unknown-linux-gnu.s"], output: "helloworld-host-x86_64-unknown-linux-gnu.o"
13. "x86_64-unknown-linux-gnu" - "GNU::Linker", inputs: ["helloworld-host-x86_64-unknown-linux-gnu.o", "helloworld-host-x86_64-unknown-linux-gnu.o"], output: "helloworld"

And here are names generated after this patch applied,

1. "x86_64-unknown-linux-gnu" - "clang", inputs: ["helloworld.c"], output: "helloworld-host-x86_64-unknown-linux-gnu.i"
2. "x86_64-unknown-linux-gnu" - "clang", inputs: ["helloworld-host-x86_64-unknown-linux-gnu.i"], output: "helloworld-host-x86_64-unknown-linux-gnu.bc"
3. "x86_64-unknown-linux-gnu" - "clang", inputs: ["helloworld-host-x86_64-unknown-linux-gnu.bc"], output: "helloworld-host-x86_64-unknown-linux-gnu.s"
4. "x86_64-unknown-linux-gnu" - "clang::as", inputs: ["helloworld-host-x86_64-unknown-linux-gnu.s"], output: "helloworld-host-x86_64-unknown-linux-gnu.o"
5. "nvptx64-nvidia-cuda" - "clang", inputs: ["helloworld.c"], output: "helloworld-openmp-nvptx64-nvidia-cuda.i"
6. "nvptx64-nvidia-cuda" - "clang", inputs: ["helloworld-openmp-nvptx64-nvidia-cuda.i", "helloworld-host-x86_64-unknown-linux-gnu.bc"], output: "helloworld-openmp-nvptx64-nvidia-cuda.bc"
7. "nvptx64-nvidia-cuda" - "clang", inputs: ["helloworld-openmp-nvptx64-nvidia-cuda.bc"], output: "helloworld-openmp-nvptx64-nvidia-cuda.s"
8. "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["helloworld-openmp-nvptx64-nvidia-cuda.s"], output: "helloworld-openmp-nvptx64-nvidia-cuda.o"
9. "nvptx64-nvidia-cuda" - "NVPTX::OpenMPLinker", inputs: ["helloworld-openmp-nvptx64-nvidia-cuda.o"], output: "a.out-openmp-nvptx64-nvidia-cuda"
10. "x86_64-unknown-linux-gnu" - "offload wrapper", inputs: ["a.out-openmp-nvptx64-nvidia-cuda"], output: "helloworld-wrapper-host-x86_64-unknown-linux-gnu.bc"
11. "x86_64-unknown-linux-gnu" - "clang", inputs: ["helloworld-wrapper-host-x86_64-unknown-linux-gnu.bc"], output: "helloworld-wrapper-host-x86_64-unknown-linux-gnu.s"
12. "x86_64-unknown-linux-gnu" - "clang::as", inputs: ["helloworld-wrapper-host-x86_64-unknown-linux-gnu.s"], output: "helloworld-wrapper-host-x86_64-unknown-linux-gnu.o"
13. "x86_64-unknown-linux-gnu" - "GNU::Linker", inputs: ["helloworld-host-x86_64-unknown-linux-gnu.o", "helloworld-wrapper-host-x86_64-unknown-linux-gnu.o"], output: "helloworld"

So for having OffloadingPrefix different for 11 & 12 would require to distinguish latter from 3 & 4 which I don't think is possible. However, the changes to BaseInput in OffloadWrapperJobAction [10] will also reflect in the dependent backend [11] and assemble [12] actions as BaseInput is present in InputInfo of the next actions (line number 4696).

[jdoerfert]

LGTM, assuming it doesn't break support the reasoning makes sense.

[JonChesterfield]

Works everywhere we have tried it. Fundamentally it renames a temporary file, so shouldn't break anything. Will be great to have -save-temps working for nvptx.

clang/lib/Driver/Driver.cpp

InputInfo Driver::BuildJobsForActionNoCache(
   else {
     // We only have to generate a prefix for the host if this is not a top-level
     // action.
     std::string OffloadingPrefix = Action::GetOffloadingFileNamePrefix(
         A->getOffloadingDeviceKind(), TC->getTriple().normalize(),
         /*CreatePrefixForHost=*/!!A->getOffloadingHostActiveKinds() &&
             !AtTopLevel);
     if (isa<OffloadWrapperJobAction>(JA)) {
-      OffloadingPrefix += "-wrapper";
       if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))
         BaseInput = FinalOutput->getValue();
       else
         BaseInput = getDefaultImageName();
+      BaseInput =
+          C.getArgs().MakeArgString(std::string(BaseInput) + "-wrapper");
     }
     Result = InputInfo(A, GetNamedOutputPath(C, *JA, BaseInput, BoundArch,
                                              AtTopLevel, MultipleArchs,
                                              OffloadingPrefix),
                        BaseInput);
   }
 
   if (CCCPrintBindings && !CCGenDiagnostics) {

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

 // RUN:   | FileCheck -check-prefix=CUDA_RED_RECS %s
 // CUDA_RED_RECS: clang{{.*}}"-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"
 // CUDA_RED_RECS-SAME: "-fopenmp-cuda-teams-reduction-recs-num=2048"
 
 // RUN:   %clang -### -no-canonical-prefixes -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=OPENMP_NVPTX_WRAPPERS %s
 // OPENMP_NVPTX_WRAPPERS: clang{{.*}}"-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"
 // OPENMP_NVPTX_WRAPPERS-SAME: "-internal-isystem" "{{.*}}openmp_wrappers"
+
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN:          -save-temps -no-canonical-prefixes -ccc-print-bindings %s -o openmp-offload-gpu 2>&1 \
+// RUN:   | FileCheck -check-prefix=SAVE_TEMPS_NAMES %s
+
+// SAVE_TEMPS_NAMES-NOT: "GNU::Linker"{{.*}}["[[SAVE_TEMPS_INPUT1:.*\.o]]", "[[SAVE_TEMPS_INPUT1]]"]