From 1e8dad3bef64ada8b293e6c538b1f41ad9727cc0 Mon Sep 17 00:00:00 2001 From: Billy Zhu Date: Fri, 15 Mar 2024 09:58:25 -0700 Subject: [PATCH] [MLIR][LLVM] Support Recursive DITypes (#80251) Following the discussion from [this thread](https://discourse.llvm.org/t/handling-cyclic-dependencies-in-debug-info/67526/11), this PR adds support for recursive DITypes. This PR adds: 1. DIRecursiveTypeAttrInterface: An interface that DITypeAttrs can implement to indicate that it supports recursion. See full description in code. 2. Importer & exporter support (The only DITypeAttr that implements the interface is DICompositeTypeAttr, so the exporter is only implemented for composites too. There will be two methods that each llvm DI type that supports mutation needs to implement since there's nothing general). --------- Co-authored-by: Tobias Gysi --- mlir/include/mlir-c/Dialect/LLVM.h | 8 +- .../mlir/Dialect/LLVMIR/CMakeLists.txt | 2 + .../mlir/Dialect/LLVMIR/LLVMAttrDefs.td | 42 +++++-- mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h | 2 + .../mlir/Dialect/LLVMIR/LLVMInterfaces.td | 56 ++++++++- mlir/lib/CAPI/Dialect/LLVM.cpp | 16 +-- mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp | 19 +++ mlir/lib/Target/LLVMIR/DebugImporter.cpp | 102 ++++++++++++---- mlir/lib/Target/LLVMIR/DebugImporter.h | 18 ++- mlir/lib/Target/LLVMIR/DebugTranslation.cpp | 114 ++++++++++++++---- mlir/lib/Target/LLVMIR/DebugTranslation.h | 27 +++++ mlir/test/CAPI/llvm.c | 2 +- mlir/test/Target/LLVMIR/Import/debug-info.ll | 59 +++------ .../Target/LLVMIR/Import/import-failure.ll | 32 ----- mlir/test/Target/LLVMIR/llvmir-debug.mlir | 60 +++++++++ 15 files changed, 416 insertions(+), 143 deletions(-) diff --git a/mlir/include/mlir-c/Dialect/LLVM.h b/mlir/include/mlir-c/Dialect/LLVM.h index d823afb659c8db..b3d7a788ccbba3 100644 --- a/mlir/include/mlir-c/Dialect/LLVM.h +++ b/mlir/include/mlir-c/Dialect/LLVM.h @@ -229,10 +229,10 @@ MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDIBasicTypeAttrGet( /// Creates a LLVM DICompositeType attribute. MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDICompositeTypeAttrGet( - MlirContext ctx, unsigned int tag, MlirAttribute name, MlirAttribute file, - uint32_t line, MlirAttribute scope, MlirAttribute baseType, int64_t flags, - uint64_t sizeInBits, uint64_t alignInBits, intptr_t nElements, - MlirAttribute const *elements); + MlirContext ctx, unsigned int tag, MlirAttribute recId, MlirAttribute name, + MlirAttribute file, uint32_t line, MlirAttribute scope, + MlirAttribute baseType, int64_t flags, uint64_t sizeInBits, + uint64_t alignInBits, intptr_t nElements, MlirAttribute const *elements); /// Creates a LLVM DIDerivedType attribute. MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMDIDerivedTypeAttrGet( diff --git a/mlir/include/mlir/Dialect/LLVMIR/CMakeLists.txt b/mlir/include/mlir/Dialect/LLVMIR/CMakeLists.txt index 862abf00d03450..759de745440c21 100644 --- a/mlir/include/mlir/Dialect/LLVMIR/CMakeLists.txt +++ b/mlir/include/mlir/Dialect/LLVMIR/CMakeLists.txt @@ -29,6 +29,8 @@ add_mlir_doc(LLVMIntrinsicOps LLVMIntrinsicOps Dialects/ -gen-op-doc) set(LLVM_TARGET_DEFINITIONS LLVMInterfaces.td) mlir_tablegen(LLVMInterfaces.h.inc -gen-op-interface-decls) mlir_tablegen(LLVMInterfaces.cpp.inc -gen-op-interface-defs) +mlir_tablegen(LLVMAttrInterfaces.h.inc -gen-attr-interface-decls) +mlir_tablegen(LLVMAttrInterfaces.cpp.inc -gen-attr-interface-defs) mlir_tablegen(LLVMTypeInterfaces.h.inc -gen-type-interface-decls) mlir_tablegen(LLVMTypeInterfaces.cpp.inc -gen-type-interface-defs) add_public_tablegen_target(MLIRLLVMInterfacesIncGen) diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td index a831b076fb864f..d4d56ae0f762bc 100644 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td @@ -10,6 +10,7 @@ #define LLVMIR_ATTRDEFS include "mlir/Dialect/LLVMIR/LLVMDialect.td" +include "mlir/Dialect/LLVMIR/LLVMInterfaces.td" include "mlir/IR/AttrTypeBase.td" include "mlir/IR/CommonAttrConstraints.td" @@ -238,7 +239,7 @@ def LoopAnnotationAttr : LLVM_Attr<"LoopAnnotation", "loop_annotation"> { //===----------------------------------------------------------------------===// class LLVM_DIParameter + string errorCase, string printName = parseName> : AttrOrTypeParameter<"unsigned", "debug info " # summary> { let parser = [{ [&]() -> FailureOr { SMLoc tagLoc = $_parser.getCurrentLocation(); @@ -246,33 +247,35 @@ class LLVM_DIParameter; def LLVM_DIEncodingParameter : LLVM_DIParameter< - "encoding", /*default=*/"0", "AttributeEncoding" + "encoding", /*default=*/"0", "AttributeEncoding", /*errorCase=*/"0" >; def LLVM_DILanguageParameter : LLVM_DIParameter< - "language", /*default=*/"", "Language" + "language", /*default=*/"", "Language", /*errorCase=*/"0" >; def LLVM_DITagParameter : LLVM_DIParameter< - "tag", /*default=*/"", "Tag" + "tag", /*default=*/"", "Tag", /*errorCase=*/"llvm::dwarf::DW_TAG_invalid" >; def LLVM_DIOperationEncodingParameter : LLVM_DIParameter< - "operation encoding", /*default=*/"", "OperationEncoding" + "operation encoding", /*default=*/"", "OperationEncoding", /*errorCase=*/"0" >; //===----------------------------------------------------------------------===// @@ -357,9 +360,11 @@ def LLVM_DICompileUnitAttr : LLVM_Attr<"DICompileUnit", "di_compile_unit", //===----------------------------------------------------------------------===// def LLVM_DICompositeTypeAttr : LLVM_Attr<"DICompositeType", "di_composite_type", - /*traits=*/[], "DITypeAttr"> { + [LLVM_DIRecursiveTypeAttrInterface], + "DITypeAttr"> { let parameters = (ins LLVM_DITagParameter:$tag, + OptionalParameter<"DistinctAttr">:$recId, OptionalParameter<"StringAttr">:$name, OptionalParameter<"DIFileAttr">:$file, OptionalParameter<"uint32_t">:$line, @@ -371,6 +376,21 @@ def LLVM_DICompositeTypeAttr : LLVM_Attr<"DICompositeType", "di_composite_type", OptionalArrayRefParameter<"DINodeAttr">:$elements ); let assemblyFormat = "`<` struct(params) `>`"; + let extraClassDeclaration = [{ + /// Requirements of DIRecursiveTypeAttrInterface. + /// @{ + + /// Get whether this attr describes a recursive self reference. + bool isRecSelf() { return getTag() == 0; } + + /// Get a copy of this type attr but with the recursive ID set to `recId`. + DIRecursiveTypeAttrInterface withRecId(DistinctAttr recId); + + /// Build a rec-self instance using the provided `recId`. + static DIRecursiveTypeAttrInterface getRecSelf(DistinctAttr recId); + + /// @} + }]; } //===----------------------------------------------------------------------===// diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h index c370bfa2b733d6..091a817caf3d6e 100644 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h @@ -84,6 +84,8 @@ using linkage::Linkage; } // namespace LLVM } // namespace mlir +#include "mlir/Dialect/LLVMIR/LLVMAttrInterfaces.h.inc" + #define GET_ATTRDEF_CLASSES #include "mlir/Dialect/LLVMIR/LLVMOpsAttrDefs.h.inc" diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMInterfaces.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMInterfaces.td index 3b2a132a881e4e..e7a1da8ee560ef 100644 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMInterfaces.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMInterfaces.td @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// // -// This file defines op and type interfaces for the LLVM dialect in MLIR. +// This file defines interfaces for the LLVM dialect in MLIR. // //===----------------------------------------------------------------------===// @@ -319,4 +319,58 @@ def LLVM_PointerElementTypeInterface ]; } +//===----------------------------------------------------------------------===// +// LLVM dialect attr interfaces. +//===----------------------------------------------------------------------===// + +def LLVM_DIRecursiveTypeAttrInterface + : AttrInterface<"DIRecursiveTypeAttrInterface"> { + let description = [{ + This attribute represents a DITypeAttr that is recursive. Only DITypeAttrs + that translate to LLVM DITypes that support mutation should implement this + interface. + + There are two modes for conforming attributes: + + 1. "rec-decl": + - This attr is a recursive declaration identified by a recId. + + 2. "rec-self": + - This attr is considered a recursive self reference. + - This attr itself is a placeholder type that should be conceptually + replaced with the closest parent attr of the same type with the same + recId. + + For example, to represent a linked list struct: + + #rec_self = di_composite_type + #ptr = di_derived_type + #field = di_derived_type + #rec = di_composite_type + #var = di_local_variable + + Note that a rec-self without an outer rec-decl with the same recId is + conceptually the same as an "unbound" variable. The context needs to provide + meaning to the rec-self. + }]; + let cppNamespace = "::mlir::LLVM"; + let methods = [ + InterfaceMethod<[{ + Get whether this attr describes a recursive self reference. + }], "bool", "isRecSelf", (ins)>, + InterfaceMethod<[{ + Get the recursive ID used for matching "rec-decl" with "rec-self". + If this attr instance is not recursive, return a null attribute. + }], "DistinctAttr", "getRecId", (ins)>, + InterfaceMethod<[{ + Get a copy of this type attr but with the recursive ID set to `recId`. + }], "DIRecursiveTypeAttrInterface", "withRecId", + (ins "DistinctAttr":$recId)>, + StaticInterfaceMethod<[{ + Build a rec-self instance using the provided `recId`. + }], "DIRecursiveTypeAttrInterface", "getRecSelf", + (ins "DistinctAttr":$recId)> + ]; +} + #endif // LLVMIR_INTERFACES diff --git a/mlir/lib/CAPI/Dialect/LLVM.cpp b/mlir/lib/CAPI/Dialect/LLVM.cpp index 2d938ce5f4834c..d0fd5ceecfff5f 100644 --- a/mlir/lib/CAPI/Dialect/LLVM.cpp +++ b/mlir/lib/CAPI/Dialect/LLVM.cpp @@ -152,18 +152,18 @@ MlirAttribute mlirLLVMDIBasicTypeAttrGet(MlirContext ctx, unsigned int tag, } MlirAttribute mlirLLVMDICompositeTypeAttrGet( - MlirContext ctx, unsigned int tag, MlirAttribute name, MlirAttribute file, - uint32_t line, MlirAttribute scope, MlirAttribute baseType, int64_t flags, - uint64_t sizeInBits, uint64_t alignInBits, intptr_t nElements, - MlirAttribute const *elements) { + MlirContext ctx, unsigned int tag, MlirAttribute recId, MlirAttribute name, + MlirAttribute file, uint32_t line, MlirAttribute scope, + MlirAttribute baseType, int64_t flags, uint64_t sizeInBits, + uint64_t alignInBits, intptr_t nElements, MlirAttribute const *elements) { SmallVector elementsStorage; elementsStorage.reserve(nElements); return wrap(DICompositeTypeAttr::get( - unwrap(ctx), tag, cast(unwrap(name)), - cast(unwrap(file)), line, cast(unwrap(scope)), - cast(unwrap(baseType)), DIFlags(flags), sizeInBits, - alignInBits, + unwrap(ctx), tag, cast(unwrap(recId)), + cast(unwrap(name)), cast(unwrap(file)), line, + cast(unwrap(scope)), cast(unwrap(baseType)), + DIFlags(flags), sizeInBits, alignInBits, llvm::map_to_vector(unwrapList(nElements, elements, elementsStorage), [](Attribute a) { return a.cast(); }))); } diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp index 645a45dd96befb..a44e83313c9c1f 100644 --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp @@ -35,6 +35,7 @@ static LogicalResult parseExpressionArg(AsmParser &parser, uint64_t opcode, static void printExpressionArg(AsmPrinter &printer, uint64_t opcode, ArrayRef args); +#include "mlir/Dialect/LLVMIR/LLVMAttrInterfaces.cpp.inc" #include "mlir/Dialect/LLVMIR/LLVMOpsEnums.cpp.inc" #define GET_ATTRDEF_CLASSES #include "mlir/Dialect/LLVMIR/LLVMOpsAttrDefs.cpp.inc" @@ -185,6 +186,24 @@ void printExpressionArg(AsmPrinter &printer, uint64_t opcode, }); } +//===----------------------------------------------------------------------===// +// DICompositeTypeAttr +//===----------------------------------------------------------------------===// + +DIRecursiveTypeAttrInterface +DICompositeTypeAttr::withRecId(DistinctAttr recId) { + return DICompositeTypeAttr::get(getContext(), getTag(), recId, getName(), + getFile(), getLine(), getScope(), + getBaseType(), getFlags(), getSizeInBits(), + getAlignInBits(), getElements()); +} + +DIRecursiveTypeAttrInterface +DICompositeTypeAttr::getRecSelf(DistinctAttr recId) { + return DICompositeTypeAttr::get(recId.getContext(), 0, recId, {}, {}, 0, {}, + {}, DIFlags(), 0, 0, {}); +} + //===----------------------------------------------------------------------===// // TargetFeaturesAttr //===----------------------------------------------------------------------===// diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp index c631617f973544..5ba90bba18b197 100644 --- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp +++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp @@ -13,6 +13,7 @@ #include "mlir/IR/Location.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" +#include "llvm/ADT/TypeSwitch.h" #include "llvm/BinaryFormat/Dwarf.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DebugInfoMetadata.h" @@ -51,10 +52,9 @@ DICompileUnitAttr DebugImporter::translateImpl(llvm::DICompileUnit *node) { std::optional emissionKind = symbolizeDIEmissionKind(node->getEmissionKind()); return DICompileUnitAttr::get( - context, DistinctAttr::create(UnitAttr::get(context)), - node->getSourceLanguage(), translate(node->getFile()), - getStringAttrOrNull(node->getRawProducer()), node->isOptimized(), - emissionKind.value()); + context, getOrCreateDistinctID(node), node->getSourceLanguage(), + translate(node->getFile()), getStringAttrOrNull(node->getRawProducer()), + node->isOptimized(), emissionKind.value()); } DICompositeTypeAttr DebugImporter::translateImpl(llvm::DICompositeType *node) { @@ -64,11 +64,7 @@ DICompositeTypeAttr DebugImporter::translateImpl(llvm::DICompositeType *node) { assert(element && "expected a non-null element type"); elements.push_back(translate(element)); } - // Drop the elements parameter if a cyclic dependency is detected. We - // currently cannot model these cycles and thus drop the parameter if - // required. A cyclic dependency is detected if one of the element nodes - // translates to a nullptr since the node is already on the translation stack. - // TODO: Support debug metadata with cyclic dependencies. + // Drop the elements parameter if any of the elements are invalid. if (llvm::is_contained(elements, nullptr)) elements.clear(); DITypeAttr baseType = translate(node->getBaseType()); @@ -77,14 +73,15 @@ DICompositeTypeAttr DebugImporter::translateImpl(llvm::DICompositeType *node) { if (node->getTag() == llvm::dwarf::DW_TAG_array_type && !baseType) return nullptr; return DICompositeTypeAttr::get( - context, node->getTag(), getStringAttrOrNull(node->getRawName()), - translate(node->getFile()), node->getLine(), translate(node->getScope()), - baseType, flags.value_or(DIFlags::Zero), node->getSizeInBits(), + context, node->getTag(), /*recId=*/{}, + getStringAttrOrNull(node->getRawName()), translate(node->getFile()), + node->getLine(), translate(node->getScope()), baseType, + flags.value_or(DIFlags::Zero), node->getSizeInBits(), node->getAlignInBits(), elements); } DIDerivedTypeAttr DebugImporter::translateImpl(llvm::DIDerivedType *node) { - // Return nullptr if the base type is a cyclic dependency. + // Return nullptr if the base type is invalid. DITypeAttr baseType = translate(node->getBaseType()); if (node->getBaseType() && !baseType) return nullptr; @@ -179,10 +176,10 @@ DISubprogramAttr DebugImporter::translateImpl(llvm::DISubprogram *node) { // Only definitions require a distinct identifier. mlir::DistinctAttr id; if (node->isDistinct()) - id = DistinctAttr::create(UnitAttr::get(context)); + id = getOrCreateDistinctID(node); std::optional subprogramFlags = symbolizeDISubprogramFlags(node->getSubprogram()->getSPFlags()); - // Return nullptr if the scope or type is a cyclic dependency. + // Return nullptr if the scope or type is invalid. DIScopeAttr scope = translate(node->getScope()); if (node->getScope() && !scope) return nullptr; @@ -229,7 +226,7 @@ DebugImporter::translateImpl(llvm::DISubroutineType *node) { } types.push_back(translate(type)); } - // Return nullptr if any of the types is a cyclic dependency. + // Return nullptr if any of the types is invalid. if (llvm::is_contained(types, nullptr)) return nullptr; return DISubroutineTypeAttr::get(context, node->getCC(), types); @@ -247,12 +244,42 @@ DINodeAttr DebugImporter::translate(llvm::DINode *node) { if (DINodeAttr attr = nodeToAttr.lookup(node)) return attr; - // Return nullptr if a cyclic dependency is detected since the same node is - // being traversed twice. This check avoids infinite recursion if the debug - // metadata contains cycles. - if (!translationStack.insert(node)) - return nullptr; - auto guard = llvm::make_scope_exit([&]() { translationStack.pop_back(); }); + // If the node type is capable of being recursive, check if it's seen before. + auto recSelfCtor = getRecSelfConstructor(node); + if (recSelfCtor) { + // If a cyclic dependency is detected since the same node is being traversed + // twice, emit a recursive self type, and mark the duplicate node on the + // translationStack so it can emit a recursive decl type. + auto [iter, inserted] = translationStack.try_emplace(node, nullptr); + if (!inserted) { + // The original node may have already been assigned a recursive ID from + // a different self-reference. Use that if possible. + DistinctAttr recId = iter->second; + if (!recId) { + recId = DistinctAttr::create(UnitAttr::get(context)); + iter->second = recId; + } + unboundRecursiveSelfRefs.back().insert(recId); + return cast(recSelfCtor(recId)); + } + } + + unboundRecursiveSelfRefs.emplace_back(); + + auto guard = llvm::make_scope_exit([&]() { + if (recSelfCtor) + translationStack.pop_back(); + + // Copy unboundRecursiveSelfRefs down to the previous level. + if (unboundRecursiveSelfRefs.size() == 1) + assert(unboundRecursiveSelfRefs.back().empty() && + "internal error: unbound recursive self reference at top level."); + else + unboundRecursiveSelfRefs[unboundRecursiveSelfRefs.size() - 2].insert( + unboundRecursiveSelfRefs.back().begin(), + unboundRecursiveSelfRefs.back().end()); + unboundRecursiveSelfRefs.pop_back(); + }); // Convert the debug metadata if possible. auto translateNode = [this](llvm::DINode *node) -> DINodeAttr { @@ -289,7 +316,19 @@ DINodeAttr DebugImporter::translate(llvm::DINode *node) { return nullptr; }; if (DINodeAttr attr = translateNode(node)) { - nodeToAttr.insert({node, attr}); + // If this node was marked as recursive, set its recId. + if (auto recType = dyn_cast(attr)) { + if (DistinctAttr recId = translationStack.lookup(node)) { + attr = cast(recType.withRecId(recId)); + // Remove the unbound recursive ID from the set of unbound self + // references in the translation stack. + unboundRecursiveSelfRefs.back().erase(recId); + } + } + + // Only cache fully self-contained nodes. + if (unboundRecursiveSelfRefs.back().empty()) + nodeToAttr.try_emplace(node, attr); return attr; } return nullptr; @@ -346,3 +385,20 @@ StringAttr DebugImporter::getStringAttrOrNull(llvm::MDString *stringNode) { return StringAttr(); return StringAttr::get(context, stringNode->getString()); } + +DistinctAttr DebugImporter::getOrCreateDistinctID(llvm::DINode *node) { + DistinctAttr &id = nodeToDistinctAttr[node]; + if (!id) + id = DistinctAttr::create(UnitAttr::get(context)); + return id; +} + +function_ref +DebugImporter::getRecSelfConstructor(llvm::DINode *node) { + using CtorType = function_ref; + return TypeSwitch(node) + .Case([&](llvm::DICompositeType *concreteNode) { + return CtorType(DICompositeTypeAttr::getRecSelf); + }) + .Default(CtorType()); +} diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.h b/mlir/lib/Target/LLVMIR/DebugImporter.h index 7d4a371284b68b..bcf628fc4234fb 100644 --- a/mlir/lib/Target/LLVMIR/DebugImporter.h +++ b/mlir/lib/Target/LLVMIR/DebugImporter.h @@ -82,12 +82,28 @@ class DebugImporter { /// null attribute otherwise. StringAttr getStringAttrOrNull(llvm::MDString *stringNode); + /// Get the DistinctAttr used to represent `node` if one was already created + /// for it, or create a new one if not. + DistinctAttr getOrCreateDistinctID(llvm::DINode *node); + + /// Get the `getRecSelf` constructor for the translated type of `node` if its + /// translated DITypeAttr supports recursion. Otherwise, returns nullptr. + function_ref + getRecSelfConstructor(llvm::DINode *node); + /// A mapping between LLVM debug metadata and the corresponding attribute. DenseMap nodeToAttr; + /// A mapping between distinct LLVM debug metadata nodes and the corresponding + /// distinct id attribute. + DenseMap nodeToDistinctAttr; /// A stack that stores the metadata nodes that are being traversed. The stack /// is used to detect cyclic dependencies during the metadata translation. - SetVector translationStack; + /// A node is pushed with a null value. If it is ever seen twice, it is given + /// a recursive id attribute, indicating that it is a recursive node. + llvm::MapVector translationStack; + /// All the unbound recursive self references in the translation stack. + SmallVector> unboundRecursiveSelfRefs; MLIRContext *context; ModuleOp mlirModule; diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp index 16918aab549788..eaf9373e7616c5 100644 --- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp @@ -117,11 +117,7 @@ static DINodeT *getDistinctOrUnique(bool isDistinct, Ts &&...args) { } llvm::DICompositeType * -DebugTranslation::translateImpl(DICompositeTypeAttr attr) { - SmallVector elements; - for (auto member : attr.getElements()) - elements.push_back(translate(member)); - +DebugTranslation::translateImplGetPlaceholder(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; @@ -133,6 +129,8 @@ DebugTranslation::translateImpl(DICompositeTypeAttr attr) { isDistinct = true; } + llvm::TempMDTuple placeholderElements = + llvm::MDNode::getTemporary(llvmCtx, std::nullopt); return getDistinctOrUnique( isDistinct, llvmCtx, attr.getTag(), getMDStringOrNull(attr.getName()), translate(attr.getFile()), attr.getLine(), translate(attr.getScope()), @@ -140,8 +138,23 @@ DebugTranslation::translateImpl(DICompositeTypeAttr attr) { attr.getAlignInBits(), /*OffsetInBits=*/0, /*Flags=*/static_cast(attr.getFlags()), - llvm::MDNode::get(llvmCtx, elements), - /*RuntimeLang=*/0, /*VTableHolder=*/nullptr); + /*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::DIDerivedType *DebugTranslation::translateImpl(DIDerivedTypeAttr attr) { @@ -200,22 +213,71 @@ DebugTranslation::translateImpl(DIGlobalVariableAttr attr) { attr.getIsDefined(), nullptr, nullptr, attr.getAlignInBits(), nullptr); } +llvm::DIType * +DebugTranslation::translateRecursive(DIRecursiveTypeAttrInterface attr) { + DistinctAttr recursiveId = attr.getRecId(); + if (attr.isRecSelf()) { + auto *iter = recursiveTypeMap.find(recursiveId); + assert(iter != recursiveTypeMap.end() && "unbound DI recursive self type"); + return iter->second; + } + + auto setRecursivePlaceholder = [&](llvm::DIType *placeholder) { + auto [iter, inserted] = + recursiveTypeMap.try_emplace(recursiveId, placeholder); + assert(inserted && "illegal reuse of recursive id"); + }; + + llvm::DIType *result = + TypeSwitch(attr) + .Case([&](auto attr) { + auto *placeholder = translateImplGetPlaceholder(attr); + setRecursivePlaceholder(placeholder); + translateImplFillPlaceholder(attr, placeholder); + return placeholder; + }); + + assert(recursiveTypeMap.back().first == recursiveId && + "internal inconsistency: unexpected recursive translation stack"); + recursiveTypeMap.pop_back(); + + return result; +} + llvm::DIScope *DebugTranslation::translateImpl(DIScopeAttr attr) { return cast(translate(DINodeAttr(attr))); } llvm::DISubprogram *DebugTranslation::translateImpl(DISubprogramAttr attr) { + if (auto iter = distinctAttrToNode.find(attr.getId()); + iter != distinctAttrToNode.end()) + return cast(iter->second); + + llvm::DIScope *scope = translate(attr.getScope()); + llvm::DIFile *file = translate(attr.getFile()); + llvm::DIType *type = translate(attr.getType()); + llvm::DICompileUnit *compileUnit = translate(attr.getCompileUnit()); + + // Check again after recursive calls in case this distinct node recurses back + // to itself. + if (auto iter = distinctAttrToNode.find(attr.getId()); + iter != distinctAttrToNode.end()) + return cast(iter->second); + bool isDefinition = static_cast(attr.getSubprogramFlags() & LLVM::DISubprogramFlags::Definition); - return getDistinctOrUnique( - isDefinition, llvmCtx, translate(attr.getScope()), - getMDStringOrNull(attr.getName()), - getMDStringOrNull(attr.getLinkageName()), translate(attr.getFile()), - attr.getLine(), translate(attr.getType()), attr.getScopeLine(), + llvm::DISubprogram *node = getDistinctOrUnique( + isDefinition, llvmCtx, scope, getMDStringOrNull(attr.getName()), + getMDStringOrNull(attr.getLinkageName()), file, attr.getLine(), type, + attr.getScopeLine(), /*ContainingType=*/nullptr, /*VirtualIndex=*/0, /*ThisAdjustment=*/0, llvm::DINode::FlagZero, static_cast(attr.getSubprogramFlags()), - translate(attr.getCompileUnit())); + compileUnit); + + if (attr.getId()) + distinctAttrToNode.try_emplace(attr.getId(), node); + return node; } llvm::DIModule *DebugTranslation::translateImpl(DIModuleAttr attr) { @@ -268,15 +330,23 @@ llvm::DINode *DebugTranslation::translate(DINodeAttr attr) { if (llvm::DINode *node = attrToNode.lookup(attr)) return node; - llvm::DINode *node = - TypeSwitch(attr) - .Case( - [&](auto attr) { return translateImpl(attr); }); + llvm::DINode *node = nullptr; + // Recursive types go through a dedicated handler. All other types are + // dispatched directly to their specific handlers. + if (auto recTypeAttr = dyn_cast(attr)) + if (recTypeAttr.getRecId()) + node = translateRecursive(recTypeAttr); + + if (!node) + node = TypeSwitch(attr) + .Case( + [&](auto attr) { return translateImpl(attr); }); + attrToNode.insert({attr, node}); return node; } diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.h b/mlir/lib/Target/LLVMIR/DebugTranslation.h index 627c6846844983..d2171fed8c594f 100644 --- a/mlir/lib/Target/LLVMIR/DebugTranslation.h +++ b/mlir/lib/Target/LLVMIR/DebugTranslation.h @@ -88,6 +88,24 @@ 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. + 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); + /// Constructs a string metadata node from the string attribute. Returns /// nullptr if `stringAttr` is null or contains and empty string. llvm::MDString *getMDStringOrNull(StringAttr stringAttr); @@ -102,6 +120,15 @@ class DebugTranslation { /// metadata. 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. + /// This helps identify attrs that should translate into the same LLVM debug + /// node. + DenseMap distinctAttrToNode; + /// A mapping between filename and llvm debug file. /// TODO: Change this to DenseMap when we can /// access the Identifier filename in FileLineColLoc. diff --git a/mlir/test/CAPI/llvm.c b/mlir/test/CAPI/llvm.c index 2fd98b29f487c8..48b887ee4a954c 100644 --- a/mlir/test/CAPI/llvm.c +++ b/mlir/test/CAPI/llvm.c @@ -301,7 +301,7 @@ static void testDebugInfoAttributes(MlirContext ctx) { // CHECK: #llvm.di_composite_type<{{.*}}> mlirAttributeDump(mlirLLVMDICompositeTypeAttrGet( - ctx, 0, foo, file, 1, compile_unit, di_type, 0, 64, 8, 1, &di_type)); + ctx, 0, id, foo, file, 1, compile_unit, di_type, 0, 64, 8, 1, &di_type)); MlirAttribute subroutine_type = mlirLLVMDISubroutineTypeAttrGet(ctx, 0x0, 1, &di_type); diff --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll index 9ef6580bcf2408..9af40d8c8d3ee6 100644 --- a/mlir/test/Target/LLVMIR/Import/debug-info.ll +++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll @@ -296,12 +296,17 @@ define void @class_method() { ret void, !dbg !9 } -; Verify the elements parameter is dropped due to the cyclic dependencies. -; CHECK: #[[COMP:.+]] = #llvm.di_composite_type -; CHECK: #[[COMP_PTR:.+]] = #llvm.di_derived_type +; Verify the cyclic composite type is identified, even though conversion begins from the subprogram type. +; CHECK: #[[COMP_SELF:.+]] = #llvm.di_composite_type +; CHECK: #[[COMP_PTR:.+]] = #llvm.di_derived_type ; CHECK: #[[SP_TYPE:.+]] = #llvm.di_subroutine_type -; CHECK: #[[SP:.+]] = #llvm.di_subprogram, compileUnit = #{{.*}}, scope = #[[COMP]], name = "class_method", file = #{{.*}}, subprogramFlags = Definition, type = #[[SP_TYPE]]> -; CHECK: #[[LOC]] = loc(fused<#[[SP]]> +; CHECK: #[[SP_INNER:.+]] = #llvm.di_subprogram +; CHECK: #[[COMP:.+]] = #llvm.di_composite_type + +; CHECK: #[[COMP_PTR_OUTER:.+]] = #llvm.di_derived_type +; CHECK: #[[SP_TYPE_OUTER:.+]] = #llvm.di_subroutine_type +; CHECK: #[[SP_OUTER:.+]] = #llvm.di_subprogram +; CHECK: #[[LOC]] = loc(fused<#[[SP_OUTER]]> !llvm.dbg.cu = !{!1} !llvm.module.flags = !{!0} @@ -318,15 +323,18 @@ define void @class_method() { ; // ----- -; Verify the elements parameter is dropped due to the cyclic dependencies. -; CHECK: #[[$COMP:.+]] = #llvm.di_composite_type -; CHECK: #[[$COMP_PTR:.+]] = #llvm.di_derived_type -; CHECK: #[[$VAR0:.+]] = #llvm.di_local_variable +; Verify the cyclic composite type is handled correctly. +; CHECK: #[[COMP_SELF:.+]] = #llvm.di_composite_type +; CHECK: #[[COMP_PTR_INNER:.+]] = #llvm.di_derived_type +; CHECK: #[[FIELD:.+]] = #llvm.di_derived_type +; CHECK: #[[COMP:.+]] = #llvm.di_composite_type +; CHECK: #[[COMP_PTR_OUTER:.+]] = #llvm.di_derived_type +; CHECK: #[[VAR0:.+]] = #llvm.di_local_variable -; CHECK-LABEL: @class_field +; CHECK: @class_field ; CHECK-SAME: %[[ARG0:[a-zA-Z0-9]+]] define void @class_field(ptr %arg1) { - ; CHECK: llvm.intr.dbg.value #[[$VAR0]] = %[[ARG0]] : !llvm.ptr + ; CHECK: llvm.intr.dbg.value #[[VAR0]] = %[[ARG0]] : !llvm.ptr call void @llvm.dbg.value(metadata ptr %arg1, metadata !7, metadata !DIExpression()), !dbg !9 ret void } @@ -563,35 +571,6 @@ define void @func_in_module(ptr %arg) !dbg !8 { ; // ----- -; Verifies that array types that have an unimportable base type are removed to -; avoid producing invalid IR. -; CHECK: #[[DI_LOCAL_VAR:.+]] = #llvm.di_local_variable< -; CHECK-NOT: type = - -; CHECK-LABEL: @array_with_cyclic_base_type -define i32 @array_with_cyclic_base_type(ptr %0) !dbg !3 { - call void @llvm.dbg.value(metadata ptr %0, metadata !4, metadata !DIExpression()), !dbg !7 - ret i32 0 -} - -; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none) -declare void @llvm.dbg.value(metadata, metadata, metadata) - - -!llvm.module.flags = !{!0} -!llvm.dbg.cu = !{!1} - -!0 = !{i32 2, !"Debug Info Version", i32 3} -!1 = distinct !DICompileUnit(language: DW_LANG_C, file: !2) -!2 = !DIFile(filename: "debug-info.ll", directory: "/") -!3 = distinct !DISubprogram(name: "func", scope: !2, file: !2, line: 46, scopeLine: 48, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !1) -!4 = !DILocalVariable(name: "op", arg: 5, scope: !3, file: !2, line: 47, type: !5) -!5 = !DICompositeType(tag: DW_TAG_array_type, size: 42, baseType: !6) -!6 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !5) -!7 = !DILocation(line: 0, scope: !3) - -; // ----- - ; Verifies that import compile units respect the distinctness of the input. ; CHECK-LABEL: @distinct_cu_func0 define void @distinct_cu_func0() !dbg !4 { diff --git a/mlir/test/Target/LLVMIR/Import/import-failure.ll b/mlir/test/Target/LLVMIR/Import/import-failure.ll index 9a4e939d106516..3f4efab70e1c02 100644 --- a/mlir/test/Target/LLVMIR/Import/import-failure.ll +++ b/mlir/test/Target/LLVMIR/Import/import-failure.ll @@ -85,38 +85,6 @@ define void @unsupported_argument(i64 %arg1) { ; // ----- -; Check that debug intrinsics that depend on cyclic metadata are dropped. - -declare void @llvm.dbg.value(metadata, metadata, metadata) - -; CHECK: import-failure.ll -; CHECK-SAME: warning: dropped instruction: call void @llvm.dbg.label(metadata !{{.*}}) -; CHECK: import-failure.ll -; CHECK-SAME: warning: dropped intrinsic: call void @llvm.dbg.value(metadata i64 %{{.*}}, metadata !3, metadata !DIExpression()) -define void @cylic_metadata(i64 %arg1) { - call void @llvm.dbg.value(metadata i64 %arg1, metadata !10, metadata !DIExpression()), !dbg !14 - call void @llvm.dbg.label(metadata !13), !dbg !14 - ret void -} - -!llvm.dbg.cu = !{!1} -!llvm.module.flags = !{!0} -!0 = !{i32 2, !"Debug Info Version", i32 3} -!1 = distinct !DICompileUnit(language: DW_LANG_C, file: !2) -!2 = !DIFile(filename: "import-failure.ll", directory: "/") -!3 = !DICompositeType(tag: DW_TAG_array_type, size: 42, baseType: !4) -!4 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !3) -!5 = distinct !DISubprogram(name: "class_method", scope: !2, file: !2, type: !6, spFlags: DISPFlagDefinition, unit: !1) -!6 = !DISubroutineType(types: !7) -!7 = !{!3} -!10 = !DILocalVariable(scope: !5, name: "arg1", file: !2, line: 1, arg: 1, align: 64); -!11 = !DILexicalBlock(scope: !5) -!12 = !DILexicalBlockFile(scope: !11, discriminator: 0) -!13 = !DILabel(scope: !12, name: "label", file: !2, line: 42) -!14 = !DILocation(line: 1, column: 2, scope: !5) - -; // ----- - ; global_dtors with non-null data fields cannot be represented in MLIR. ; CHECK: ; CHECK-SAME: error: unhandled global variable: @llvm.global_dtors diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir index cfd5239515c9c0..5d70bff52bb2b9 100644 --- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir +++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir @@ -342,3 +342,63 @@ llvm.func @func_line_tables() { llvm.func @func_debug_directives() { llvm.return } loc(fused<#di_subprogram_2>["foo2.mlir":0:0]) + +// ----- + +// Ensure recursive types with multiple external references work. + +// Common base nodes. +#di_file = #llvm.di_file<"test.mlir" in "/"> +#di_null_type = #llvm.di_null_type +#di_compile_unit = #llvm.di_compile_unit, sourceLanguage = DW_LANG_C, file = #di_file, isOptimized = false, emissionKind = None> + +// Recursive type itself. +#di_struct_self = #llvm.di_composite_type> +#di_ptr_inner = #llvm.di_derived_type +#di_subroutine_inner = #llvm.di_subroutine_type +#di_subprogram_inner = #llvm.di_subprogram< + id = distinct[2]<>, + compileUnit = #di_compile_unit, + scope = #di_struct_self, + name = "class_method", + file = #di_file, + subprogramFlags = Definition, + type = #di_subroutine_inner> +#di_struct = #llvm.di_composite_type< + tag = DW_TAG_class_type, + recId = distinct[0]<>, + name = "class_name", + file = #di_file, + line = 42, + flags = "TypePassByReference|NonTrivial", + elements = #di_subprogram_inner> + +// Outer types referencing the entire recursive type. +#di_ptr_outer = #llvm.di_derived_type +#di_subroutine_outer = #llvm.di_subroutine_type +#di_subprogram_outer = #llvm.di_subprogram< + id = distinct[2]<>, + compileUnit = #di_compile_unit, + scope = #di_struct, + name = "class_method", + file = #di_file, + subprogramFlags = Definition, + type = #di_subroutine_outer> + +#loc3 = loc(fused<#di_subprogram_outer>["test.mlir":1:1]) + +// CHECK: @class_method +// CHECK: ret void, !dbg ![[LOC:.*]] + +// CHECK: ![[CU:.*]] = distinct !DICompileUnit( +// CHECK: ![[SP:.*]] = distinct !DISubprogram(name: "class_method", scope: ![[STRUCT:.*]], file: !{{.*}}, type: ![[SUBROUTINE:.*]], spFlags: DISPFlagDefinition, unit: ![[CU]]) +// CHECK: ![[STRUCT]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "class_name", {{.*}}, elements: ![[ELEMS:.*]]) +// CHECK: ![[ELEMS]] = !{![[SP]]} +// CHECK: ![[SUBROUTINE]] = !DISubroutineType(types: ![[SUBROUTINE_ELEMS:.*]]) +// CHECK: ![[SUBROUTINE_ELEMS]] = !{null, ![[PTR:.*]]} +// CHECK: ![[PTR]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[STRUCT]], size: 64) +// CHECK: ![[LOC]] = !DILocation(line: 1, column: 1, scope: ![[SP]]) + +llvm.func @class_method() { + llvm.return loc(#loc3) +} loc(#loc3)