[Mlir-commits] [mlir] [mlir][ROCDL] Fix file leak in SeralizeToHsaco and its newer form (PR #67711)

[Krzysztof Drewniak]

https://github.com/krzysz00 created https://github.com/llvm/llvm-project/pull/67711

SerializetToHsaco, as currently implemented, leaks the file descriptor of the .hsaco temporary file, which causes issues in long-running parallel compilation setups.

See also https://github.com/ROCmSoftwarePlatform/rocMLIR/pull/1257

>From e41bdaeb1b6e0ffda6bffd7147144bbafba22597 Mon Sep 17 00:00:00 2001
From: Krzysztof Drewniak <Krzysztof.Drewniak at amd.com>
Date: Thu, 28 Sep 2023 08:29:06 -0500
Subject: [PATCH] [mlir][ROCDL] Fix file leak in SeralizeToHsaco and its newer
 form

SerializetToHsaco, as currently implemented, leaks the file descriptor
of the .hsaco temporary file, which causes issues in long-running
parallel compilation setups.

See also https://github.com/ROCmSoftwarePlatform/rocMLIR/pull/1257
---
 mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp | 9 ++++++---
 mlir/lib/Target/LLVM/ROCDL/Target.cpp                | 6 ++++--
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp b/mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
index 9db0a942f7c38fd..dcf3bf016eee438 100644
--- a/mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
+++ b/mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
@@ -12,7 +12,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Dialect/GPU/Transforms/Passes.h"
-#include "mlir/Dialect/LLVMIR/ROCDLDialect.h"
 #include "mlir/IR/Location.h"
 #include "mlir/IR/MLIRContext.h"
 
@@ -43,6 +42,7 @@
 #include "llvm/MC/TargetRegistry.h"
 
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
@@ -410,6 +410,8 @@ SerializeToHsacoPass::createHsaco(const SmallVectorImpl<char> &isaBinary) {
     return {};
   }
   llvm::FileRemover cleanupHsaco(tempHsacoFilename);
+  // Prevent file descriptor leak.
+  llvm::sys::fs::closeFile(tempHsacoFD);
 
   std::string theRocmPath = getRocmPath();
   llvm::SmallString<32> lldPath(theRocmPath);
@@ -423,13 +425,14 @@ SerializeToHsacoPass::createHsaco(const SmallVectorImpl<char> &isaBinary) {
   }
 
   // Load the HSA code object.
-  auto hsacoFile = openInputFile(tempHsacoFilename);
+  auto hsacoFile =
+      llvm::MemoryBuffer::getFile(tempHsacoFilename, /*IsText=*/false);
   if (!hsacoFile) {
     emitError(loc, "read HSA code object from temp file error");
     return {};
   }
 
-  StringRef buffer = hsacoFile->getBuffer();
+  StringRef buffer = (*hsacoFile)->getBuffer();
   return std::make_unique<std::vector<char>>(buffer.begin(), buffer.end());
 }
 
diff --git a/mlir/lib/Target/LLVM/ROCDL/Target.cpp b/mlir/lib/Target/LLVM/ROCDL/Target.cpp
index 611d08fe3e79e56..c6f2ee1e4722df5 100644
--- a/mlir/lib/Target/LLVM/ROCDL/Target.cpp
+++ b/mlir/lib/Target/LLVM/ROCDL/Target.cpp
@@ -386,6 +386,7 @@ AMDGPUSerializer::compileToBinary(const std::string &serializedISA) {
     return std::nullopt;
   }
   llvm::FileRemover cleanupHsaco(tempHsacoFilename);
+  llvm::sys::fs::closeFile(tempHsacoFD);
 
   llvm::SmallString<128> lldPath(toolkitPath);
   llvm::sys::path::append(lldPath, "llvm", "bin", "ld.lld");
@@ -398,14 +399,15 @@ AMDGPUSerializer::compileToBinary(const std::string &serializedISA) {
   }
 
   // Load the HSA code object.
-  auto hsacoFile = openInputFile(tempHsacoFilename);
+  auto hsacoFile =
+      llvm::MemoryBuffer::getFile(tempHsacoFilename, /*IsText=*/false);
   if (!hsacoFile) {
     getOperation().emitError()
         << "Failed to read the HSA code object from the temp file.";
     return std::nullopt;
   }
 
-  StringRef buffer = hsacoFile->getBuffer();
+  StringRef buffer = (*hsacoFile)->getBuffer();
 
   return SmallVector<char, 0>(buffer.begin(), buffer.end());
 }