[HIP] Fix -save-temps

[yaxunl]

Summary

Currently clang does not save some of the intermediate file generated during device compilation for HIP when -save-temps is specified.

This patch fixes that.

[tra]

lib/Driver/Driver.cpp

4400 ↗

(On Diff #223944)

... so we derive the file name from the main file name.

4405 ↗

(On Diff #223944)

llvm::sys::path::replace_extension(Output, ""); ?

lib/Driver/ToolChains/HIP.cpp

253 ↗

(On Diff #223944)

/*OutputIsAsm=*/true

lib/Driver/ToolChains/HIP.h

62 ↗

(On Diff #223944)

OutputIsAsm? Otherwise it's not quite clear what IsAsm refers to.

[yaxunl]

lib/Driver/Driver.cpp

4400 ↗

(On Diff #223944)

for -fno-gpu-rdc, there is only one file. We derive the name from that file. In -fgpu-rdc case, the file name is determined by the output file name elsewhere.

4405 ↗

(On Diff #223944)

will do when committing

lib/Driver/ToolChains/HIP.cpp

253 ↗

(On Diff #223944)

will do

lib/Driver/ToolChains/HIP.h

62 ↗

(On Diff #223944)

will do

[ashi1]

LGTM

clang/lib/Driver/Driver.cpp

NamedOutput =
         MakeCLOutputFilename(C.getArgs(), Val, BaseName, types::TY_Image);
   } else if (JA.getType() == types::TY_Image) {
     if (IsCLMode()) {
       // clang-cl uses BaseName for the executable name.
       NamedOutput =
           MakeCLOutputFilename(C.getArgs(), "", BaseName, types::TY_Image);
     } else {
       SmallString<128> Output(getDefaultImageName());
+      // HIP image for device compilation with -fno-gpu-rdc is per compilation
+      // unit.
+      bool IsHIPNoRDC = JA.getOffloadingDeviceKind() == Action::OFK_HIP &&
+                        !C.getArgs().hasFlag(options::OPT_fgpu_rdc,
+                                             options::OPT_fno_gpu_rdc, false);
+      if (IsHIPNoRDC) {
+        Output = BaseName;
+        llvm::sys::path::replace_extension(Output, "");
+      }
       Output += OffloadingPrefix;
       if (MultipleArchs && !BoundArch.empty()) {
         Output += "-";
         Output.append(BoundArch);
       }
+      if (IsHIPNoRDC)
+        Output += ".out";
       NamedOutput = C.getArgs().MakeArgString(Output.c_str());
     }
   } else if (JA.getType() == types::TY_PCH && IsCLMode()) {
     NamedOutput = C.getArgs().MakeArgString(GetClPchPath(C, BaseName));
   } else {
     const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
     assert(Suffix && "All types used for output should have a suffix.");
 

clang/lib/Driver/ToolChains/CommonArgs.cpp

for (const auto &II : Inputs) {
     }
   }
 
   if (DeviceInputs.empty())
     return;
 
   // Create temporary linker script. Keep it if save-temps is enabled.
   const char *LKS;
-  SmallString<256> Name = llvm::sys::path::filename(Output.getFilename());
+  std::string Name = llvm::sys::path::filename(Output.getFilename());
   if (C.getDriver().isSaveTempsEnabled()) {
-    llvm::sys::path::replace_extension(Name, "lk");
-    LKS = C.getArgs().MakeArgString(Name.c_str());
+    LKS = C.getArgs().MakeArgString(Name + ".lk");
   } else {
-    llvm::sys::path::replace_extension(Name, "");
-    Name = C.getDriver().GetTemporaryPath(Name, "lk");
-    LKS = C.addTempFile(C.getArgs().MakeArgString(Name.c_str()));
+    auto TmpName = C.getDriver().GetTemporaryPath(Name, "lk");
+    LKS = C.addTempFile(C.getArgs().MakeArgString(TmpName));
   }
 
   // Add linker script option to the command.
   CmdArgs.push_back("-T");
   CmdArgs.push_back(LKS);
 
   // Create a buffer to write the contents of the linker script.
   std::string LksBuffer;
   llvm::raw_string_ostream LksStream(LksBuffer);
 
   // Get the HIP offload tool chain.
   auto *HIPTC = static_cast<const toolchains::CudaToolChain *>(
       C.getSingleOffloadToolChain<Action::OFK_HIP>());
   assert(HIPTC->getTriple().getArch() == llvm::Triple::amdgcn &&
          "Wrong platform");
   (void)HIPTC;
 
-  // The output file name needs to persist through the compilation, therefore
-  // it needs to be created through MakeArgString.
-  std::string BundleFileName = C.getDriver().GetTemporaryPath("BUNDLE", "hipfb");
-  const char *BundleFile =
-      C.addTempFile(C.getArgs().MakeArgString(BundleFileName.c_str()));
+  const char *BundleFile;
+  if (C.getDriver().isSaveTempsEnabled()) {
+    BundleFile = C.getArgs().MakeArgString(Name + ".hipfb");
+  } else {
+    auto TmpName = C.getDriver().GetTemporaryPath(Name, "hipfb");
+    BundleFile = C.addTempFile(C.getArgs().MakeArgString(TmpName));
+  }
   AMDGCN::constructHIPFatbinCommand(C, JA, BundleFile, DeviceInputs, Args, T);
 
   // Add commands to embed target binaries. We ensure that each section and
   // image is 16-byte aligned. This is not mandatory, but increases the
   // likelihood of data to be aligned with a cache block in several main host
   // machines.
   LksStream << "/*\n";
   LksStream << "       HIP Offload Linker Script\n";
   LksStream << " *** Automatically generated by Clang ***\n";
   LksStream << "*/\n";
   LksStream << "TARGET(binary)\n";
-  LksStream << "INPUT(" << BundleFileName << ")\n";
+  LksStream << "INPUT(" << BundleFile << ")\n";
   LksStream << "SECTIONS\n";
   LksStream << "{\n";
   LksStream << "  .hip_fatbin :\n";
   LksStream << "  ALIGN(0x10)\n";
   LksStream << "  {\n";
   LksStream << "    PROVIDE_HIDDEN(__hip_fatbin = .);\n";
-  LksStream << "    " << BundleFileName << "\n";
+  LksStream << "    " << BundleFile << "\n";
   LksStream << "  }\n";
   LksStream << "  /DISCARD/ :\n";
   LksStream << "  {\n";
   LksStream << "    * ( __CLANG_OFFLOAD_BUNDLE__* )\n";
   LksStream << "  }\n";
   LksStream << "}\n";
   LksStream << "INSERT BEFORE .data\n";
   LksStream.flush();

clang/lib/Driver/ToolChains/HIP.h

const char *constructOptCommand(Compilation &C, const JobAction &JA,
                                   const char *InputFileName) const;
 
   /// \return llc output file name.
   const char *constructLlcCommand(Compilation &C, const JobAction &JA,
                                   const InputInfoList &Inputs,
                                   const llvm::opt::ArgList &Args,
                                   llvm::StringRef SubArchName,
                                   llvm::StringRef OutputFilePrefix,
-                                  const char *InputFileName) const;
+                                  const char *InputFileName,
+                                  bool OutputIsAsm = false) const;
 
   void constructLldCommand(Compilation &C, const JobAction &JA,
                            const InputInfoList &Inputs, const InputInfo &Output,
                            const llvm::opt::ArgList &Args,
                            const char *InputFileName) const;
 };
 
 } // end namespace AMDGCN

clang/lib/Driver/ToolChains/HIP.cpp

if (llvm::sys::fs::exists(FullName)) {
       CmdArgs.push_back("-mlink-builtin-bitcode");
       CmdArgs.push_back(Args.MakeArgString(FullName));
       return;
     }
   }
   D.Diag(diag::err_drv_no_such_file) << BCName;
 }
 
+static const char *getOutputFileName(Compilation &C, StringRef Base,
+                                     const char *Postfix,
+                                     const char *Extension) {
+  const char *OutputFileName;
+  if (C.getDriver().isSaveTempsEnabled()) {
+    OutputFileName =
+        C.getArgs().MakeArgString(Base.str() + Postfix + "." + Extension);
+  } else {
+    std::string TmpName =
+        C.getDriver().GetTemporaryPath(Base.str() + Postfix, Extension);
+    OutputFileName = C.addTempFile(C.getArgs().MakeArgString(TmpName));
+  }
+  return OutputFileName;
+}
 } // namespace
 
 const char *AMDGCN::Linker::constructLLVMLinkCommand(
     Compilation &C, const JobAction &JA, const InputInfoList &Inputs,
     const ArgList &Args, StringRef SubArchName,
     StringRef OutputFilePrefix) const {
   ArgStringList CmdArgs;
   // Add the input bc's created by compile step.
   for (const auto &II : Inputs)
     CmdArgs.push_back(II.getFilename());
 
   // Add an intermediate output file.
   CmdArgs.push_back("-o");
-  std::string TmpName =
-      C.getDriver().GetTemporaryPath(OutputFilePrefix.str() + "-linked", "bc");
-  const char *OutputFileName =
-      C.addTempFile(C.getArgs().MakeArgString(TmpName));
+  auto OutputFileName = getOutputFileName(C, OutputFilePrefix, "-linked", "bc");
   CmdArgs.push_back(OutputFileName);
   SmallString<128> ExecPath(C.getDriver().Dir);
   llvm::sys::path::append(ExecPath, "llvm-link");
   const char *Exec = Args.MakeArgString(ExecPath);
   C.addCommand(std::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
   return OutputFileName;
 }
 
const char *AMDGCN::Linker::constructOptCommand(
   OptArgs.push_back("-mtriple=amdgcn-amd-amdhsa");
   OptArgs.push_back(Args.MakeArgString("-mcpu=" + SubArchName));
 
   for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
     OptArgs.push_back(A->getValue(0));
   }
 
   OptArgs.push_back("-o");
-  std::string TmpFileName = C.getDriver().GetTemporaryPath(
-      OutputFilePrefix.str() + "-optimized", "bc");
-  const char *OutputFileName =
-      C.addTempFile(C.getArgs().MakeArgString(TmpFileName));
+  auto OutputFileName =
+      getOutputFileName(C, OutputFilePrefix, "-optimized", "bc");
   OptArgs.push_back(OutputFileName);
   SmallString<128> OptPath(C.getDriver().Dir);
   llvm::sys::path::append(OptPath, "opt");
   const char *OptExec = Args.MakeArgString(OptPath);
   C.addCommand(std::make_unique<Command>(JA, *this, OptExec, OptArgs, Inputs));
   return OutputFileName;
 }
 
 const char *AMDGCN::Linker::constructLlcCommand(
     Compilation &C, const JobAction &JA, const InputInfoList &Inputs,
     const llvm::opt::ArgList &Args, llvm::StringRef SubArchName,
-    llvm::StringRef OutputFilePrefix, const char *InputFileName) const {
+    llvm::StringRef OutputFilePrefix, const char *InputFileName,
+    bool OutputIsAsm) const {
   // Construct llc command.
-  ArgStringList LlcArgs{InputFileName, "-mtriple=amdgcn-amd-amdhsa",
-                        "-filetype=obj",
-                        Args.MakeArgString("-mcpu=" + SubArchName)};
+  ArgStringList LlcArgs{
+      InputFileName, "-mtriple=amdgcn-amd-amdhsa",
+      Args.MakeArgString(Twine("-filetype=") + (OutputIsAsm ? "asm" : "obj")),
 
   // Extract all the -m options
   std::vector<llvm::StringRef> Features;
   handleTargetFeaturesGroup(
     Args, Features, options::OPT_m_amdgpu_Features_Group);
 
   // Add features to mattr such as xnack
   std::string MAttrString = "-mattr=";
   for(auto OneFeature : Features) {
     MAttrString.append(Args.MakeArgString(OneFeature));
     if (OneFeature != Features.back())
       MAttrString.append(",");
   }
   if(!Features.empty())
     LlcArgs.push_back(Args.MakeArgString(MAttrString));
 
   for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
     LlcArgs.push_back(A->getValue(0));
   }
 
   // Add output filename
   LlcArgs.push_back("-o");
-  std::string LlcOutputFileName =
-      C.getDriver().GetTemporaryPath(OutputFilePrefix, "o");
-  const char *LlcOutputFile =
-      C.addTempFile(C.getArgs().MakeArgString(LlcOutputFileName));
+  auto LlcOutputFile =
+      getOutputFileName(C, OutputFilePrefix, "", OutputIsAsm ? "s" : "o");
   LlcArgs.push_back(LlcOutputFile);
   SmallString<128> LlcPath(C.getDriver().Dir);
   llvm::sys::path::append(LlcPath, "llc");
   const char *Llc = Args.MakeArgString(LlcPath);
   C.addCommand(std::make_unique<Command>(JA, *this, Llc, LlcArgs, Inputs));
   return LlcOutputFile;
 }
 
void AMDGCN::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
   assert(getToolChain().getTriple().getArch() == llvm::Triple::amdgcn &&
          "Unsupported target");
 
   std::string SubArchName = JA.getOffloadingArch();
   assert(StringRef(SubArchName).startswith("gfx") && "Unsupported sub arch");
 
   // Prefix for temporary file name.
-  std::string Prefix =
-      llvm::sys::path::stem(Inputs[0].getFilename()).str() + "-" + SubArchName;
+  std::string Prefix = llvm::sys::path::stem(Inputs[0].getFilename()).str();
+  if (!C.getDriver().isSaveTempsEnabled())
+    Prefix += "-" + SubArchName;
 
   // Each command outputs different files.
   const char *LLVMLinkCommand =
       constructLLVMLinkCommand(C, JA, Inputs, Args, SubArchName, Prefix);
   const char *OptCommand = constructOptCommand(C, JA, Inputs, Args, SubArchName,
                                                Prefix, LLVMLinkCommand);
+  if (C.getDriver().isSaveTempsEnabled())
+    constructLlcCommand(C, JA, Inputs, Args, SubArchName, Prefix, OptCommand,
+                        /*OutputIsAsm=*/true);
   const char *LlcCommand =
       constructLlcCommand(C, JA, Inputs, Args, SubArchName, Prefix, OptCommand);
   constructLldCommand(C, JA, Inputs, Output, Args, LlcCommand);
 }
 
 HIPToolChain::HIPToolChain(const Driver &D, const llvm::Triple &Triple,
                              const ToolChain &HostTC, const ArgList &Args)
     : ToolChain(D, Triple, Args), HostTC(HostTC) {

clang/test/Driver/hip-save-temps.hip

+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// -fno-gpu-rdc without -o
+// RUN: %clang -### -target x86_64-linux-gnu -nogpulib -save-temps \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK,NORDC,NOUT %s
+
+// -fno-gpu-rdc with -o
+// RUN: %clang -### -target x86_64-linux-gnu -nogpulib -save-temps \
+// RUN:   -o executable -x hip --cuda-gpu-arch=gfx900 %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK,NORDC,WOUT %s
+
+// -fgpu-rdc without -o
+// RUN: %clang -### -target x86_64-linux-gnu -nogpulib -save-temps \
+// RUN:   -fgpu-rdc -x hip --cuda-gpu-arch=gfx900 %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK,RDC,RDC-NOUT,NOUT %s
+
+// -fgpu-rdc with -o
+// RUN: %clang -### -target x86_64-linux-gnu -nogpulib -save-temps \
+// RUN:   -o executable -fgpu-rdc -x hip --cuda-gpu-arch=gfx900 %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK,RDC,RDC-WOUT,WOUT %s
+
+// CHECK: {{.*}}clang{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900.cui"
+// CHECK: {{.*}}llvm-link{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900-linked.bc"
+// CHECK: {{.*}}opt{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900-optimized.bc"
+// CHECK: {{.*}}llc{{.*}}"-filetype=asm"{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900.s"
+// CHECK: {{.*}}llc{{.*}}"-filetype=obj"{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900.o"
+// NORDC: {{.*}}lld{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900.out"
+// RDC: {{.*}}lld{{.*}}"-o" "a.out-hip-amdgcn-amd-amdhsa-gfx900"
+// NORDC: {{.*}}clang-offload-bundler{{.*}}"-outputs=hip-save-temps.hip-hip-amdgcn-amd-amdhsa.hipfb"
+// CHECK: {{.*}}clang{{.*}}"-o" "hip-save-temps-host-x86_64-unknown-linux-gnu.cui"
+// CHECK: {{.*}}clang{{.*}}"-o" "hip-save-temps-host-x86_64-unknown-linux-gnu.bc"
+// CHECK: {{.*}}clang{{.*}}"-o" "hip-save-temps-host-x86_64-unknown-linux-gnu.s"
+// CHECK: {{.*}}clang{{.*}}"-o" "hip-save-temps{{.*}}.o"
+// RDC-NOUT: {{.*}}clang-offload-bundler{{.*}}"-outputs=a.out.hipfb"
+// RDC-WOUT: {{.*}}clang-offload-bundler{{.*}}"-outputs=executable.hipfb"
+// NOUT: {{.*}}ld{{.*}}"-o" "a.out"
+// WOUT: {{.*}}ld{{.*}}"-o" "executable"
+