From bc2350a3412676977ec2ffc801e661aa0cec9de6 Mon Sep 17 00:00:00 2001 From: Yaxun Liu Date: Wed, 9 Oct 2019 18:46:43 +0000 Subject: [PATCH] [HIP] Fix -save-temps 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. Differential Revision: https://reviews.llvm.org/D68665 llvm-svn: 374198 --- clang/lib/Driver/Driver.cpp | 11 +++++ clang/lib/Driver/ToolChains/CommonArgs.cpp | 26 ++++++------ clang/lib/Driver/ToolChains/HIP.cpp | 49 ++++++++++++++-------- clang/lib/Driver/ToolChains/HIP.h | 3 +- clang/test/Driver/hip-save-temps.hip | 41 ++++++++++++++++++ 5 files changed, 98 insertions(+), 32 deletions(-) create mode 100644 clang/test/Driver/hip-save-temps.hip diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index df3e82b85e5c..a09527429827 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4419,11 +4419,22 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA, 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()) { diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 18b182424095..47964099bd9c 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1387,14 +1387,12 @@ void tools::AddHIPLinkerScript(const ToolChain &TC, Compilation &C, // 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. @@ -1412,11 +1410,13 @@ void tools::AddHIPLinkerScript(const ToolChain &TC, Compilation &C, "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 @@ -1428,14 +1428,14 @@ void tools::AddHIPLinkerScript(const ToolChain &TC, Compilation &C, 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"; diff --git a/clang/lib/Driver/ToolChains/HIP.cpp b/clang/lib/Driver/ToolChains/HIP.cpp index f3e58cbeb4d7..ad9384df6a24 100644 --- a/clang/lib/Driver/ToolChains/HIP.cpp +++ b/clang/lib/Driver/ToolChains/HIP.cpp @@ -48,6 +48,20 @@ static void addBCLib(const Driver &D, const ArgList &Args, 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( @@ -61,10 +75,7 @@ const char *AMDGCN::Linker::constructLLVMLinkCommand( // 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"); @@ -109,10 +120,8 @@ const char *AMDGCN::Linker::constructOptCommand( } 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"); @@ -124,11 +133,13 @@ const char *AMDGCN::Linker::constructOptCommand( 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")), + Args.MakeArgString("-mcpu=" + SubArchName)}; // Extract all the -m options std::vector Features; @@ -151,10 +162,8 @@ const char *AMDGCN::Linker::constructLlcCommand( // 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"); @@ -230,14 +239,18 @@ void AMDGCN::Linker::ConstructJob(Compilation &C, const JobAction &JA, 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); diff --git a/clang/lib/Driver/ToolChains/HIP.h b/clang/lib/Driver/ToolChains/HIP.h index a650095d054d..2d146ce5cc6f 100644 --- a/clang/lib/Driver/ToolChains/HIP.h +++ b/clang/lib/Driver/ToolChains/HIP.h @@ -58,7 +58,8 @@ private: 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, diff --git a/clang/test/Driver/hip-save-temps.hip b/clang/test/Driver/hip-save-temps.hip new file mode 100644 index 000000000000..f7f2c6ef57ff --- /dev/null +++ b/clang/test/Driver/hip-save-temps.hip @@ -0,0 +1,41 @@ +// 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" +