diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp index 15737aa681c593..f104b72209c391 100644 --- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp +++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp @@ -28,7 +28,7 @@ using namespace mlir::LLVM::detail; DebugImporter::DebugImporter(ModuleOp mlirModule, bool dropDICompositeTypeElements) - : recursionPruner(mlirModule.getContext()), + : cache([&](llvm::DINode *node) { return createRecSelf(node); }), context(mlirModule.getContext()), mlirModule(mlirModule), dropDICompositeTypeElements(dropDICompositeTypeElements) {} @@ -287,16 +287,9 @@ DINodeAttr DebugImporter::translate(llvm::DINode *node) { return nullptr; // Check for a cached instance. - if (DINodeAttr attr = nodeToAttr.lookup(node)) - return attr; - - // Register with the recursive translator. If it can be handled without - // recursing into it, return the result immediately. - if (DINodeAttr attr = recursionPruner.pruneOrPushTranslationStack(node)) - return attr; - - auto guard = llvm::make_scope_exit( - [&]() { recursionPruner.popTranslationStack(node); }); + auto cacheEntry = cache.lookupOrInit(node); + if (std::optional result = cacheEntry.get()) + return *result; // Convert the debug metadata if possible. auto translateNode = [this](llvm::DINode *node) -> DINodeAttr { @@ -335,20 +328,20 @@ DINodeAttr DebugImporter::translate(llvm::DINode *node) { return nullptr; }; if (DINodeAttr attr = translateNode(node)) { - auto [result, isSelfContained] = - recursionPruner.finalizeTranslation(node, attr); - // Only cache fully self-contained nodes. - if (isSelfContained) - nodeToAttr.try_emplace(node, result); - return result; + // If this node was repeated, lookup its recursive ID and assign it to the + // base result. + if (cacheEntry.wasRepeated()) { + DistinctAttr recId = nodeToRecId.lookup(node); + auto recType = cast(attr); + attr = cast(recType.withRecId(recId)); + } + cacheEntry.resolve(attr); + return attr; } + cacheEntry.resolve(nullptr); return nullptr; } -//===----------------------------------------------------------------------===// -// RecursionPruner -//===----------------------------------------------------------------------===// - /// Get the `getRecSelf` constructor for the translated type of `node` if its /// translated DITypeAttr supports recursion. Otherwise, returns nullptr. static function_ref @@ -361,104 +354,20 @@ getRecSelfConstructor(llvm::DINode *node) { .Default(CtorType()); } -DINodeAttr DebugImporter::RecursionPruner::pruneOrPushTranslationStack( - llvm::DINode *node) { - // If the node type is capable of being recursive, check if it's seen - // before. +std::optional DebugImporter::createRecSelf(llvm::DINode *node) { 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); - if (!inserted) { - // The original node may have already been assigned a recursive ID from - // a different self-reference. Use that if possible. - DIRecursiveTypeAttrInterface recSelf = iter->second.recSelf; - if (!recSelf) { - DistinctAttr recId = nodeToRecId.lookup(node); - if (!recId) { - recId = DistinctAttr::create(UnitAttr::get(context)); - nodeToRecId[node] = recId; - } - recSelf = recSelfCtor(recId); - iter->second.recSelf = recSelf; - } - // Inject the self-ref into the previous layer. - translationStack.back().second.unboundSelfRefs.insert(recSelf); - return cast(recSelf); - } + if (!recSelfCtor) + return std::nullopt; + + // The original node may have already been assigned a recursive ID from + // a different self-reference. Use that if possible. + DistinctAttr recId = nodeToRecId.lookup(node); + if (!recId) { + recId = DistinctAttr::create(UnitAttr::get(context)); + nodeToRecId[node] = recId; } - - return lookup(node); -} - -std::pair -DebugImporter::RecursionPruner::finalizeTranslation(llvm::DINode *node, - DINodeAttr result) { - // If `node` is not a potentially recursive type, it will not be on the - // translation stack. Nothing to set in this case. - if (translationStack.empty()) - return {result, true}; - if (translationStack.back().first != node) - return {result, translationStack.back().second.unboundSelfRefs.empty()}; - - TranslationState &state = translationStack.back().second; - - // If this node is actually recursive, set the recId onto `result`. - if (DIRecursiveTypeAttrInterface recSelf = state.recSelf) { - auto recType = cast(result); - result = cast(recType.withRecId(recSelf.getRecId())); - // Remove this recSelf from the set of unbound selfRefs. - state.unboundSelfRefs.erase(recSelf); - } - - // Insert the result into our internal cache if it's not self-contained. - if (!state.unboundSelfRefs.empty()) { - [[maybe_unused]] auto [_, inserted] = dependentCache.try_emplace( - node, DependentTranslation{result, state.unboundSelfRefs}); - assert(inserted && "invalid state: caching the same DINode twice"); - return {result, false}; - } - return {result, true}; -} - -void DebugImporter::RecursionPruner::popTranslationStack(llvm::DINode *node) { - // If `node` is not a potentially recursive type, it will not be on the - // translation stack. Nothing to handle in this case. - if (translationStack.empty() || translationStack.back().first != node) - return; - - // At the end of the stack, all unbound self-refs must be resolved already, - // and the entire cache should be accounted for. - TranslationState &currLayerState = translationStack.back().second; - if (translationStack.size() == 1) { - assert(currLayerState.unboundSelfRefs.empty() && - "internal error: unbound recursive self reference at top level."); - translationStack.pop_back(); - return; - } - - // Copy unboundSelfRefs down to the previous level. - TranslationState &nextLayerState = (++translationStack.rbegin())->second; - nextLayerState.unboundSelfRefs.insert(currLayerState.unboundSelfRefs.begin(), - currLayerState.unboundSelfRefs.end()); - translationStack.pop_back(); -} - -DINodeAttr DebugImporter::RecursionPruner::lookup(llvm::DINode *node) { - auto cacheIter = dependentCache.find(node); - if (cacheIter == dependentCache.end()) - return {}; - - DependentTranslation &entry = cacheIter->second; - if (llvm::set_is_subset(entry.unboundSelfRefs, - translationStack.back().second.unboundSelfRefs)) - return entry.attr; - - // Stale cache entry. - dependentCache.erase(cacheIter); - return {}; + DIRecursiveTypeAttrInterface recSelf = recSelfCtor(recId); + return cast(recSelf); } //===----------------------------------------------------------------------===// diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.h b/mlir/lib/Target/LLVMIR/DebugImporter.h index fac86dbe2cdd22..4a2bf35c160e14 100644 --- a/mlir/lib/Target/LLVMIR/DebugImporter.h +++ b/mlir/lib/Target/LLVMIR/DebugImporter.h @@ -17,6 +17,7 @@ #include "mlir/Dialect/LLVMIR/LLVMDialect.h" #include "mlir/IR/BuiltinOps.h" #include "mlir/IR/MLIRContext.h" +#include "mlir/Support/CyclicReplacerCache.h" #include "llvm/IR/DebugInfoMetadata.h" namespace mlir { @@ -87,102 +88,18 @@ class DebugImporter { /// for it, or create a new one if not. DistinctAttr getOrCreateDistinctID(llvm::DINode *node); - /// A mapping between LLVM debug metadata and the corresponding attribute. - DenseMap nodeToAttr; + std::optional createRecSelf(llvm::DINode *node); + /// A mapping between distinct LLVM debug metadata nodes and the corresponding /// distinct id attribute. DenseMap nodeToDistinctAttr; - /// Translation helper for recursive DINodes. - /// Works alongside a stack-based DINode translator (the "main translator") - /// for gracefully handling DINodes that are recursive. - /// - /// Usage: - /// - Before translating a node, call `pruneOrPushTranslationStack` to see if - /// the pruner can preempt this translation. If this is a node that the - /// pruner already knows how to handle, it will return the translated - /// DINodeAttr. - /// - After a node is successfully translated by the main translator, call - /// `finalizeTranslation` to save the translated result with the pruner, and - /// give it a chance to further modify the result. - /// - Regardless of success or failure by the main translator, always call - /// `popTranslationStack` at the end of translating a node. This is - /// necessary to keep the internal book-keeping in sync. - /// - /// This helper maintains an internal cache so that no recursive type will - /// be translated more than once by the main translator. - /// This internal cache is different from the cache maintained by the main - /// translator because it may store nodes that are not self-contained (i.e. - /// contain unbounded recursive self-references). - class RecursionPruner { - public: - RecursionPruner(MLIRContext *context) : context(context) {} - - /// If this node is a recursive instance that was previously seen, returns a - /// self-reference. If this node was previously cached, returns the cached - /// result. Otherwise, returns null attr, and a translation stack frame is - /// created for this node. Expects `finalizeTranslation` & - /// `popTranslationStack` to be called on this node later. - DINodeAttr pruneOrPushTranslationStack(llvm::DINode *node); - - /// Register the translated result of `node`. Returns the finalized result - /// (with recId if recursive) and whether the result is self-contained - /// (i.e. contains no unbound self-refs). - std::pair finalizeTranslation(llvm::DINode *node, - DINodeAttr result); - - /// Pop off a frame from the translation stack after a node is done being - /// translated. - void popTranslationStack(llvm::DINode *node); - - private: - /// Returns the cached result (if exists) or null. - /// The cache entry will be removed if not all of its dependent self-refs - /// exists. - DINodeAttr lookup(llvm::DINode *node); - - MLIRContext *context; - - /// A cached translation that contains the translated attribute as well - /// as any unbound self-references that it depends on. - struct DependentTranslation { - /// The translated attr. May contain unbound self-references for other - /// recursive attrs. - DINodeAttr attr; - /// The set of unbound self-refs that this cached entry refers to. All - /// these self-refs must exist for the cached entry to be valid. - DenseSet unboundSelfRefs; - }; - /// A mapping between LLVM debug metadata and the corresponding attribute. - /// Only contains those with unboundSelfRefs. Fully self-contained attrs - /// will be cached by the outer main translator. - DenseMap dependentCache; - - /// Each potentially recursive node will have a TranslationState pushed onto - /// the `translationStack` to keep track of whether this node is actually - /// recursive (i.e. has self-references inside), and other book-keeping. - struct TranslationState { - /// The rec-self if this node is indeed a recursive node (i.e. another - /// instance of itself is seen while translating it). Null if this node - /// has not been seen again deeper in the translation stack. - DIRecursiveTypeAttrInterface recSelf; - /// All the unbound recursive self references in this layer of the - /// translation stack. - DenseSet unboundSelfRefs; - }; - /// A stack that stores the metadata nodes that are being traversed. The - /// stack is used to handle cyclic dependencies during metadata translation. - /// Each node is pushed with an empty TranslationState. If it is ever seen - /// later when the stack is deeper, the node is recursive, and its - /// TranslationState is assigned a recSelf. - llvm::MapVector translationStack; - - /// A mapping between DINodes that are recursive, and their assigned recId. - /// This is kept so that repeated occurrences of the same node can reuse the - /// same ID and be deduplicated. - DenseMap nodeToRecId; - }; - RecursionPruner recursionPruner; + /// A mapping between DINodes that are recursive, and their assigned recId. + /// This is kept so that repeated occurrences of the same node can reuse the + /// same ID and be deduplicated. + DenseMap nodeToRecId; + + CyclicReplacerCache cache; MLIRContext *context; ModuleOp mlirModule; diff --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll index 2173cc33e68a4f..a194ecbf2eb204 100644 --- a/mlir/test/Target/LLVMIR/Import/debug-info.ll +++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll @@ -308,16 +308,16 @@ define void @class_method() { } ; 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_INNER:.+]] = #llvm.di_subprogram -; CHECK: #[[COMP:.+]] = #llvm.di_composite_type +; CHECK-DAG: #[[COMP_SELF:.+]] = #llvm.di_composite_type +; CHECK-DAG: #[[COMP_PTR:.+]] = #llvm.di_derived_type +; CHECK-DAG: #[[SP_TYPE:.+]] = #llvm.di_subroutine_type +; CHECK-DAG: #[[SP_INNER:.+]] = #llvm.di_subprogram +; CHECK-DAG: #[[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]]> +; CHECK-DAG: #[[COMP_PTR_OUTER:.+]] = #llvm.di_derived_type +; CHECK-DAG: #[[SP_TYPE_OUTER:.+]] = #llvm.di_subroutine_type +; CHECK-DAG: #[[SP_OUTER:.+]] = #llvm.di_subprogram +; CHECK-DAG: #[[LOC]] = loc(fused<#[[SP_OUTER]]> !llvm.dbg.cu = !{!1} !llvm.module.flags = !{!0} @@ -335,12 +335,12 @@ define void @class_method() { ; // ----- ; 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-DAG: #[[COMP_SELF:.+]] = #llvm.di_composite_type +; CHECK-DAG: #[[COMP_PTR_INNER:.+]] = #llvm.di_derived_type +; CHECK-DAG: #[[FIELD:.+]] = #llvm.di_derived_type +; CHECK-DAG: #[[COMP:.+]] = #llvm.di_composite_type +; CHECK-DAG: #[[COMP_PTR_OUTER:.+]] = #llvm.di_derived_type +; CHECK-DAG: #[[VAR0:.+]] = #llvm.di_local_variable ; CHECK: @class_field ; CHECK-SAME: %[[ARG0:[a-zA-Z0-9]+]] @@ -727,9 +727,7 @@ define void @class_field(ptr %arg1) !dbg !18 { ; CHECK-DAG: #[[B_TO_C]] = #llvm.di_derived_type<{{.*}}name = "->C", {{.*}}baseType = #[[C_FROM_B:.+]]> ; CHECK-DAG: #[[C_FROM_B]] = #llvm.di_composite_type<{{.*}}recId = [[C_RECID:.+]], {{.*}}name = "C", {{.*}}elements = #[[TO_A_SELF:.+]], #[[TO_B_SELF:.+]], #[[TO_C_SELF:.+]]> -; CHECK-DAG: #[[C_FROM_A]] = #llvm.di_composite_type<{{.*}}recId = [[C_RECID]], {{.*}}name = "C", {{.*}}elements = #[[TO_A_SELF]], #[[TO_B_INNER:.+]], #[[TO_C_SELF]] -; CHECK-DAG: #[[TO_B_INNER]] = #llvm.di_derived_type<{{.*}}name = "->B", {{.*}}baseType = #[[B_INNER:.+]]> -; CHECK-DAG: #[[B_INNER]] = #llvm.di_composite_type<{{.*}}name = "B", {{.*}}elements = #[[TO_C_SELF]]> +; CHECK-DAG: #[[C_FROM_A]] = #llvm.di_composite_type<{{.*}}recId = [[C_RECID]], {{.*}}name = "C", {{.*}}elements = #[[TO_A_SELF]], #[[A_TO_B:.+]], #[[TO_C_SELF]] ; CHECK-DAG: #[[TO_A_SELF]] = #llvm.di_derived_type<{{.*}}name = "->A", {{.*}}baseType = #[[A_SELF:.+]]> ; CHECK-DAG: #[[TO_B_SELF]] = #llvm.di_derived_type<{{.*}}name = "->B", {{.*}}baseType = #[[B_SELF:.+]]>