[CUDA][HIP] Fix -gsplit-dwarf option

[yaxunl]

Summary

when -gsplit option is used with clang driver, clang driver will create
a filename with .dwo option based on the input file name and pass
it to clang -cc1. This file is used for storing the debug info. Since
CUDA/HIP generate separate object files for different GPU arch's,
this file should be different for different GPU arch. This patch
adds _ and GPU arch to the stem of the dwo file.

[tra]

Does this naming scheme the same as used for .o files? We may want to keep them in sync.

Other than that, LGTM.

clang/lib/Driver/ToolChains/CommonArgs.cpp

909

I think the same approach would make sense for CUDA, too.

[yaxunl]

In D87791#2277887, @tra wrote:

Does this naming scheme the same as used for .o files? We may want to keep them in sync.

Other than that, LGTM.

.o file is different story.

For -fno-gpu-rdc, the .o files for device compilation are temporary files which are deleted after the device ISA are generated and embedded in host .o file. There is only one output .o file which is the host object file.

For -fgpu-rdc, the .o files for device compilation are also temporary files which are bundled into the clang-offload-bundle. There is only one output .o file which is a bundle.

Therefore in either case there is no need to rename the intermediate .o files since they are temporary files which have unique names.

The .dwo files are not temporary files. They are supposed to be shipped with .o files for debugging info.

Since .dwo files are not temporary files, it is not necessary to follow the -save-temps name convention. For the host object, we keep the original .dwo file name. For the device object, we add '_' and GPU arch to the stem, which is sufficient and concise.

clang/lib/Driver/ToolChains/CommonArgs.cpp

909

will include OFK_CUDA.

[tra]

In D87791#2278417, @yaxunl wrote:

Therefore in either case there is no need to rename the intermediate .o files since they are temporary files which have unique names.

The .dwo files are not temporary files. They are supposed to be shipped with .o files for debugging info.

Ack.
BTW, is split-dwarf useful for AMD GPUs on device side? I don't think we can currently utilize DWO files on device side with CUDA at all. To think of it, it's probably going to break GPU-side debugging as CUDA can only deal with dwarf info embedded in the GPU binary.
If it does not work for AMD GPUs, perhaps we should just disable it for GPUs.

Since .dwo files are not temporary files, it is not necessary to follow the -save-temps name convention. For the host object, we keep the original .dwo file name. For the device object, we add '_' and GPU arch to the stem, which is sufficient and concise.

What will happen with -save-temps ? Will dwo files match object file names?

[yaxunl]

In D87791#2279821, @tra wrote:

In D87791#2278417, @yaxunl wrote:

Therefore in either case there is no need to rename the intermediate .o files since they are temporary files which have unique names.

The .dwo files are not temporary files. They are supposed to be shipped with .o files for debugging info.

Ack.
BTW, is split-dwarf useful for AMD GPUs on device side? I don't think we can currently utilize DWO files on device side with CUDA at all. To think of it, it's probably going to break GPU-side debugging as CUDA can only deal with dwarf info embedded in the GPU binary.
If it does not work for AMD GPUs, perhaps we should just disable it for GPUs.

It is requested by our debugger team, so it should work with amdgpu.

Since .dwo files are not temporary files, it is not necessary to follow the -save-temps name convention. For the host object, we keep the original .dwo file name. For the device object, we add '_' and GPU arch to the stem, which is sufficient and concise.

What will happen with -save-temps ? Will dwo files match object file names?

with -save-temps, the saved temporary files will be like test-hip-amdgcn-amd-amdhsa-gfx906.o. The dwo file will be like test_gfx906.dwo.

[tra]

In D87791#2279864, @yaxunl wrote:

It is requested by our debugger team, so it should work with amdgpu.

Is the naming scheme for GPU-side DWO files dictated by debugger? If that's the case, it may be worth adding a comment about that.

LGTM.

[MaskRay]

(Note that ideally -gsplit-dwarf should not imply -g2 but it currents does so. And Clang and GCC have not agreed whether we should add a new flag like -fsplit-dwarf. /For -gsplit-dwarf builds, it is the best to ensure -g is also specified/.)

clang/lib/Driver/ToolChains/CommonArgs.cpp

920–921

Does stem change the semantics?

[yaxunl]

In D87791#2279885, @tra wrote:

In D87791#2279864, @yaxunl wrote:

It is requested by our debugger team, so it should work with amdgpu.

Is the naming scheme for GPU-side DWO files dictated by debugger? If that's the case, it may be worth adding a comment about that.

LGTM.

It is the preferred naming by the debugger. Will add a comment.

clang/lib/Driver/ToolChains/CommonArgs.cpp

920–921

No. replace_extension will take stem first and then add new extension.

clang/lib/Driver/ToolChains/Clang.cpp

#endif
   // Add the split debug info name to the command lines here so we
   // can propagate it to the backend.
   bool SplitDWARF = (DwarfFission != DwarfFissionKind::None) &&
                     (TC.getTriple().isOSBinFormatELF() ||
                      TC.getTriple().isOSBinFormatWasm()) &&
                     (isa<AssembleJobAction>(JA) || isa<CompileJobAction>(JA) ||
                      isa<BackendJobAction>(JA));
   if (SplitDWARF) {
-    const char *SplitDWARFOut = SplitDebugName(Args, Input, Output);
+    const char *SplitDWARFOut = SplitDebugName(JA, Args, Input, Output);
     CmdArgs.push_back("-split-dwarf-file");
     CmdArgs.push_back(SplitDWARFOut);
     if (DwarfFission == DwarfFissionKind::Split) {
       CmdArgs.push_back("-split-dwarf-output");
       CmdArgs.push_back(SplitDWARFOut);
     }
   }
 
void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
   const llvm::Triple &T = getToolChain().getTriple();
   Arg *A;
   if (getDebugFissionKind(D, Args, A) == DwarfFissionKind::Split &&
       T.isOSBinFormatELF()) {
     CmdArgs.push_back("-split-dwarf-output");
-    CmdArgs.push_back(SplitDebugName(Args, Input, Output));
+    CmdArgs.push_back(SplitDebugName(JA, Args, Input, Output));
   }
 
   assert(Input.isFilename() && "Invalid input.");
   CmdArgs.push_back(Input.getFilename());
 
   const char *Exec = getToolChain().getDriver().getClangProgramPath();
   C.addCommand(std::make_unique<Command>(
       JA, *this, ResponseFileSupport::AtFileUTF8(), Exec, CmdArgs, Inputs));

clang/lib/Driver/ToolChains/CommonArgs.h

 
 void linkXRayRuntimeDeps(const ToolChain &TC,
                          llvm::opt::ArgStringList &CmdArgs);
 
 void AddRunTimeLibs(const ToolChain &TC, const Driver &D,
                     llvm::opt::ArgStringList &CmdArgs,
                     const llvm::opt::ArgList &Args);
 
-const char *SplitDebugName(const llvm::opt::ArgList &Args,
+const char *SplitDebugName(const JobAction &JA, const llvm::opt::ArgList &Args,
                            const InputInfo &Input, const InputInfo &Output);
 
 void SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,
                     const JobAction &JA, const llvm::opt::ArgList &Args,
                     const InputInfo &Output, const char *OutFile);
 
 void addLTOOptions(const ToolChain &ToolChain, const llvm::opt::ArgList &Args,
                    llvm::opt::ArgStringList &CmdArgs, const InputInfo &Output,

clang/lib/Driver/ToolChains/CommonArgs.cpp

 bool tools::areOptimizationsEnabled(const ArgList &Args) {
   // Find the last -O arg and see if it is non-zero.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group))
     return !A->getOption().matches(options::OPT_O0);
   // Defaults to -O0.
   return false;
 }
 
-const char *tools::SplitDebugName(const ArgList &Args, const InputInfo &Input,
+const char *tools::SplitDebugName(const JobAction &JA, const ArgList &Args,
+                                  const InputInfo &Input,
                                   const InputInfo &Output) {
+  // Adds '_' and GPU arch to the stem of .dwo file for HIP, which is
+  // expected by gdb.
+  auto AddPostfix = [JA](auto &F) {
+    if (JA.getOffloadingDeviceKind() == Action::OFK_HIP)
+      F += (Twine("_") + JA.getOffloadingArch()).str();
+  };
   if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
     if (StringRef(A->getValue()) == "single")
       return Args.MakeArgString(Output.getFilename());
 
   Arg *FinalOutput = Args.getLastArg(options::OPT_o);
   if (FinalOutput && Args.hasArg(options::OPT_c)) {
-    SmallString<128> T(FinalOutput->getValue());
+    SmallString<128> T(llvm::sys::path::stem(FinalOutput->getValue()));
+    AddPostfix(T);
     llvm::sys::path::replace_extension(T, "dwo");
     return Args.MakeArgString(T);
   } else {
     // Use the compilation dir.
     SmallString<128> T(
         Args.getLastArgValue(options::OPT_fdebug_compilation_dir));
     SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput()));
+    AddPostfix(F);
     llvm::sys::path::replace_extension(F, "dwo");
     T += F;
     return Args.MakeArgString(F);
   }
 }
 
 void tools::SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,
                            const JobAction &JA, const ArgList &Args,

clang/lib/Driver/ToolChains/Gnu.cpp

C.addCommand(std::make_unique<Command>(
       JA, *this, ResponseFileSupport::AtFileCurCP(), Exec, CmdArgs, Inputs));
 
   // Handle the debug info splitting at object creation time if we're
   // creating an object.
   // TODO: Currently only works on linux with newer objcopy.
   if (Args.hasArg(options::OPT_gsplit_dwarf) &&
       getToolChain().getTriple().isOSLinux())
     SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
-                   SplitDebugName(Args, Inputs[0], Output));
+                   SplitDebugName(JA, Args, Inputs[0], Output));
 }
 
 namespace {
 // Filter to remove Multilibs that don't exist as a suffix to Path
 class FilterNonExistent {
   StringRef Base, File;
   llvm::vfs::FileSystem &VFS;
 

clang/lib/Driver/ToolChains/MinGW.cpp

for (const auto &II : Inputs)
     CmdArgs.push_back(II.getFilename());
 
   const char *Exec = Args.MakeArgString(getToolChain().GetProgramPath("as"));
   C.addCommand(std::make_unique<Command>(JA, *this, ResponseFileSupport::None(),
                                          Exec, CmdArgs, Inputs));
 
   if (Args.hasArg(options::OPT_gsplit_dwarf))
     SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
-                   SplitDebugName(Args, Inputs[0], Output));
+                   SplitDebugName(JA, Args, Inputs[0], Output));
 }
 
 void tools::MinGW::Linker::AddLibGCC(const ArgList &Args,
                                      ArgStringList &CmdArgs) const {
   if (Args.hasArg(options::OPT_mthreads))
     CmdArgs.push_back("-lmingwthrd");
   CmdArgs.push_back("-lmingw32");
 

clang/test/Driver/hip-gsplit-dwarf-options.hip

+// REQUIRES: zlib, clang-driver, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -c \
+// RUN:   --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -c \
+// RUN:   -fgpu-rdc --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu \
+// RUN:   -fgpu-rdc --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s
+
+// CHECK-DAG: {{".*clang.*".* "-target-cpu" "gfx906".* "-split-dwarf-output" "hip-gsplit-dwarf-options_gfx906:xnack\+.dwo"}}
+// CHECK-DAG: {{".*clang.*".* "-target-cpu" "gfx900".* "-split-dwarf-output" "hip-gsplit-dwarf-options_gfx900.dwo"}}
+// CHECK-DAG: {{".*clang.*".* "-target-cpu" "x86-64".* "-split-dwarf-output" "hip-gsplit-dwarf-options.dwo"}}