[Mlir-commits] [mlir] [MLIR][LLVM] DI Recursive Type fix for recursion via scope of composites (PR #85850)
Billy Zhu
llvmlistbot at llvm.org
Tue Mar 19 13:27:10 PDT 2024
https://github.com/zyx-billy updated https://github.com/llvm/llvm-project/pull/85850
>From b6277c24a60000d14354448892ed2f949e8e2629 Mon Sep 17 00:00:00 2001
From: Billy Zhu <billyzhu at modular.com>
Date: Tue, 19 Mar 2024 11:58:32 -0700
Subject: [PATCH 1/3] create temporary instead
---
mlir/lib/Target/LLVMIR/DebugTranslation.cpp | 50 +++++++++++----------
mlir/lib/Target/LLVMIR/DebugTranslation.h | 24 ++++------
mlir/test/Target/LLVMIR/llvmir-debug.mlir | 21 +++++++++
3 files changed, 55 insertions(+), 40 deletions(-)
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
index 1248de3db07048..126a671a58e808 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -116,8 +116,20 @@ static DINodeT *getDistinctOrUnique(bool isDistinct, Ts &&...args) {
return DINodeT::get(std::forward<Ts>(args)...);
}
+llvm::TempDICompositeType
+DebugTranslation::translateTemporaryImpl(DICompositeTypeAttr attr) {
+ return llvm::DICompositeType::getTemporary(
+ llvmCtx, attr.getTag(), getMDStringOrNull(attr.getName()), nullptr,
+ attr.getLine(), nullptr, nullptr, attr.getSizeInBits(),
+ attr.getAlignInBits(),
+ /*OffsetInBits=*/0,
+ /*Flags=*/static_cast<llvm::DINode::DIFlags>(attr.getFlags()),
+ /*Elements=*/nullptr, /*RuntimeLang=*/0,
+ /*VTableHolder=*/nullptr);
+}
+
llvm::DICompositeType *
-DebugTranslation::translateImplGetPlaceholder(DICompositeTypeAttr attr) {
+DebugTranslation::translateImpl(DICompositeTypeAttr attr) {
// TODO: Use distinct attributes to model this, once they have landed.
// Depending on the tag, composite types must be distinct.
bool isDistinct = false;
@@ -129,8 +141,10 @@ DebugTranslation::translateImplGetPlaceholder(DICompositeTypeAttr attr) {
isDistinct = true;
}
- llvm::TempMDTuple placeholderElements =
- llvm::MDNode::getTemporary(llvmCtx, std::nullopt);
+ SmallVector<llvm::Metadata *> elements;
+ for (DINodeAttr member : attr.getElements())
+ elements.push_back(translate(member));
+
return getDistinctOrUnique<llvm::DICompositeType>(
isDistinct, llvmCtx, attr.getTag(), getMDStringOrNull(attr.getName()),
translate(attr.getFile()), attr.getLine(), translate(attr.getScope()),
@@ -138,23 +152,8 @@ DebugTranslation::translateImplGetPlaceholder(DICompositeTypeAttr attr) {
attr.getAlignInBits(),
/*OffsetInBits=*/0,
/*Flags=*/static_cast<llvm::DINode::DIFlags>(attr.getFlags()),
- /*Elements=*/placeholderElements.get(), /*RuntimeLang=*/0,
- /*VTableHolder=*/nullptr);
-}
-
-void DebugTranslation::translateImplFillPlaceholder(
- DICompositeTypeAttr attr, llvm::DICompositeType *placeholder) {
- SmallVector<llvm::Metadata *> elements;
- for (DINodeAttr member : attr.getElements())
- elements.push_back(translate(member));
- placeholder->replaceElements(llvm::MDNode::get(llvmCtx, elements));
-}
-
-llvm::DICompositeType *
-DebugTranslation::translateImpl(DICompositeTypeAttr attr) {
- llvm::DICompositeType *placeholder = translateImplGetPlaceholder(attr);
- translateImplFillPlaceholder(attr, placeholder);
- return placeholder;
+ llvm::MDNode::get(llvmCtx, elements),
+ /*RuntimeLang=*/0, /*VTableHolder=*/nullptr);
}
llvm::DIDerivedType *DebugTranslation::translateImpl(DIDerivedTypeAttr attr) {
@@ -234,10 +233,13 @@ DebugTranslation::translateRecursive(DIRecursiveTypeAttrInterface attr) {
llvm::DIType *result =
TypeSwitch<DIRecursiveTypeAttrInterface, llvm::DIType *>(attr)
.Case<DICompositeTypeAttr>([&](auto attr) {
- auto *placeholder = translateImplGetPlaceholder(attr);
- setRecursivePlaceholder(placeholder);
- translateImplFillPlaceholder(attr, placeholder);
- return placeholder;
+ auto temporary = translateTemporaryImpl(attr);
+ setRecursivePlaceholder(temporary.get());
+ // Must call `translateImpl` directly instead of `translate` to
+ // avoid handling the recursive interface again.
+ auto *concrete = translateImpl(attr);
+ temporary->replaceAllUsesWith(concrete);
+ return concrete;
});
assert(recursiveTypeMap.back().first == recursiveId &&
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.h b/mlir/lib/Target/LLVMIR/DebugTranslation.h
index d2171fed8c594f..c7a5228cbf5e92 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.h
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.h
@@ -88,23 +88,16 @@ class DebugTranslation {
llvm::DISubroutineType *translateImpl(DISubroutineTypeAttr attr);
llvm::DIType *translateImpl(DITypeAttr attr);
- /// Attributes that support self recursion need to implement two methods and
- /// hook into the `translateImpl` overload for `DIRecursiveTypeAttr`.
- /// - `<llvm type> translateImplGetPlaceholder(<mlir type>)`:
- /// Translate the DI attr without translating any potentially recursive
- /// nested DI attrs.
- /// - `void translateImplFillPlaceholder(<mlir type>, <llvm type>)`:
- /// Given the placeholder returned by `translateImplGetPlaceholder`, fill
- /// any holes by recursively translating nested DI attrs. This method must
- /// mutate the placeholder that is passed in, instead of creating a new one.
+ /// Attributes that support self recursion need to implement an additional
+ /// method to hook into `translateRecursive`.
+ /// - `<temp llvm type> translateTemporaryImpl(<mlir type>)`:
+ /// Create a temporary translation of the DI attr without recursively
+ /// translating any nested DI attrs.
llvm::DIType *translateRecursive(DIRecursiveTypeAttrInterface attr);
- /// Get a placeholder DICompositeType without recursing into the elements.
- llvm::DICompositeType *translateImplGetPlaceholder(DICompositeTypeAttr attr);
- /// Completes the DICompositeType `placeholder` by recursively translating the
- /// elements.
- void translateImplFillPlaceholder(DICompositeTypeAttr attr,
- llvm::DICompositeType *placeholder);
+ /// Translate the given attribute to a temporary llvm debug metadata of the
+ /// corresponding type.
+ llvm::TempDICompositeType translateTemporaryImpl(DICompositeTypeAttr attr);
/// Constructs a string metadata node from the string attribute. Returns
/// nullptr if `stringAttr` is null or contains and empty string.
@@ -121,7 +114,6 @@ class DebugTranslation {
DenseMap<Attribute, llvm::DINode *> attrToNode;
/// A mapping between recursive ID and the translated DIType.
- /// DIType.
llvm::MapVector<DistinctAttr, llvm::DIType *> recursiveTypeMap;
/// A mapping between a distinct ID and the translated LLVM metadata node.
diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index 5d70bff52bb2b9..33faa95d7d1967 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -402,3 +402,24 @@ llvm.func @func_debug_directives() {
llvm.func @class_method() {
llvm.return loc(#loc3)
} loc(#loc3)
+
+// -----
+
+// Test recursive composite type with recursion via its scope is translated.
+
+#di_composite_type_self = #llvm.di_composite_type<tag = DW_TAG_null, recId = distinct[0]<>>
+#di_file = #llvm.di_file<"test.mlir" in "/">
+#di_subroutine_type = #llvm.di_subroutine_type<types = #di_composite_type_self>
+#di_subprogram = #llvm.di_subprogram<scope = #di_file, name = "my_func", file = #di_file, line = 182, scopeLine = 182, subprogramFlags = Optimized, type = #di_subroutine_type>
+// The composite type whose scope leads to a self-recursive cycle.
+#di_composite_type = #llvm.di_composite_type<tag = DW_TAG_class_type, recId = distinct[0]<>, name = "id", file = #di_file, line = 204, scope = #di_subprogram, flags = "Public|TypePassByReference|NonTrivial", sizeInBits = 128>
+#di_global_variable = #llvm.di_global_variable<scope = #di_file, name = "my_global", file = #di_file, line = 4641, type = #di_composite_type, isDefined = true>
+#di_global_variable_expression = #llvm.di_global_variable_expression<var = #di_global_variable, expr = <>>
+
+llvm.mlir.global @global_variable() {dbg_expr = #di_global_variable_expression} : !llvm.struct<(struct<(i64)>, i32)>
+
+// CHECK: distinct !DIGlobalVariable(name: "my_global", {{.*}}, type: ![[COMP:[0-9]+]],
+// CHECK: ![[COMP]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "id", scope: ![[SCOPE:[0-9]+]],
+// CHECK: ![[SCOPE]] = !DISubprogram(name: "my_func", {{.*}}, type: ![[SUBROUTINE:[0-9]+]],
+// CHECK: ![[SUBROUTINE]] = !DISubroutineType(types: ![[SR_TYPES:[0-9]+]])
+// CHECK: ![[SR_TYPES]] = !{![[COMP]]}
>From f3862e44db6243395f70563311d2fe210bb07548 Mon Sep 17 00:00:00 2001
From: Billy Zhu <billyzhu at modular.com>
Date: Tue, 19 Mar 2024 13:20:41 -0700
Subject: [PATCH 2/3] Apply suggestions from code review
Co-authored-by: Tobias Gysi <tobias.gysi at nextsilicon.com>
---
mlir/test/Target/LLVMIR/llvmir-debug.mlir | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index 33faa95d7d1967..d6ad0b3d6e57fc 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -405,13 +405,12 @@ llvm.func @class_method() {
// -----
-// Test recursive composite type with recursion via its scope is translated.
+// Ensures composite types with a recursive scope work.
#di_composite_type_self = #llvm.di_composite_type<tag = DW_TAG_null, recId = distinct[0]<>>
#di_file = #llvm.di_file<"test.mlir" in "/">
#di_subroutine_type = #llvm.di_subroutine_type<types = #di_composite_type_self>
#di_subprogram = #llvm.di_subprogram<scope = #di_file, name = "my_func", file = #di_file, line = 182, scopeLine = 182, subprogramFlags = Optimized, type = #di_subroutine_type>
-// The composite type whose scope leads to a self-recursive cycle.
#di_composite_type = #llvm.di_composite_type<tag = DW_TAG_class_type, recId = distinct[0]<>, name = "id", file = #di_file, line = 204, scope = #di_subprogram, flags = "Public|TypePassByReference|NonTrivial", sizeInBits = 128>
#di_global_variable = #llvm.di_global_variable<scope = #di_file, name = "my_global", file = #di_file, line = 4641, type = #di_composite_type, isDefined = true>
#di_global_variable_expression = #llvm.di_global_variable_expression<var = #di_global_variable, expr = <>>
>From ef9017e78c19d53d8f52da07892277e79583b0d9 Mon Sep 17 00:00:00 2001
From: Billy Zhu <billyzhu at modular.com>
Date: Tue, 19 Mar 2024 13:26:48 -0700
Subject: [PATCH 3/3] strip all optional params from test case
---
mlir/test/Target/LLVMIR/llvmir-debug.mlir | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index d6ad0b3d6e57fc..c042628334d4c5 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -410,15 +410,15 @@ llvm.func @class_method() {
#di_composite_type_self = #llvm.di_composite_type<tag = DW_TAG_null, recId = distinct[0]<>>
#di_file = #llvm.di_file<"test.mlir" in "/">
#di_subroutine_type = #llvm.di_subroutine_type<types = #di_composite_type_self>
-#di_subprogram = #llvm.di_subprogram<scope = #di_file, name = "my_func", file = #di_file, line = 182, scopeLine = 182, subprogramFlags = Optimized, type = #di_subroutine_type>
-#di_composite_type = #llvm.di_composite_type<tag = DW_TAG_class_type, recId = distinct[0]<>, name = "id", file = #di_file, line = 204, scope = #di_subprogram, flags = "Public|TypePassByReference|NonTrivial", sizeInBits = 128>
-#di_global_variable = #llvm.di_global_variable<scope = #di_file, name = "my_global", file = #di_file, line = 4641, type = #di_composite_type, isDefined = true>
-#di_global_variable_expression = #llvm.di_global_variable_expression<var = #di_global_variable, expr = <>>
+#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>
+#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<(struct<(i64)>, i32)>
+llvm.mlir.global @global_variable() {dbg_expr = #di_global_variable_expression} : !llvm.struct<()>
-// CHECK: distinct !DIGlobalVariable(name: "my_global", {{.*}}, type: ![[COMP:[0-9]+]],
-// CHECK: ![[COMP]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "id", scope: ![[SCOPE:[0-9]+]],
-// CHECK: ![[SCOPE]] = !DISubprogram(name: "my_func", {{.*}}, type: ![[SUBROUTINE:[0-9]+]],
+// CHECK: distinct !DIGlobalVariable({{.*}}type: ![[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]+]])
// CHECK: ![[SR_TYPES]] = !{![[COMP]]}
More information about the Mlir-commits
mailing list