[MLIR][LLVM] Fix recursive DI type export memory leak

[zyx-billy]

Followup to discussion #87295 (comment).

The export cache should not cache temporary nodes.

[Dinistro]

Can we ensure that get's landed soonish? While I understand the concern for about how to test this, it is probably better to land this and ship a test as a followup, given that this breaks production flows.

[gysit]

LGTM!

I think it makes sense to land this soonish since it fixes an upstream bug.

[zyx-billy]

Sounds good. Let me fix the failures and land this then.

[zyx-billy]

OK turns out the test isn't too hard. Just had to make the previous nested recursive-decl test also use the nested decl again outside, which will die if it was cached to a temporary.

[llvmbot]

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: Billy Zhu (zyx-billy)

Changes

Followup to discussion #87295 (comment).

The export cache should not cache temporary nodes.

---

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

2 Files Affected:

• (modified) mlir/lib/Target/LLVMIR/DebugTranslation.cpp (+2-1)
• (modified) mlir/test/Target/LLVMIR/llvmir-debug.mlir (+6-2)

diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
index 46e2e7f2ba5dc4..2de5e372d88c0e 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -352,7 +352,8 @@ llvm::DINode *DebugTranslation::translate(DINodeAttr attr) {
                      DISubroutineTypeAttr>(
                    [&](auto attr) { return translateImpl(attr); });
 
-  attrToNode.insert({attr, node});
+  if (node && !node->isTemporary())
+    attrToNode.insert({attr, node});
   return node;
 }
 
diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index f4c18bf6bd53c0..8ab1a1b290dad3 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -442,12 +442,16 @@ llvm.mlir.global @global_variable() {dbg_expr = #di_global_variable_expression}
 #di_subprogram = #llvm.di_subprogram<scope = #di_file, file = #di_file, subprogramFlags = Optimized, type = #di_subroutine_type>
 #di_composite_type = #llvm.di_composite_type<tag = DW_TAG_class_type, recId = distinct[0]<>, scope = #di_subprogram>
 
-#di_global_variable = #llvm.di_global_variable<file = #di_file, line = 1, type = #di_composite_type>
+// Use the inner type standalone outside too. Ensures it's not cached wrong.
+#di_var_type = #llvm.di_subroutine_type<types = #di_composite_type, #di_composite_type_inner>
+#di_global_variable = #llvm.di_global_variable<file = #di_file, line = 1, type = #di_var_type>
 #di_global_variable_expression = #llvm.di_global_variable_expression<var = #di_global_variable>
 
 llvm.mlir.global @global_variable() {dbg_expr = #di_global_variable_expression} : !llvm.struct<()>
 
-// CHECK: distinct !DIGlobalVariable({{.*}}type: ![[COMP:[0-9]+]],
+// CHECK: distinct !DIGlobalVariable({{.*}}type: ![[VAR:[0-9]+]],
+// CHECK: ![[VAR]] = !DISubroutineType(types: ![[COMPS:[0-9]+]])
+// CHECK: ![[COMPS]] = !{![[COMP:[0-9]+]],
 // CHECK: ![[COMP]] = distinct !DICompositeType({{.*}}scope: ![[SCOPE:[0-9]+]],
 // CHECK: ![[SCOPE]] = !DISubprogram({{.*}}type: ![[SUBROUTINE:[0-9]+]],
 // CHECK: ![[SUBROUTINE]] = !DISubroutineType(types: ![[SR_TYPES:[0-9]+]])

[gysit]

Nice thanks!