[OpenMP] Bugfix: output file name drops the absolute path where full path is needed.

[gtbercea]

Summary

When composing the output file name, the path to the file is being dropped. The full path is required.

[tra]

Shouldn't this temp .cubin file go into the temporary directory, as opposed to the same directory as the input file?

[gtbercea]

In D37912#872294, @tra wrote:

Shouldn't this temp .cubin file go into the temporary directory, as opposed to the same directory as the input file?

That is indeed the intention. The filename already contains the "/tmp/" I just make sure that doesn't get dropped.

[tra]

In D37912#872318, @gtbercea wrote:

In D37912#872294, @tra wrote:

Shouldn't this temp .cubin file go into the temporary directory, as opposed to the same directory as the input file?

That is indeed the intention. The filename already contains the "/tmp/" I just make sure that doesn't get dropped.

Is that guaranteed to always be the case? If not, you're just hiding the problem.
IMO, it would be better to construct a temporary path for the file, if that's the intention. All you need is TC.getDriver().GetTemporaryPath(...).

[gtbercea]

Fix tests.

[hfinkel]

lib/Driver/ToolChains/Cuda.cpp

442

Naming this FullPath seems inaccurate. It's not the full path, it's the root path (which is just the root_name + root_directory, but not any directory after that).

In any case, I don't see why all of this is necessary. Can't you just have:

SmallString<256> Name = II.getFilename();
llvm::sys::path::replace_extension(Name, "cubin");

const char *CubinF =
    C.addTempFile(C.getArgs().MakeArgString(Name));

[tra]

lib/Driver/ToolChains/Cuda.cpp

441

I don't think explicit StringRef is needed here. SmallString<256> Name(II.getFilename()); should do the job.

[gtbercea]

Address comment.

lib/Driver/ToolChains/Cuda.cpp

if (II.getType() == types::TY_LLVM_IR ||
       continue;
     }
 
     // Currently, we only pass the input files to the linker, we do not pass
     // any libraries that may be valid only for the host.
     if (!II.isFilename())
       continue;
 
-    SmallString<256> Name = llvm::sys::path::filename(II.getFilename());
+    SmallString<256> Name = StringRef(II.getFilename());
     llvm::sys::path::replace_extension(Name, "cubin");
 
     const char *CubinF =
         C.addTempFile(C.getArgs().MakeArgString(Name));
 
     CmdArgs.push_back(CubinF);
   }
 
   AddOpenMPLinkerScript(getToolChain(), C, Output, Inputs, Args, CmdArgs, JA);

test/Driver/openmp-offload-gpu.c

 /// ###########################################################################
 
 /// Check cubin file generation and usage by nvlink
 // RUN:   touch %t1.o
 // RUN:   touch %t2.o
 // RUN:   %clang -### -no-canonical-prefixes -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda %t1.o %t2.o 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-TWOCUBIN %s
 
-// CHK-TWOCUBIN: nvlink"{{.*}}"openmp-offload-{{.*}}.cubin" "openmp-offload-{{.*}}.cubin"
+// CHK-TWOCUBIN: nvlink{{.*}}openmp-offload-{{.*}}.cubin" "{{.*}}openmp-offload-{{.*}}.cubin"
 
 /// ###########################################################################
 
 /// Check cubin file generation and usage by nvlink when toolchain has BindArchAction
 // RUN:   touch %t1.o
 // RUN:   touch %t2.o
 // RUN:   %clang -### -no-canonical-prefixes -target x86_64-apple-darwin17.0.0 -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda %t1.o %t2.o 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-TWOCUBIN-DARWIN %s
 
-// CHK-TWOCUBIN-DARWIN: nvlink"{{.*}}"openmp-offload-{{.*}}.cubin" "openmp-offload-{{.*}}.cubin"
+// CHK-TWOCUBIN-DARWIN: nvlink{{.*}}openmp-offload-{{.*}}.cubin" "{{.*}}openmp-offload-{{.*}}.cubin"
 
 /// ###########################################################################
 
 /// Check PTXAS is passed -c flag when offloading to an NVIDIA device using OpenMP.
 // RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -no-canonical-prefixes %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-PTXAS-DEFAULT %s
 
 // CHK-PTXAS-DEFAULT: ptxas{{.*}}" "-c"