[clang] 1f8a3ce - [HIP] Fix temporary files

Yaxun Liu via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 9 18:42:43 PST 2023


Author: Yaxun (Sam) Liu
Date: 2023-03-09T21:41:58-05:00
New Revision: 1f8a3ce325be51a1004657b08a607825447fee1b

URL: https://github.com/llvm/llvm-project/commit/1f8a3ce325be51a1004657b08a607825447fee1b
DIFF: https://github.com/llvm/llvm-project/commit/1f8a3ce325be51a1004657b08a607825447fee1b.diff

LOG: [HIP] Fix temporary files

Currently HIP toolchain uses Driver::GetTemporaryDirectory to
create a temporary directory for some temporary files during
compilation. The temporary directories are not automatically
deleted after compilation. This slows down compilation
on Windows.

Switch to use GetTemporaryPath which only creates temporay
files which will be deleted automatically.

Keep the original input file name convention for Darwin host
toolchain since it is needed for deterministic binary
(https://reviews.llvm.org/D111269)

Fixes: SWDEV-386058

Reviewed by: Artem Belevich

Differential Revision: https://reviews.llvm.org/D145509

Added: 
    clang/test/Driver/hip-temps-linux.hip
    clang/test/Driver/hip-temps-windows.hip

Modified: 
    clang/include/clang/Driver/Driver.h
    clang/lib/Driver/Driver.cpp
    clang/test/Driver/hip-link-bc-to-bc.hip

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h
index c9136ec4ae690..691deb7a90eeb 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -627,10 +627,19 @@ class Driver {
   /// Returns the default name for linked images (e.g., "a.out").
   const char *getDefaultImageName() const;
 
-  // Creates a temp file with $Prefix-%%%%%%.$Suffix
+  /// Creates a temp file.
+  /// 1. If \p MultipleArch is false or \p BoundArch is empty, the temp file is
+  ///    in the temporary directory with name $Prefix-%%%%%%.$Suffix.
+  /// 2. If \p MultipleArch is true and \p BoundArch is not empty,
+  ///    2a. If \p NeedUniqueDirectory is false, the temp file is in the
+  ///        temporary directory with name $Prefix-$BoundArch-%%%%%.$Suffix.
+  ///    2b. If \p NeedUniqueDirectory is true, the temp file is in a unique
+  ///        subdiretory with random name under the temporary directory, and
+  ///        the temp file itself has name $Prefix-$BoundArch.$Suffix.
   const char *CreateTempFile(Compilation &C, StringRef Prefix, StringRef Suffix,
                              bool MultipleArchs = false,
-                             StringRef BoundArch = {}) const;
+                             StringRef BoundArch = {},
+                             bool NeedUniqueDirectory = false) const;
 
   /// GetNamedOutputPath - Return the name to use for the output of
   /// the action \p JA. The result is appended to the compilation's

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 0567441225d0c..0f5f35c14a7d0 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -5545,7 +5545,8 @@ static bool HasPreprocessOutput(const Action &JA) {
 
 const char *Driver::CreateTempFile(Compilation &C, StringRef Prefix,
                                    StringRef Suffix, bool MultipleArchs,
-                                   StringRef BoundArch) const {
+                                   StringRef BoundArch,
+                                   bool NeedUniqueDirectory) const {
   SmallString<128> TmpName;
   Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
   std::optional<std::string> CrashDirectory =
@@ -5565,9 +5566,15 @@ const char *Driver::CreateTempFile(Compilation &C, StringRef Prefix,
     }
   } else {
     if (MultipleArchs && !BoundArch.empty()) {
-      TmpName = GetTemporaryDirectory(Prefix);
-      llvm::sys::path::append(TmpName,
-                              Twine(Prefix) + "-" + BoundArch + "." + Suffix);
+      if (NeedUniqueDirectory) {
+        TmpName = GetTemporaryDirectory(Prefix);
+        llvm::sys::path::append(TmpName,
+                                Twine(Prefix) + "-" + BoundArch + "." + Suffix);
+      } else {
+        TmpName =
+            GetTemporaryPath((Twine(Prefix) + "-" + BoundArch).str(), Suffix);
+      }
+
     } else {
       TmpName = GetTemporaryPath(Prefix, Suffix);
     }
@@ -5683,7 +5690,16 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
     StringRef Name = llvm::sys::path::filename(BaseInput);
     std::pair<StringRef, StringRef> Split = Name.split('.');
     const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
-    return CreateTempFile(C, Split.first, Suffix, MultipleArchs, BoundArch);
+    // The non-offloading toolchain on Darwin requires deterministic input
+    // file name for binaries to be deterministic, therefore it needs unique
+    // directory.
+    llvm::Triple Triple(C.getDriver().getTargetTriple());
+    bool NeedUniqueDirectory =
+        (JA.getOffloadingDeviceKind() == Action::OFK_None ||
+         JA.getOffloadingDeviceKind() == Action::OFK_Host) &&
+        Triple.isOSDarwin();
+    return CreateTempFile(C, Split.first, Suffix, MultipleArchs, BoundArch,
+                          NeedUniqueDirectory);
   }
 
   SmallString<128> BasePath(BaseInput);

diff  --git a/clang/test/Driver/hip-link-bc-to-bc.hip b/clang/test/Driver/hip-link-bc-to-bc.hip
index 1cfcdad4e4978..6b31456657fff 100644
--- a/clang/test/Driver/hip-link-bc-to-bc.hip
+++ b/clang/test/Driver/hip-link-bc-to-bc.hip
@@ -11,10 +11,10 @@
 // RUN:   2>&1 | FileCheck -check-prefix=BITCODE %s
 
 // BITCODE: "{{.*}}clang-offload-bundler" "-type=bc" "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx906" "-input={{.*}}bundle1.bc" "-output=[[B1HOST:.*\.bc]]" "-output=[[B1DEV1:.*\.bc]]" "-unbundle" "-allow-missing-bundles"
-// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B1DEV2:.*bundle1-gfx906.bc]]" "-x" "ir" "[[B1DEV1]]"
+// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B1DEV2:.*bundle1-gfx906-.*\.bc]]" "-x" "ir" "[[B1DEV1]]"
 
 // BITCODE: "{{.*}}clang-offload-bundler" "-type=bc" "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx906" "-input={{.*}}bundle2.bc" "-output=[[B2HOST:.*\.bc]]" "-output=[[B2DEV1:.*\.bc]]" "-unbundle" "-allow-missing-bundles"
-// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B2DEV2:.*bundle2-gfx906.bc]]" "-x" "ir" "[[B2DEV1]]"
+// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B2DEV2:.*bundle2-gfx906-.*\.bc]]" "-x" "ir" "[[B2DEV1]]"
 
 // BITCODE: "{{.*}}llvm-link" "-o" "bundle1-hip-amdgcn-amd-amdhsa-gfx906.bc" "[[B1DEV2]]" "[[B2DEV2]]"
 

diff  --git a/clang/test/Driver/hip-temps-linux.hip b/clang/test/Driver/hip-temps-linux.hip
new file mode 100644
index 0000000000000..83a7528dd4560
--- /dev/null
+++ b/clang/test/Driver/hip-temps-linux.hip
@@ -0,0 +1,18 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: system-linux
+
+// Check no temporary files or directores are left after compilation.
+// RUN: rm -rf %t/mytmp
+// RUN: mkdir -p %t/mytmp
+// RUN: env TMPDIR="%t/mytmp" %clang --target=x86_64-linux-gnu -nogpulib -nogpuinc \
+// RUN:   --rocm-path=%S/Inputs/rocm -nostdinc -nostdlib -c \
+// RUN:   --offload-arch=gfx1030 -emit-llvm -v %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK %s
+// RUN: ls %t/mytmp >%t/mytmp.txt 2>&1
+// RUN: touch %t/empty.txt
+// RUN: 
diff  %t/mytmp.txt %t/empty.txt
+
+// CHECK: -o {{.*}}/mytmp/hip-temps-linux-gfx1030-{{.*}}.bc
+
+int main() {}

diff  --git a/clang/test/Driver/hip-temps-windows.hip b/clang/test/Driver/hip-temps-windows.hip
new file mode 100644
index 0000000000000..ac1ac208a65dc
--- /dev/null
+++ b/clang/test/Driver/hip-temps-windows.hip
@@ -0,0 +1,18 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: system-windows
+
+// Check no temporary files or directores are left after compilation.
+// RUN: rm -rf %t/mytmp
+// RUN: mkdir -p %t/mytmp
+// RUN: env TMP="%t/mytmp" %clang --target=x86_64-pc-windows-msvc -nogpulib -nogpuinc \
+// RUN:   --rocm-path=%S/Inputs/rocm -nostdinc -nostdlib -c \
+// RUN:   --offload-arch=gfx1030 -emit-llvm -v %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK %s
+// RUN: ls %t/mytmp >%t/mytmp.txt 2>&1
+// RUN: touch %t/empty.txt
+// RUN: 
diff  %t/mytmp.txt %t/empty.txt
+
+// CHECK: -o "{{.*}}mytmp{{/|\\\\}}hip-temps-windows-gfx1030-{{.*}}.bc"
+
+int main() {}


        


More information about the cfe-commits mailing list