diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp index 1248de3db0704..126a671a58e80 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(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(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 elements; + for (DINodeAttr member : attr.getElements()) + elements.push_back(translate(member)); + return getDistinctOrUnique( 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(attr.getFlags()), - /*Elements=*/placeholderElements.get(), /*RuntimeLang=*/0, - /*VTableHolder=*/nullptr); -} - -void DebugTranslation::translateImplFillPlaceholder( - DICompositeTypeAttr attr, llvm::DICompositeType *placeholder) { - SmallVector 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(attr) .Case([&](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 d2171fed8c594..c7a5228cbf5e9 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`. - /// - ` translateImplGetPlaceholder()`: - /// Translate the DI attr without translating any potentially recursive - /// nested DI attrs. - /// - `void translateImplFillPlaceholder(, )`: - /// 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`. + /// - ` translateTemporaryImpl()`: + /// 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 attrToNode; /// A mapping between recursive ID and the translated DIType. - /// DIType. llvm::MapVector 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 5d70bff52bb2b..c042628334d4c 100644 --- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir +++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir @@ -402,3 +402,23 @@ llvm.func @func_debug_directives() { llvm.func @class_method() { llvm.return loc(#loc3) } loc(#loc3) + +// ----- + +// Ensures composite types with a recursive scope work. + +#di_composite_type_self = #llvm.di_composite_type> +#di_file = #llvm.di_file<"test.mlir" in "/"> +#di_subroutine_type = #llvm.di_subroutine_type +#di_subprogram = #llvm.di_subprogram +#di_composite_type = #llvm.di_composite_type, scope = #di_subprogram> +#di_global_variable = #llvm.di_global_variable +#di_global_variable_expression = #llvm.di_global_variable_expression + +llvm.mlir.global @global_variable() {dbg_expr = #di_global_variable_expression} : !llvm.struct<()> + +// 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]]}