Skip to content

Conversation

@IgWod-IMG
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Aug 13, 2025

@llvm/pr-subscribers-mlir-spirv

Author: Igor Wodiany (IgWod-IMG)

Changes

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

3 Files Affected:

  • (modified) mlir/include/mlir/Target/SPIRV/Serialization.h (+6-6)
  • (modified) mlir/lib/Target/SPIRV/TranslateRegistration.cpp (+1-1)
  • (modified) mlir/test/Target/SPIRV/mlir-translate.mlir (+1-1)
diff --git a/mlir/include/mlir/Target/SPIRV/Serialization.h b/mlir/include/mlir/Target/SPIRV/Serialization.h
index bc58093ead864..e474101402e58 100644
--- a/mlir/include/mlir/Target/SPIRV/Serialization.h
+++ b/mlir/include/mlir/Target/SPIRV/Serialization.h
@@ -38,14 +38,14 @@ struct SerializationOptions {
   /// or an absolute path followed by the prefix. For example:
   ///
   ///   * "foo" - Create files with a `foo` prefix in the current working
-  ///     directory. For example: `fooXYZ123`, `fooABC456` ... `fooXXXXXX`.
-  ///     The last 6 characters will be a unique combination as
-  ///     generated by `llvm::sys::fs::createUniqueFile`.
+  ///     directory. For example: `fooXYZ123.spv`, `fooABC456.spv` ...
+  ///     `fooXXXXXX.spv`. The last 6 characters will be a unique combination
+  ///     as generated by `llvm::sys::fs::createUniqueFile`.
   ///
   ///   * "my/dir/foo" - Create files in `my/dir` with a `foo` prefix. The
-  ///     `my/dir` need to exists. For example: `fooXYZ123`, `fooABC456` ...
-  ///     `fooXXXXXX` will be created and stored in `/my/dir`. Filenames
-  ///     follow the same pattern as above.
+  ///     `my/dir` need to exists. For example: `fooXYZ123.spv`,
+  ///     `fooABC456.spv` ... `fooXXXXXX.spv` will be created and stored in
+  ///     `/my/dir`. Filenames follow the same pattern as above.
   ///
   ///   * "/home/user/my/dir" - Same as above but using an absolute path.
   std::string validationFilePrefix = "";
diff --git a/mlir/lib/Target/SPIRV/TranslateRegistration.cpp b/mlir/lib/Target/SPIRV/TranslateRegistration.cpp
index 4391ee714c519..796354e154c02 100644
--- a/mlir/lib/Target/SPIRV/TranslateRegistration.cpp
+++ b/mlir/lib/Target/SPIRV/TranslateRegistration.cpp
@@ -106,7 +106,7 @@ serializeModule(spirv::ModuleOp moduleOp, raw_ostream &output,
     int fd = 0;
 
     std::error_code errorCode = llvm::sys::fs::createUniqueFile(
-        options.validationFilePrefix + "%%%%%%", fd, filename);
+        options.validationFilePrefix + "%%%%%%.spv", fd, filename);
     if (errorCode)
       return moduleOp.emitError("error creating validation output file: ")
              << errorCode.message() << "\n";
diff --git a/mlir/test/Target/SPIRV/mlir-translate.mlir b/mlir/test/Target/SPIRV/mlir-translate.mlir
index 9f91fc97dcaed..cbce351dd35b5 100644
--- a/mlir/test/Target/SPIRV/mlir-translate.mlir
+++ b/mlir/test/Target/SPIRV/mlir-translate.mlir
@@ -5,7 +5,7 @@
 // RUN: rm -rf %t
 // RUN: mkdir %t && mlir-translate --serialize-spirv --no-implicit-module \
 // RUN: --split-input-file --spirv-save-validation-files-with-prefix=%t/foo %s \
-// RUN: && ls %t | wc -l | FileCheck %s
+// RUN: && ls %t/foo*.spv | wc -l | FileCheck %s
 // RUN: rm -rf %t
 
 // CHECK: 4

@llvmbot
Copy link
Member

llvmbot commented Aug 13, 2025

@llvm/pr-subscribers-mlir

Author: Igor Wodiany (IgWod-IMG)

Changes

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

3 Files Affected:

  • (modified) mlir/include/mlir/Target/SPIRV/Serialization.h (+6-6)
  • (modified) mlir/lib/Target/SPIRV/TranslateRegistration.cpp (+1-1)
  • (modified) mlir/test/Target/SPIRV/mlir-translate.mlir (+1-1)
diff --git a/mlir/include/mlir/Target/SPIRV/Serialization.h b/mlir/include/mlir/Target/SPIRV/Serialization.h
index bc58093ead864..e474101402e58 100644
--- a/mlir/include/mlir/Target/SPIRV/Serialization.h
+++ b/mlir/include/mlir/Target/SPIRV/Serialization.h
@@ -38,14 +38,14 @@ struct SerializationOptions {
   /// or an absolute path followed by the prefix. For example:
   ///
   ///   * "foo" - Create files with a `foo` prefix in the current working
-  ///     directory. For example: `fooXYZ123`, `fooABC456` ... `fooXXXXXX`.
-  ///     The last 6 characters will be a unique combination as
-  ///     generated by `llvm::sys::fs::createUniqueFile`.
+  ///     directory. For example: `fooXYZ123.spv`, `fooABC456.spv` ...
+  ///     `fooXXXXXX.spv`. The last 6 characters will be a unique combination
+  ///     as generated by `llvm::sys::fs::createUniqueFile`.
   ///
   ///   * "my/dir/foo" - Create files in `my/dir` with a `foo` prefix. The
-  ///     `my/dir` need to exists. For example: `fooXYZ123`, `fooABC456` ...
-  ///     `fooXXXXXX` will be created and stored in `/my/dir`. Filenames
-  ///     follow the same pattern as above.
+  ///     `my/dir` need to exists. For example: `fooXYZ123.spv`,
+  ///     `fooABC456.spv` ... `fooXXXXXX.spv` will be created and stored in
+  ///     `/my/dir`. Filenames follow the same pattern as above.
   ///
   ///   * "/home/user/my/dir" - Same as above but using an absolute path.
   std::string validationFilePrefix = "";
diff --git a/mlir/lib/Target/SPIRV/TranslateRegistration.cpp b/mlir/lib/Target/SPIRV/TranslateRegistration.cpp
index 4391ee714c519..796354e154c02 100644
--- a/mlir/lib/Target/SPIRV/TranslateRegistration.cpp
+++ b/mlir/lib/Target/SPIRV/TranslateRegistration.cpp
@@ -106,7 +106,7 @@ serializeModule(spirv::ModuleOp moduleOp, raw_ostream &output,
     int fd = 0;
 
     std::error_code errorCode = llvm::sys::fs::createUniqueFile(
-        options.validationFilePrefix + "%%%%%%", fd, filename);
+        options.validationFilePrefix + "%%%%%%.spv", fd, filename);
     if (errorCode)
       return moduleOp.emitError("error creating validation output file: ")
              << errorCode.message() << "\n";
diff --git a/mlir/test/Target/SPIRV/mlir-translate.mlir b/mlir/test/Target/SPIRV/mlir-translate.mlir
index 9f91fc97dcaed..cbce351dd35b5 100644
--- a/mlir/test/Target/SPIRV/mlir-translate.mlir
+++ b/mlir/test/Target/SPIRV/mlir-translate.mlir
@@ -5,7 +5,7 @@
 // RUN: rm -rf %t
 // RUN: mkdir %t && mlir-translate --serialize-spirv --no-implicit-module \
 // RUN: --split-input-file --spirv-save-validation-files-with-prefix=%t/foo %s \
-// RUN: && ls %t | wc -l | FileCheck %s
+// RUN: && ls %t/foo*.spv | wc -l | FileCheck %s
 // RUN: rm -rf %t
 
 // CHECK: 4

@IgWod-IMG IgWod-IMG enabled auto-merge (squash) August 13, 2025 16:12
@IgWod-IMG IgWod-IMG merged commit d4045a4 into llvm:main Aug 13, 2025
13 checks passed
@IgWod-IMG
Copy link
Contributor Author

It looks like that auto-merge ignores Test MLIR SPIR-V and do the merge once other CI checks pass. We probably should set it as required or something.

@IgWod-IMG IgWod-IMG deleted the img_val-files-extension branch August 13, 2025 16:19
@kuhar
Copy link
Member

kuhar commented Aug 13, 2025

It looks like that auto-merge ignores Test MLIR SPIR-V and do the merge once other CI checks pass. We probably should set it as required or something.

IMO that's fine since we don't have that much mileage on this check yet... worst case we can always revert or fix post-commit

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.

3 participants