From 46e9b214329de820368ecf2956bd0c3aff43cf36 Mon Sep 17 00:00:00 2001 From: Andrew Young Date: Thu, 27 Oct 2022 09:57:10 -0700 Subject: [PATCH] [FIRRTL] Dedup speedup (#4197) This change consists of a couple improvements to the deduplication pass, the first two of which increase performance. 1. Adds an NLA Cache, which maps a namepath attribute to an HierPathOp which matches. Dedup uses this to reuse HierPathOps in the circuit instead of creating new ones. This used to cause problem by passes which assumed ops did not share NLAs. This has an added effect of doing some HierPathOp garbage collection when we delete a module. 2. The targetMap, which maps an NLA to the operations which use it, now uses an SmallDenseSet. The original logic could end up recording the same target multiple times. AnnoTarget was made hashable for this. 3. Annotation merging used to merge identical annotations, now it keeps both annotations and makes them non-local. The original behaviour is kept for only DontTouch annotations, where it will only keep one. 4. When a module is merged into another, the original modules annotations are now first instead of second in the annotation list. 5. Some simplification of functions, especially unused argument removal, and creating functions for common code. --- .../circt/Dialect/FIRRTL/FIRRTLAnnotations.h | 22 ++ lib/Dialect/FIRRTL/Transforms/Dedup.cpp | 279 +++++++++--------- test/Dialect/FIRRTL/SFCTests/dedup.fir | 31 +- test/Dialect/FIRRTL/dedup.mlir | 79 ++--- 4 files changed, 217 insertions(+), 194 deletions(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h b/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h index d13415d4c361..14f79b72dbae 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h +++ b/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h @@ -535,6 +535,28 @@ struct DenseMapInfo { static bool isEqual(Annotation LHS, Annotation RHS) { return LHS == RHS; } }; +/// Make `AnnoTarget` hash. +template <> +struct DenseMapInfo { + using AnnoTarget = circt::firrtl::AnnoTarget; + using AnnoTargetImpl = circt::firrtl::detail::AnnoTargetImpl; + static AnnoTarget getEmptyKey() { + auto *o = DenseMapInfo::getEmptyKey(); + auto i = DenseMapInfo::getEmptyKey(); + return AnnoTarget(AnnoTargetImpl(o, i)); + } + static AnnoTarget getTombstoneKey() { + auto *o = DenseMapInfo::getTombstoneKey(); + auto i = DenseMapInfo::getTombstoneKey(); + return AnnoTarget(AnnoTargetImpl(o, i)); + } + static unsigned getHashValue(AnnoTarget val) { + auto impl = val.getImpl(); + return hash_combine(impl.getOp(), impl.getPortNo()); + } + static bool isEqual(AnnoTarget lhs, AnnoTarget rhs) { return lhs == rhs; } +}; + } // namespace llvm #endif // CIRCT_DIALECT_FIRRTL_ANNOTATIONS_H diff --git a/lib/Dialect/FIRRTL/Transforms/Dedup.cpp b/lib/Dialect/FIRRTL/Transforms/Dedup.cpp index 666dd8ad65dd..ca5cf208db45 100644 --- a/lib/Dialect/FIRRTL/Transforms/Dedup.cpp +++ b/lib/Dialect/FIRRTL/Transforms/Dedup.cpp @@ -641,7 +641,11 @@ struct Deduper { symbolTable(symbolTable), nlaTable(nlaTable), nlaBlock(circuit.getBodyBlock()), nonLocalString(StringAttr::get(context, "circt.nonlocal")), - classString(StringAttr::get(context, "class")) {} + classString(StringAttr::get(context, "class")) { + // Populate the NLA cache. + for (auto nla : circuit.getOps()) + nlaCache[nla.getNamepathAttr()] = nla.getSymNameAttr(); + } /// Remove the "fromModule", and replace all references to it with the /// "toModule". Modules should be deduplicated in a bottom-up order. Any @@ -652,21 +656,19 @@ struct Deduper { // used to update NLAs that reference the "fromModule". RenameMap renameMap; - // This is a lazily constructed set of NLAs used to turn a local - // annotation into non-local annotations. - SmallVector toNLAs; - SmallVector fromNLAs; - // Merge the two modules. - mergeOps(renameMap, toModule, toNLAs, toModule, fromModule, fromNLAs, - fromModule); + mergeOps(renameMap, toModule, toModule, fromModule, fromModule); - // Rewrite NLAs pathing through these modules to refer to the to module. + // Rewrite NLAs pathing through these modules to refer to the to module. It + // is safe to do this at this point because NLAs cannot be one element long. + // This means that all NLAs which require more context cannot be targetting + // something in the module it self. if (auto to = dyn_cast(*toModule)) rewriteModuleNLAs(renameMap, to, cast(*fromModule)); else rewriteExtModuleNLAs(renameMap, toModule.moduleNameAttr(), fromModule.moduleNameAttr()); + replaceInstances(toModule, fromModule); } @@ -676,11 +678,8 @@ struct Deduper { // Record any annotations on the module. recordAnnotations(module); // Record port annotations. - for (const auto &pair : llvm::enumerate(module.getPortAnnotations())) - for (auto anno : AnnotationSet(pair.value().cast())) - if (auto nlaRef = anno.getMember("circt.nonlocal")) - targetMap[nlaRef.getAttr()].push_back( - PortAnnoTarget(module, pair.index())); + for (unsigned i = 0, e = module.getNumPorts(); i < e; ++i) + recordAnnotations(PortAnnoTarget(module, i)); // Record any annotations in the module body. module->walk([&](Operation *op) { recordAnnotations(op); }); } @@ -692,25 +691,27 @@ struct Deduper { return it->second; } + /// For a specific annotation target, record all the unique NLAs which + /// target it in the `targetMap`. + void recordAnnotations(AnnoTarget target) { + for (auto anno : target.getAnnotations()) + if (auto nlaRef = anno.getMember("circt.nonlocal")) + targetMap[nlaRef.getAttr()].insert(target); + } + /// Record all targets which use an NLA. void recordAnnotations(Operation *op) { // Record annotations. - for (auto anno : AnnotationSet(op)) - if (auto nlaRef = anno.getMember("circt.nonlocal")) - targetMap[nlaRef.getAttr()].push_back(OpAnnoTarget(op)); + recordAnnotations(OpAnnoTarget(op)); // Record port annotations only if this is a mem operation. auto mem = dyn_cast(op); if (!mem) return; - // Record port annotations. Breadcrumbs don't appear on port annotations, so - // we can skip the class check that we have above. - for (const auto &pair : llvm::enumerate(mem.getPortAnnotations())) - for (auto anno : AnnotationSet(pair.value().cast())) - if (auto nlaRef = anno.getMember("circt.nonlocal")) - targetMap[nlaRef.getAttr()].push_back( - PortAnnoTarget(mem, pair.index())); + // Record port annotations. + for (unsigned i = 0, e = mem->getNumResults(); i < e; ++i) + recordAnnotations(PortAnnoTarget(mem, i)); } /// This deletes and replaces all instances of the "fromModule" with instances @@ -731,12 +732,13 @@ struct Deduper { fromModule->erase(); } - /// Look up the instantiations of this module and create an NLA for each + /// Look up the instantiations of the `from` module and create an NLA for each /// one, appending the baseNamepath to each NLA. This is used to add more - /// context to an already existing NLA. + /// context to an already existing NLA. The `fromModule` is used to indicate + /// which module the annotation is coming from before the merge, and will be + /// used to create the namepaths. SmallVector - createNLAs(StringAttr toModuleName, Operation *fromModule, - ArrayRef baseNamepath, + createNLAs(Operation *fromModule, ArrayRef baseNamepath, SymbolTable::Visibility vis = SymbolTable::Visibility::Private) { // Create an attribute array with a placeholder in the first element, where // the root refence of the NLA will be inserted. @@ -750,15 +752,20 @@ struct Deduper { auto inst = instanceRecord->getInstance(); namepath[0] = OpAnnoTarget(inst).getNLAReference(getNamespace(parent)); auto arrayAttr = ArrayAttr::get(context, namepath); - auto nla = OpBuilder::atBlockBegin(nlaBlock).create( - loc, "nla", arrayAttr); - // Insert it into the symbol table to get a unique name. - symbolTable.insert(nla); - auto nlaName = nla.getNameAttr(); - auto nlaRef = FlatSymbolRefAttr::get(nlaName); + // Check the NLA cache to see if we already have this NLA. + auto &cacheEntry = nlaCache[arrayAttr]; + if (!cacheEntry) { + auto nla = OpBuilder::atBlockBegin(nlaBlock).create( + loc, "nla", arrayAttr); + // Insert it into the symbol table to get a unique name. + symbolTable.insert(nla); + // Store it in the cache. + cacheEntry = nla.getNameAttr(); + nla.setVisibility(vis); + nlaTable->addNLA(nla); + } + auto nlaRef = FlatSymbolRefAttr::get(cacheEntry.cast()); nlas.push_back(nlaRef); - nla.setVisibility(vis); - nlaTable->addNLA(nla); } return nlas; } @@ -767,11 +774,9 @@ struct Deduper { /// This returns an array of symbol references which can be used to reference /// the NLAs. SmallVector - createNLAs(FModuleLike toModule, StringAttr toModuleName, - FModuleLike fromModule, + createNLAs(StringAttr toModuleName, FModuleLike fromModule, SymbolTable::Visibility vis = SymbolTable::Visibility::Private) { - return createNLAs(toModuleName, fromModule, - FlatSymbolRefAttr::get(toModule.moduleNameAttr()), vis); + return createNLAs(fromModule, FlatSymbolRefAttr::get(toModuleName), vis); } /// Clone the annotation for each NLA in a list. The attribute list should @@ -780,7 +785,7 @@ struct Deduper { void cloneAnnotation(SmallVectorImpl &nlas, Annotation anno, ArrayRef attributes, unsigned nonLocalIndex, - SmallVector &newAnnotations) { + SmallVectorImpl &newAnnotations) { SmallVector mutableAttributes(attributes.begin(), attributes.end()); for (auto &nla : nlas) { @@ -793,13 +798,14 @@ struct Deduper { } } - /// This erases the NLA op, all breadcrumb trails, and removes the NLA from - /// every module's NLA map, but it does not delete the NLA reference from - /// the target operation's annotations. + /// This erases the NLA op, and removes the NLA from every module's NLA map, + /// but it does not delete the NLA reference from the target operation's + /// annotations. void eraseNLA(HierPathOp nla) { // Erase the NLA from the leaf module's nlaMap. targetMap.erase(nla.getNameAttr()); nlaTable->erase(nla); + nlaCache.erase(nla.getNamepathAttr()); symbolTable.erase(nla); } @@ -814,6 +820,8 @@ struct Deduper { auto moduleNLAs = nlaTable->lookup(fromModule.getNameAttr()).vec(); // Change the NLA to target the toModule. nlaTable->renameModuleAndInnerRef(toName, fromName, renameMap); + // Now we walk the NLA searching for ones that require more context to be + // added. for (auto nla : moduleNLAs) { auto elements = nla.getNamepath().getValue(); // If we don't need to add more context, we're done here. @@ -821,10 +829,12 @@ struct Deduper { continue; // Create the replacement NLAs. SmallVector namepath(elements.begin(), elements.end()); - auto nlaRefs = - createNLAs(toName, fromModule, namepath, nla.getVisibility()); + auto nlaRefs = createNLAs(fromModule, namepath, nla.getVisibility()); + // Copy out the targets, because we will be updating the map. + auto &set = targetMap[nla.getSymNameAttr()]; + SmallVector targets(set.begin(), set.end()); // Replace the uses of the old NLA with the new NLAs. - for (auto target : targetMap[nla.getSymNameAttr()]) { + for (auto target : targets) { // We have to clone any annotation which uses the old NLA for each new // NLA. This array collects the new set of annotations. SmallVector newAnnotations; @@ -844,12 +854,13 @@ struct Deduper { cloneAnnotation(nlaRefs, anno, ArrayRef(anno.begin(), anno.end()), nonLocalIndex, newAnnotations); } + // Apply the new annotations to the operation. AnnotationSet annotations(newAnnotations, context); target.setAnnotations(annotations); // Record that target uses the NLA. for (auto nla : nlaRefs) - targetMap[nla.getAttr()].push_back(target); + targetMap[nla.getAttr()].insert(target); } // Erase the old NLA and remove it from all breadcrumbs. @@ -875,12 +886,10 @@ struct Deduper { /// Take an annotation, and update it to be a non-local annotation. If the /// annotation is already non-local and has enough context, it will be skipped - /// for now. - void makeAnnotationNonLocal(SmallVectorImpl &nlaRefs, - FModuleLike toModule, StringAttr toModuleName, - AnnoTarget to, FModuleLike fromModule, - AnnoTarget from, Annotation anno, - SmallVector &newAnnotations) { + /// for now. Return true if the annotation was made non-local. + bool makeAnnotationNonLocal(StringAttr toModuleName, AnnoTarget to, + FModuleLike fromModule, Annotation anno, + SmallVectorImpl &newAnnotations) { // Start constructing a new annotation, pushing a "circt.nonLocal" field // into the correct spot if its not already a non-local annotation. SmallVector attributes; @@ -889,24 +898,20 @@ struct Deduper { auto attr = val.value(); // Is this field "circt.nonlocal"? auto compare = attr.getName().compare(nonLocalString); - if (compare == 0) { - // This annotation is already a non-local annotation. Record that this - // operation uses that NLA and stop processing this annotation. - auto nlaName = attr.getValue().cast().getAttr(); - targetMap[nlaName].push_back(from); - newAnnotations.push_back(anno); - return; - } + assert(compare != 0 && "should not pass non-local annotations here"); if (compare == 1) { - // Push an empty place holder for the non-local annotation. + // This annotation definitely does not have "circt.nonlocal" field. Push + // an empty place holder for the non-local annotation. nonLocalIndex = val.index(); attributes.push_back(NamedAttribute(nonLocalString, nonLocalString)); break; } + // Otherwise push the current attribute and keep searching for the + // "circt.nonlocal" field. attributes.push_back(attr); } if (nonLocalIndex == -1) { - // Push the "circt.nonlocal" to the last slot. + // Push an empty "circt.nonlocal" field to the last slot. nonLocalIndex = attributes.size(); attributes.push_back(NamedAttribute(nonLocalString, nonLocalString)); } else { @@ -915,60 +920,60 @@ struct Deduper { } // Construct the NLAs if we don't have any yet. - if (nlaRefs.empty()) - nlaRefs = createNLAs(toModule, toModuleName, fromModule); + auto nlaRefs = createNLAs(toModuleName, fromModule); + for (auto nla : nlaRefs) + targetMap[nla.getAttr()].insert(to); // Clone the annotation for each new NLA. cloneAnnotation(nlaRefs, anno, attributes, nonLocalIndex, newAnnotations); - for (auto nla : nlaRefs) - targetMap[nla.getAttr()].push_back(to); + return true; } - /// Merge the annotations of a specific target, either a operation or a port - /// on an operation. - void mergeAnnotations(FModuleLike toModule, - SmallVectorImpl &toNLAs, - AnnoTarget to, AnnotationSet toAnnos, - FModuleLike fromModule, - SmallVectorImpl &fromNLAs, - AnnoTarget from, AnnotationSet fromAnnos) { - // We want to make sure all NLAs are put right above the first module. - SmallVector newAnnotations; - SmallVector alreadyHandled; - for (auto anno : fromAnnos) { - // If the ops have the same annotation, we don't have to turn it into a - // non-local annotation. - auto found = llvm::find(toAnnos, anno); - if (found != toAnnos.end()) { - alreadyHandled.push_back(std::distance(toAnnos.begin(), found)); + void copyAnnotations(FModuleLike toModule, AnnoTarget to, + FModuleLike fromModule, AnnotationSet annos, + SmallVectorImpl &newAnnotations, + bool &hasDontTouch) { + for (auto anno : annos) { + if (anno.isClass(dontTouchAnnoClass)) { + // Skip this if we already have a DontTouch. + if (hasDontTouch) + continue; + // We don't worry if the DontTouch is non-local or not, we just copy it + // as is since it doesn't matter. newAnnotations.push_back(anno); + hasDontTouch = true; continue; } - makeAnnotationNonLocal(toNLAs, toModule, toModule.moduleNameAttr(), to, - fromModule, from, anno, newAnnotations); - } - - // This is a helper to skip already handled annotations. - auto *it = alreadyHandled.begin(); - auto *end = alreadyHandled.end(); - auto getNextHandledIndex = [&]() -> unsigned { - if (it == end) - return -1; - return *(it++); - }; - auto index = getNextHandledIndex(); - - // Merge annotations from the other op, skipping the ones already handled. - for (const auto &pair : llvm::enumerate(toAnnos)) { - // If its already handled, skip it. - if (pair.index() == index) { - index = getNextHandledIndex(); + // If the annotation is already non-local, we add it as is. It is already + // added to the target map. + if (anno.getMember("circt.nonlocal")) { + newAnnotations.push_back(anno); continue; } - auto anno = pair.value(); - makeAnnotationNonLocal(fromNLAs, toModule, toModule.moduleNameAttr(), to, - toModule, to, anno, newAnnotations); + // Otherwise make the annotation non-local and add it to the set. + makeAnnotationNonLocal(toModule.moduleNameAttr(), to, fromModule, anno, + newAnnotations); } + } + + /// Merge the annotations of a specific target, either a operation or a port + /// on an operation. + void mergeAnnotations(FModuleLike toModule, AnnoTarget to, + AnnotationSet toAnnos, FModuleLike fromModule, + AnnoTarget from, AnnotationSet fromAnnos) { + // This is a list of all the annotations which will be added to `to`. + SmallVector newAnnotations; + + // We have special case handling of DontTouch to prevent it from being + // turned into a non-local annotation. + bool hasDontTouch = false; + + // Iterate the annotations, transforming most annotations into non-local + // ones. + copyAnnotations(toModule, to, toModule, toAnnos, newAnnotations, + hasDontTouch); + copyAnnotations(toModule, to, fromModule, fromAnnos, newAnnotations, + hasDontTouch); // Copy over all the new annotations. if (!newAnnotations.empty()) @@ -976,30 +981,26 @@ struct Deduper { } /// Merge all annotations and port annotations on two operations. - void mergeAnnotations(FModuleLike toModule, - SmallVectorImpl &toNLAs, - Operation *to, FModuleLike fromModule, - SmallVectorImpl &fromNLAs, - Operation *from) { + void mergeAnnotations(FModuleLike toModule, Operation *to, + FModuleLike fromModule, Operation *from) { // Merge op annotations. - mergeAnnotations(toModule, toNLAs, OpAnnoTarget(to), AnnotationSet(to), - fromModule, fromNLAs, OpAnnoTarget(to), - AnnotationSet(from)); + mergeAnnotations(toModule, OpAnnoTarget(to), AnnotationSet(to), fromModule, + OpAnnoTarget(from), AnnotationSet(from)); // Merge port annotations. if (toModule == to) { // Merge module port annotations. for (unsigned i = 0, e = toModule.getNumPorts(); i < e; ++i) - mergeAnnotations(toModule, toNLAs, PortAnnoTarget(toModule, i), + mergeAnnotations(toModule, PortAnnoTarget(toModule, i), AnnotationSet::forPort(toModule, i), fromModule, - fromNLAs, PortAnnoTarget(fromModule, i), + PortAnnoTarget(fromModule, i), AnnotationSet::forPort(fromModule, i)); } else if (auto toMem = dyn_cast(to)) { // Merge memory port annotations. auto fromMem = cast(from); for (unsigned i = 0, e = toMem.getNumResults(); i < e; ++i) - mergeAnnotations(toModule, toNLAs, PortAnnoTarget(toMem, i), - AnnotationSet::forPort(toMem, i), fromModule, fromNLAs, + mergeAnnotations(toModule, PortAnnoTarget(toMem, i), + AnnotationSet::forPort(toMem, i), fromModule, PortAnnoTarget(fromMem, i), AnnotationSet::forPort(fromMem, i)); } @@ -1066,47 +1067,39 @@ struct Deduper { /// Recursively merge two operations. // NOLINTNEXTLINE(misc-no-recursion) - void mergeOps(RenameMap &renameMap, FModuleLike toModule, - SmallVectorImpl &toNLAs, Operation *to, - FModuleLike fromModule, - SmallVectorImpl &fromNLAs, Operation *from) { + void mergeOps(RenameMap &renameMap, FModuleLike toModule, Operation *to, + FModuleLike fromModule, Operation *from) { // Merge the operation locations. if (to->getLoc() != from->getLoc()) to->setLoc(mergeLoc(context, to->getLoc(), from->getLoc())); // Recurse into any regions. for (auto regions : llvm::zip(to->getRegions(), from->getRegions())) - mergeRegions(renameMap, toModule, toNLAs, std::get<0>(regions), - fromModule, fromNLAs, std::get<1>(regions)); + mergeRegions(renameMap, toModule, std::get<0>(regions), fromModule, + std::get<1>(regions)); // Record any inner_sym renamings that happened. - if (to != from) - recordSymRenames(renameMap, toModule, to, fromModule, from); + recordSymRenames(renameMap, toModule, to, fromModule, from); // Merge the annotations. - mergeAnnotations(toModule, toNLAs, to, fromModule, fromNLAs, from); + mergeAnnotations(toModule, to, fromModule, from); } /// Recursively merge two blocks. - void mergeBlocks(RenameMap &renameMap, FModuleLike toModule, - SmallVectorImpl &toNLAs, Block &toBlock, - FModuleLike fromModule, - SmallVectorImpl &fromNLAs, - Block &fromBlock) { + void mergeBlocks(RenameMap &renameMap, FModuleLike toModule, Block &toBlock, + FModuleLike fromModule, Block &fromBlock) { for (auto ops : llvm::zip(toBlock, fromBlock)) - mergeOps(renameMap, toModule, toNLAs, &std::get<0>(ops), fromModule, - fromNLAs, &std::get<1>(ops)); + mergeOps(renameMap, toModule, &std::get<0>(ops), fromModule, + &std::get<1>(ops)); } // Recursively merge two regions. void mergeRegions(RenameMap &renameMap, FModuleLike toModule, - SmallVectorImpl &toNLAs, Region &toRegion, FModuleLike fromModule, - SmallVectorImpl &fromNLAs, Region &fromRegion) { for (auto blocks : llvm::zip(toRegion, fromRegion)) - mergeBlocks(renameMap, toModule, toNLAs, std::get<0>(blocks), fromModule, - fromNLAs, std::get<1>(blocks)); + mergeBlocks(renameMap, toModule, std::get<0>(blocks), fromModule, + std::get<1>(blocks)); } MLIRContext *context; @@ -1119,8 +1112,12 @@ struct Deduper { /// We insert all NLAs to the beginning of this block. Block *nlaBlock; - // This maps an NLA to the operations and ports that uses it. - DenseMap> targetMap; + // This maps an NLA to the operations and ports that uses it. + DenseMap> targetMap; + + // This is a cache to avoid creating duplicate NLAs. This maps the ArrayAtr + // of the NLA's path to the name of the NLA which contains it. + DenseMap nlaCache; // Cached attributes for faster comparisons and attribute building. StringAttr nonLocalString; diff --git a/test/Dialect/FIRRTL/SFCTests/dedup.fir b/test/Dialect/FIRRTL/SFCTests/dedup.fir index ed7ca6a49be4..38bb0611fba1 100644 --- a/test/Dialect/FIRRTL/SFCTests/dedup.fir +++ b/test/Dialect/FIRRTL/SFCTests/dedup.fir @@ -356,8 +356,8 @@ circuit Top : %[[ "target":"~Top|A_>b" } ]] - ; CHECK: firrtl.hierpath private @[[nlaSym1:[_a-zA-Z0-9]+]] [@Top::@[[a1Sym:[_a-zA-Z0-9]+]], @A] ; CHECK: firrtl.hierpath private @[[nlaSym2:[_a-zA-Z0-9]+]] [@Top::@[[a2Sym:[_a-zA-Z0-9]+]], @A] + ; CHECK: firrtl.hierpath private @[[nlaSym1:[_a-zA-Z0-9]+]] [@Top::@[[a1Sym:[_a-zA-Z0-9]+]], @A] ; CHECK: firrtl.module @Top ; CHECK-NEXT: firrtl.instance a1 sym @[[a1Sym]] @A( ; CHECK-NEXT: firrtl.instance a2 sym @[[a2Sym]] @A( @@ -380,8 +380,7 @@ circuit Top : %[[ ; // ----- ; "The deduping module A and A_ should rename internal signals that have -; different names". This is checking that an NLA is not created even though -; structural deduplication changes the name of "A_>b" to "A>a". +; different names". ; ; CHECK-LABEL: firrtl.circuit "Top" circuit Top : %[[ @@ -397,6 +396,8 @@ circuit Top : %[[ } ]] ; CHECK-NOT: firrtl.hierpath + ; CHECK: firrtl.hierpath private @[[nlaSym2:[_a-zA-Z0-9]+]] [@Top::@[[a1sym:[_a-zA-Z0-9]+]], @A] + ; CHECK: firrtl.hierpath private @[[nlaSym1:[_a-zA-Z0-9]+]] [@Top::@[[a1Sym]], @A] module Top : inst a1 of A a1 is invalid @@ -406,7 +407,7 @@ circuit Top : %[[ module A : input x: UInt<1> output y: UInt<1> - ; CHECK: firrtl.node {{.+}} {annotations = [{class = "circt.test", data = "hello"}]} + ; CHECK: firrtl.node {{.+}} {annotations = [{circt.nonlocal = @[[nlaSym1]], class = "circt.test", data = "hello"}, {circt.nonlocal = @[[nlaSym2]], class = "circt.test", data = "hello"}]} node a = add(x, UInt(1)) y <= add(a, a) module A_ : @@ -475,12 +476,12 @@ circuit Top : %[[ module A_ : inst b_ of B_ ; CHECK: firrtl.module private @B - ; CHECK-SAME: {circt.nonlocal = @[[nla_a_]], class = "circt.test", data = "B_"} ; CHECK-SAME: {circt.nonlocal = @[[nla_a]], class = "circt.test", data = "B"} + ; CHECK-SAME: {circt.nonlocal = @[[nla_a_]], class = "circt.test", data = "B_"} module B : ; CHECK: firrtl.node - ; CHECK-SAME: {circt.nonlocal = @[[nla_a_]], class = "circt.test", data = "B_.bar"} ; CHECK-SAME: {circt.nonlocal = @[[nla_a]], class = "circt.test", data = "B.foo"} + ; CHECK-SAME: {circt.nonlocal = @[[nla_a_]], class = "circt.test", data = "B_.bar"} node foo = UInt<1>(0) ; CHECK-NOT: firrtl.module private @B_ module B_ : @@ -717,12 +718,12 @@ circuit Top : %[[ inst b_ of B_ ; CHECK: firrtl.module private @B() - ; CHECK-SAME: [{circt.nonlocal = @[[nla_2]], class = "circt.test", data = "nla2"}, - ; CHECK-SAME: {circt.nonlocal = @[[nla_3]], class = "circt.test", data = "nla3"}, - ; CHECK-SAME: {circt.nonlocal = @[[nla_4]], class = "circt.test", data = "nla4"}, - ; CHECK-SAME: {circt.nonlocal = @[[nla_1]], class = "circt.test", data = "nla1"}, + ; CHECK-SAME: [{circt.nonlocal = @[[nla_1]], class = "circt.test", data = "nla1"}, ; CHECK-SAME: {circt.nonlocal = @[[nla_5]], class = "circt.test", data = "nla5"}, - ; CHECK-SAME: {circt.nonlocal = @[[nla_6]], class = "circt.test", data = "nla6"}] + ; CHECK-SAME: {circt.nonlocal = @[[nla_6]], class = "circt.test", data = "nla6"}, + ; CHECK-SAME: {circt.nonlocal = @[[nla_2]], class = "circt.test", data = "nla2"}, + ; CHECK-SAME: {circt.nonlocal = @[[nla_3]], class = "circt.test", data = "nla3"}, + ; CHECK-SAME: {circt.nonlocal = @[[nla_4]], class = "circt.test", data = "nla4"}] module B : ; CHECK-NEXT: firrtl.instance c sym @[[BcSym]] ; CHECK-SAME: @C() @@ -763,10 +764,10 @@ circuit top : %[[ "target":"~top|b>q.a" } ]] - ; CHECK: firrtl.hierpath private @[[nla_1:[_a-zA-Z0-9]+]] - ; CHECK-SAME: [@top::@[[topaSym:[_a-zA-Z0-9]+]], @a] ; CHECK: firrtl.hierpath private @[[nla_2:[_a-zA-Z0-9]+]] ; CHECK-SAME: [@top::@[[topbSym:[_a-zA-Z0-9]+]], @a] + ; CHECK: firrtl.hierpath private @[[nla_1:[_a-zA-Z0-9]+]] + ; CHECK-SAME: [@top::@[[topaSym:[_a-zA-Z0-9]+]], @a] ; CHECK: firrtl.module @top module top: input ia: {z: {y: {x: UInt<1>}}, a: UInt<1>} @@ -789,8 +790,8 @@ circuit top : %[[ ; CHECK: firrtl.module private @a( ; CHECK-SAME: in %i: ; CHECK-NOT: out - ; CHECK-SAME: [{circt.fieldID = 1 : i32, circt.nonlocal = @[[nla_2]], class = "circt.test", data = "nla2"}, - ; CHECK-SAME: {circt.fieldID = 4 : i32, circt.nonlocal = @[[nla_1]], class = "circt.test", data = "nla1"}] + ; CHECK-SAME: [{circt.fieldID = 4 : i32, circt.nonlocal = @[[nla_1]], class = "circt.test", data = "nla1"}, + ; CHECK-SAME: {circt.fieldID = 1 : i32, circt.nonlocal = @[[nla_2]], class = "circt.test", data = "nla2"}] module a: input i: {z: {y: {x: UInt<1>}}, a: UInt<1>} output o: {z: {y: {x: UInt<1>}}, a: UInt<1>} diff --git a/test/Dialect/FIRRTL/dedup.mlir b/test/Dialect/FIRRTL/dedup.mlir index 2903aea50927..cc7ae62d1baf 100644 --- a/test/Dialect/FIRRTL/dedup.mlir +++ b/test/Dialect/FIRRTL/dedup.mlir @@ -91,51 +91,54 @@ firrtl.circuit "WhenOps" { // CHECK-LABEL: firrtl.circuit "Annotations" firrtl.circuit "Annotations" { - // CHECK: firrtl.hierpath private [[NLA0:@nla.*]] [@Annotations::@annotations0, @Annotations0] - // CHECK: firrtl.hierpath private [[NLA1:@nla.*]] [@Annotations::@annotations1, @Annotations0] - // CHECK: firrtl.hierpath private @annos_nla0 [@Annotations::@annotations0, @Annotations0::@d] - firrtl.hierpath private @annos_nla0 [@Annotations::@annotations0, @Annotations0::@d] - // CHECK: firrtl.hierpath private @annos_nla1 [@Annotations::@annotations1, @Annotations0::@d] - firrtl.hierpath private @annos_nla1 [@Annotations::@annotations1, @Annotations1::@i] + // CHECK: firrtl.hierpath private [[NLA0:@nla.*]] [@Annotations::@annotations1, @Annotations0] + // CHECK: firrtl.hierpath private @annos_nla0 [@Annotations::@annotations0, @Annotations0::@c] + firrtl.hierpath private @annos_nla0 [@Annotations::@annotations0, @Annotations0::@c] + // CHECK: firrtl.hierpath private @annos_nla1 [@Annotations::@annotations1, @Annotations0::@c] + firrtl.hierpath private @annos_nla1 [@Annotations::@annotations1, @Annotations1::@j] // CHECK: firrtl.hierpath private @annos_nla2 [@Annotations::@annotations0, @Annotations0] firrtl.hierpath private @annos_nla2 [@Annotations::@annotations0, @Annotations0] - // CHECK: firrtl.hierpath private @annos_nla3 [@Annotations::@annotations0, @Annotations0] - firrtl.hierpath private @annos_nla3 [@Annotations::@annotations0, @Annotations0] - // CHECK: firrtl.module @Annotations0() attributes {annotations = [{circt.nonlocal = [[NLA1]], class = "one"}]} + // CHECK: firrtl.module @Annotations0() attributes {annotations = [{circt.nonlocal = [[NLA0]], class = "one"}]} firrtl.module @Annotations0() { - // Same annotation on both ops should stay local. - // CHECK: %a = firrtl.wire {annotations = [{class = "both"}]} - %a = firrtl.wire {annotations = [{class = "both"}]} : !firrtl.uint<1> - // Annotation from other module becomes non-local. - // CHECK: %b = firrtl.wire {annotations = [{circt.nonlocal = [[NLA1]], class = "one"}]} - %b = firrtl.wire : !firrtl.uint<1> + // CHECK: %a = firrtl.wire {annotations = [{circt.nonlocal = [[NLA0]], class = "one"}]} + %a = firrtl.wire : !firrtl.uint<1> // Annotation from this module becomes non-local. - // CHECK: %c = firrtl.wire {annotations = [{circt.nonlocal = [[NLA0]], class = "one"}]} - %c = firrtl.wire {annotations = [{class = "one"}]} : !firrtl.uint<1> + // CHECK: %b = firrtl.wire {annotations = [{circt.nonlocal = @annos_nla2, class = "one"}]} + %b = firrtl.wire {annotations = [{class = "one"}]} : !firrtl.uint<1> // Two non-local annotations are unchanged, as they have enough context in the NLA already. - // CHECK: %d = firrtl.wire sym @d {annotations = [{circt.nonlocal = @annos_nla1, class = "NonLocal"}, {circt.nonlocal = @annos_nla0, class = "NonLocal"}]} - %d = firrtl.wire sym @d {annotations = [{circt.nonlocal = @annos_nla0, class = "NonLocal"}]} : !firrtl.uint<1> + // CHECK: %c = firrtl.wire sym @c {annotations = [{circt.nonlocal = @annos_nla0, class = "NonLocal"}, {circt.nonlocal = @annos_nla1, class = "NonLocal"}]} + %c = firrtl.wire sym @c {annotations = [{circt.nonlocal = @annos_nla0, class = "NonLocal"}]} : !firrtl.uint<1> // Same test as above but with the hiearchical path targeting the module. - // CHECK: %e = firrtl.wire {annotations = [{circt.nonlocal = @annos_nla3, class = "NonLocal"}, {circt.nonlocal = @annos_nla2, class = "NonLocal"}]} - %e = firrtl.wire {annotations = [{circt.nonlocal = @annos_nla2, class = "NonLocal"}]} : !firrtl.uint<1> + // CHECK: %d = firrtl.wire {annotations = [{circt.nonlocal = @annos_nla2, class = "NonLocal"}, {circt.nonlocal = @annos_nla2, class = "NonLocal"}]} + %d = firrtl.wire {annotations = [{circt.nonlocal = @annos_nla2, class = "NonLocal"}]} : !firrtl.uint<1> + + // Same annotation on both ops should become non-local. + // CHECK: %e = firrtl.wire {annotations = [{circt.nonlocal = @annos_nla2, class = "both"}, {circt.nonlocal = [[NLA0]], class = "both"}]} + %e = firrtl.wire {annotations = [{class = "both"}]} : !firrtl.uint<1> + + // Dont touch on both ops should become local. + // CHECK: %f = firrtl.wire {annotations = [{class = "firrtl.transforms.DontTouchAnnotation"}]} + // CHECK %f = firrtl.wire {annotations = [{class = "firrtl.transforms.DontTouchAnnotation"}, {circt.nonlocal = @annos_nla2, class = "firrtl.transforms.DontTouchAnnotation"}]} + %f = firrtl.wire {annotations = [{class = "firrtl.transforms.DontTouchAnnotation"}]} : !firrtl.uint<1> // Subannotations should be handled correctly. - // CHECK: %f = firrtl.wire {annotations = [{circt.fieldID = 1 : i32, circt.nonlocal = [[NLA0]], class = "subanno"}]} - %f = firrtl.wire {annotations = [{circt.fieldID = 1 : i32, class = "subanno"}]} : !firrtl.bundle> + // CHECK: %g = firrtl.wire {annotations = [{circt.fieldID = 1 : i32, circt.nonlocal = @annos_nla2, class = "subanno"}]} + %g = firrtl.wire {annotations = [{circt.fieldID = 1 : i32, class = "subanno"}]} : !firrtl.bundle> } // CHECK-NOT: firrtl.module @Annotations1 firrtl.module @Annotations1() attributes {annotations = [{class = "one"}]} { - %f = firrtl.wire {annotations = [{class = "both"}]} : !firrtl.uint<1> - %g = firrtl.wire {annotations = [{class = "one"}]} : !firrtl.uint<1> - %h = firrtl.wire : !firrtl.uint<1> - %i = firrtl.wire sym @i {annotations = [{circt.nonlocal = @annos_nla1, class = "NonLocal"}]} : !firrtl.uint<1> - %j = firrtl.wire {annotations = [{circt.nonlocal = @annos_nla3, class = "NonLocal"}]} : !firrtl.uint<1> - %k = firrtl.wire : !firrtl.bundle> + %h = firrtl.wire {annotations = [{class = "one"}]} : !firrtl.uint<1> + %i = firrtl.wire : !firrtl.uint<1> + %j = firrtl.wire sym @j {annotations = [{circt.nonlocal = @annos_nla1, class = "NonLocal"}]} : !firrtl.uint<1> + %k = firrtl.wire {annotations = [{circt.nonlocal = @annos_nla2, class = "NonLocal"}]} : !firrtl.uint<1> + %l = firrtl.wire {annotations = [{class = "both"}]} : !firrtl.uint<1> + %m = firrtl.wire {annotations = [{class = "firrtl.transforms.DontTouchAnnotation"}]} : !firrtl.uint<1> + %n = firrtl.wire : !firrtl.bundle> } firrtl.module @Annotations() { // CHECK: firrtl.instance annotations0 sym @annotations0 @Annotations0() @@ -148,16 +151,16 @@ firrtl.circuit "Annotations" { // Check that module and memory port annotations are merged correctly. // CHECK-LABEL: firrtl.circuit "PortAnnotations" firrtl.circuit "PortAnnotations" { - // CHECK: firrtl.hierpath private [[NLA0:@nla.*]] [@PortAnnotations::@portannos0, @PortAnnotations0] // CHECK: firrtl.hierpath private [[NLA1:@nla.*]] [@PortAnnotations::@portannos1, @PortAnnotations0] + // CHECK: firrtl.hierpath private [[NLA0:@nla.*]] [@PortAnnotations::@portannos0, @PortAnnotations0] // CHECK: firrtl.module @PortAnnotations0(in %a: !firrtl.uint<1> [ - // CHECK-SAME: {circt.nonlocal = [[NLA1]], class = "port1"}, - // CHECK-SAME: {circt.nonlocal = [[NLA0]], class = "port0"}]) { + // CHECK-SAME: {circt.nonlocal = [[NLA0]], class = "port0"}, + // CHECK-SAME: {circt.nonlocal = [[NLA1]], class = "port1"}]) { firrtl.module @PortAnnotations0(in %a : !firrtl.uint<1> [{class = "port0"}]) { // CHECK: %bar_r = firrtl.mem // CHECK-SAME: portAnnotations = - // CHECK-SAME: {circt.nonlocal = [[NLA1]], class = "mem1"}, - // CHECK-SAME: {circt.nonlocal = [[NLA0]], class = "mem0"} + // CHECK-SAME: {circt.nonlocal = [[NLA0]], class = "mem0"}, + // CHECK-SAME: {circt.nonlocal = [[NLA1]], class = "mem1"} %bar_r = firrtl.mem Undefined {depth = 16 : i64, name = "bar", portAnnotations = [[{class = "mem0"}]], portNames = ["r"], readLatency = 0 : i32, writeLatency = 1 : i32} : !firrtl.bundle, en: uint<1>, clk: clock, data flip: uint<8>> } // CHECK-NOT: firrtl.module @PortAnnotations1 @@ -311,15 +314,15 @@ firrtl.circuit "Foo" { // As we dedup modules, the chain on NLAs should continuously grow. // CHECK-LABEL: firrtl.circuit "Chain" firrtl.circuit "Chain" { - // CHECK: firrtl.hierpath private [[NLA0:@nla.*]] [@Chain::@chainB1, @ChainB0::@chainA0, @ChainA0::@extchain0, @ExtChain0] - // CHECK: firrtl.hierpath private [[NLA1:@nla.*]] [@Chain::@chainB0, @ChainB0::@chainA0, @ChainA0::@extchain0, @ExtChain0] + // CHECK: firrtl.hierpath private [[NLA1:@nla.*]] [@Chain::@chainB1, @ChainB0::@chainA0, @ChainA0::@extchain0, @ExtChain0] + // CHECK: firrtl.hierpath private [[NLA0:@nla.*]] [@Chain::@chainB0, @ChainB0::@chainA0, @ChainA0::@extchain0, @ExtChain0] // CHECK: firrtl.module @ChainB0() firrtl.module @ChainB0() { firrtl.instance chainA0 @ChainA0() } // CHECK: firrtl.extmodule @ExtChain0() attributes {annotations = [ - // CHECK-SAME: {circt.nonlocal = [[NLA0]], class = "1"}, - // CHECK-SAME: {circt.nonlocal = [[NLA1]], class = "0"}], defname = "ExtChain"} + // CHECK-SAME: {circt.nonlocal = [[NLA0]], class = "0"}, + // CHECK-SAME: {circt.nonlocal = [[NLA1]], class = "1"}], defname = "ExtChain"} firrtl.extmodule @ExtChain0() attributes {annotations = [{class = "0"}], defname = "ExtChain"} // CHECK-NOT: firrtl.extmodule @ExtChain1() firrtl.extmodule @ExtChain1() attributes {annotations = [{class = "1"}], defname = "ExtChain"}