Skip to content

Conversation

wsmoses
Copy link
Member

@wsmoses wsmoses commented Aug 23, 2025

Linux has a limitation of 256 characters for a path. Large function names being serialized will cause this to fail. As createTemporaryFile already unique's the file (up to 128 retries for different name variations), truncating should suffice

@llvmbot
Copy link
Member

llvmbot commented Aug 23, 2025

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: William Moses (wsmoses)

Changes

Linux has a limitation of 256 characters for a path. Large function names being serialized will cause this to fail. As createTemporaryFile already unique's the file (up to 128 retries for different name variations), truncating should suffice


Full diff: https://github.com/llvm/llvm-project/pull/155108.diff

1 Files Affected:

  • (modified) mlir/lib/Target/LLVM/NVVM/Target.cpp (+2)
diff --git a/mlir/lib/Target/LLVM/NVVM/Target.cpp b/mlir/lib/Target/LLVM/NVVM/Target.cpp
index 62eaadb3d16b1..8760ea8588e2c 100644
--- a/mlir/lib/Target/LLVM/NVVM/Target.cpp
+++ b/mlir/lib/Target/LLVM/NVVM/Target.cpp
@@ -267,6 +267,8 @@ NVPTXSerializer::NVPTXSerializer(Operation &module, NVVMTargetAttr target,
 std::optional<NVPTXSerializer::TmpFile>
 NVPTXSerializer::createTemp(StringRef name, StringRef suffix) {
   llvm::SmallString<128> filename;
+  if (name.size() > 80)
+    name = name.substr(0, 80);
   std::error_code ec =
       llvm::sys::fs::createTemporaryFile(name, suffix, filename);
   if (ec) {

@ftynse
Copy link
Member

ftynse commented Aug 23, 2025

Maybe hoist to a named constant instead of a magic number

@wsmoses wsmoses enabled auto-merge (squash) August 23, 2025 16:24
@wsmoses wsmoses merged commit b12e033 into main Aug 23, 2025
12 checks passed
@wsmoses wsmoses deleted the users/wsmoses/gpath branch August 23, 2025 16:29
@joker-eph
Copy link
Collaborator

Maybe hoist to a named constant instead of a magic number

Either that or a comment (the named constant may make this self-documenting, but otherwise a comment explaining why we're doing this seems useful here)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants