[mlir] Fix --mlir-print-ir-tree-dir when the symbol name contains /

[joker-eph]

Fixes #137622

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Mehdi Amini (joker-eph)

Changes

Fixes #137622

---

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

2 Files Affected:

• (modified) mlir/lib/Pass/IRPrinting.cpp (+9-5)
• (modified) mlir/test/Pass/ir-printing-file-tree.mlir (+2-2)

diff --git a/mlir/lib/Pass/IRPrinting.cpp b/mlir/lib/Pass/IRPrinting.cpp
index 99e0601e4749c..256247f7f688b 100644
--- a/mlir/lib/Pass/IRPrinting.cpp
+++ b/mlir/lib/Pass/IRPrinting.cpp
@@ -10,6 +10,7 @@
 #include "mlir/IR/SymbolTable.h"
 #include "mlir/Pass/PassManager.h"
 #include "mlir/Support/FileUtilities.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -210,22 +211,25 @@ struct BasicIRPrinterConfig : public PassManager::IRPrinterConfig {
 /// `op` first, `op` last). This information is used to construct the directory
 /// tree for the `FileTreeIRPrinterConfig` below.
 /// The counter for `op` will be incremented by this call.
-static std::pair<SmallVector<std::pair<std::string, StringRef>>, std::string>
+static std::pair<SmallVector<std::pair<std::string, std::string>>, std::string>
 getOpAndSymbolNames(Operation *op, StringRef passName,
                     llvm::DenseMap<Operation *, unsigned> &counters) {
-  SmallVector<std::pair<std::string, StringRef>> pathElements;
+  SmallVector<std::pair<std::string, std::string>> pathElements;
   SmallVector<unsigned> countPrefix;
 
   Operation *iter = op;
   ++counters.try_emplace(op, -1).first->second;
   while (iter) {
     countPrefix.push_back(counters[iter]);
-    StringAttr symbolName =
+    StringAttr symbolNameAttr =
         iter->getAttrOfType<StringAttr>(SymbolTable::getSymbolAttrName());
+    std::string symbolName =
+        symbolNameAttr ? symbolNameAttr.str() : "no-symbol-name";
+    llvm::replace(symbolName, '/', '_');
+
     std::string opName =
         llvm::join(llvm::split(iter->getName().getStringRef().str(), '.'), "_");
-    pathElements.emplace_back(opName, symbolName ? symbolName.strref()
-                                                 : "no-symbol-name");
+    pathElements.emplace_back(std::move(opName), std::move(symbolName));
     iter = iter->getParentOp();
   }
   // Return in the order of top level (module) down to `op`.
diff --git a/mlir/test/Pass/ir-printing-file-tree.mlir b/mlir/test/Pass/ir-printing-file-tree.mlir
index b00d77db2c603..f8a1296aad1bd 100644
--- a/mlir/test/Pass/ir-printing-file-tree.mlir
+++ b/mlir/test/Pass/ir-printing-file-tree.mlir
@@ -15,7 +15,7 @@
 // RUN:   -mlir-print-ir-after-all
 // RUN: test -f %t/builtin_module_outer/0_canonicalize.mlir
 // RUN: test -f %t/builtin_module_outer/1_canonicalize.mlir
-// RUN: test -f %t/builtin_module_outer/func_func_symA/1_0_cse.mlir
+// RUN: test -f %t/builtin_module_outer/func_func_sym_A/1_0_cse.mlir
 // RUN: test -f %t/builtin_module_outer/builtin_module_inner/1_0_canonicalize.mlir
 // RUN: test -f %t/builtin_module_outer/builtin_module_inner/func_func_symB/1_0_0_cse.mlir
 // RUN: test -f %t/builtin_module_outer/builtin_module_inner/func_func_symB/1_0_1_canonicalize.mlir
@@ -26,7 +26,7 @@
 
 builtin.module @outer {
 
-  func.func @symA() {
+  func.func @"sym/A"() {
     return
   }
 

[christopherbate]

LGTM. I was wondering if there was a standard LLVM "sanitization" function for path names. Especially on Windows there could be a lot of edge cases for strings that are allowed symbol names but are not valid Windows path names.

[joker-eph]

The test fails on Windows as expected:

| Error while creating directory C:\ws\src\build\tools\mlir\test\Pass\Output\ir-printing-file-tree.mlir.tmp\builtin_module_outer\func_func_sym\backslash: no such file or directory

I pushed the fix now, should fix the CI.

I was wondering if there was a standard LLVM "sanitization" function for path names. Especially on Windows there could be a lot of edge cases for strings that are allowed symbol names but are not valid Windows path names.

Good point: I searched in llvm/include/llvm/Support/FileSystem.h (and similar files) but couldn't find anything right now.