[FIRRTL] No annotations may be deleted

Change Annotation handling behavior so that no annotation can be deleted. Previously, there existed a method that would report if an annotation could be deleted. However, now that the only annotation that had this semantic, OMIRTracker, is gone, this method is now always `false` for a single annotation and equivalent to `empty` for an `AnnotationSet`. Use these simpler functions instead and delete the `canBeDeleted` functions. Co-authored-by: Mike Urbach <[email protected]> Signed-off-by: Schuyler Eldridge <[email protected]>
llvm · Nov 27, 2024 · 09fc391 · 09fc391
1 parent 5071345
commit 09fc391
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 44 deletions.
diff --git a/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h b/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h
@@ -105,10 +105,6 @@ class Annotation {
   void removeMember(StringAttr name);
   void removeMember(StringRef name);
 
-  /// Returns true if this is an annotation which can be safely deleted without
-  /// consequence.
-  bool canBeDeleted();
-
   using iterator = llvm::ArrayRef<NamedAttribute>::iterator;
   iterator begin() const { return getDict().begin(); }
   iterator end() const { return getDict().end(); }
@@ -247,10 +243,6 @@ class AnnotationSet {
   static bool addDontTouch(Operation *op);
   static bool removeDontTouch(Operation *op);
 
-  /// Check if every annotation can be deleted.
-  bool canBeDeleted() const;
-  static bool canBeDeleted(Operation *op);
-
   bool operator==(const AnnotationSet &other) const {
     return annotations == other.annotations;
   }

diff --git a/lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp b/lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp
@@ -191,16 +191,6 @@ bool AnnotationSet::removeDontTouch(Operation *op) {
   return changed;
 }
 
-bool AnnotationSet::canBeDeleted() const {
-  return llvm::all_of(annotations, [](Attribute attr) {
-    return Annotation(attr).canBeDeleted();
-  });
-}
-
-bool AnnotationSet::canBeDeleted(Operation *op) {
-  return AnnotationSet(op).canBeDeleted();
-}
-
 /// Add more annotations to this AttributeSet.
 void AnnotationSet::addAnnotations(ArrayRef<Annotation> newAnnotations) {
   if (newAnnotations.empty())
@@ -438,10 +428,6 @@ void Annotation::removeMember(StringRef name) {
   setDict(DictionaryAttr::getWithSorted(dict.getContext(), attributes));
 }
 
-bool Annotation::canBeDeleted() {
-  return false;
-}
-
 void Annotation::dump() { attr.dump(); }
 
 //===----------------------------------------------------------------------===//

diff --git a/lib/Dialect/FIRRTL/FIRRTLFolds.cpp b/lib/Dialect/FIRRTL/FIRRTLFolds.cpp
@@ -1761,8 +1761,7 @@ static LogicalResult canonicalizeSingleSetConnect(MatchingConnectOp op,
   // Only support wire and reg for now.
   if (!isa<WireOp>(connectedDecl) && !isa<RegOp>(connectedDecl))
     return failure();
-  if (hasDontTouch(connectedDecl) ||
-      !AnnotationSet(connectedDecl).canBeDeleted() ||
+  if (hasDontTouch(connectedDecl) || !AnnotationSet(connectedDecl).empty() ||
       !hasDroppableName(connectedDecl) ||
       cast<Forceable>(connectedDecl).isForceable())
     return failure();
@@ -1940,7 +1939,7 @@ struct FoldNodeName : public mlir::RewritePattern {
     auto node = cast<NodeOp>(op);
     auto name = node.getNameAttr();
     if (!node.hasDroppableName() || node.getInnerSym() ||
-        !AnnotationSet(node).canBeDeleted() || node.isForceable())
+        !AnnotationSet(node).empty() || node.isForceable())
       return failure();
     auto *newOp = node.getInput().getDefiningOp();
     // Best effort, do not rename InstanceOp
@@ -1958,7 +1957,7 @@ struct NodeBypass : public mlir::RewritePattern {
   LogicalResult matchAndRewrite(Operation *op,
                                 PatternRewriter &rewriter) const override {
     auto node = cast<NodeOp>(op);
-    if (node.getInnerSym() || !AnnotationSet(node).canBeDeleted() ||
+    if (node.getInnerSym() || !AnnotationSet(node).empty() ||
         node.use_empty() || node.isForceable())
       return failure();
     rewriter.replaceAllUsesWith(node.getResult(), node.getInput());
@@ -1985,8 +1984,7 @@ LogicalResult NodeOp::fold(FoldAdaptor adaptor,
     return failure();
   if (hasDontTouch(getResult())) // handles inner symbols
     return failure();
-  if (getAnnotationsAttr() &&
-      !AnnotationSet(getAnnotationsAttr()).canBeDeleted())
+  if (getAnnotationsAttr() && !AnnotationSet(getAnnotationsAttr()).empty())
     return failure();
   if (isForceable())
     return failure();
@@ -2213,7 +2211,7 @@ struct FoldResetMux : public mlir::RewritePattern {
     auto reset =
         dyn_cast_or_null<ConstantOp>(reg.getResetValue().getDefiningOp());
     if (!reset || hasDontTouch(reg.getOperation()) ||
-        !AnnotationSet(reg).canBeDeleted() || reg.isForceable())
+        !AnnotationSet(reg).empty() || reg.isForceable())
       return failure();
     // Find the one true connect, or bail
     auto con = getSingleConnectUserOf(reg.getResult());

diff --git a/lib/Dialect/FIRRTL/Transforms/AdvancedLayerSink.cpp b/lib/Dialect/FIRRTL/Transforms/AdvancedLayerSink.cpp
@@ -83,7 +83,7 @@ class EffectInfo {
 
   /// True if the given operation is NOT moveable due to some effect.
   bool effectful(Operation *op) const {
-    if (!AnnotationSet(op).canBeDeleted() || hasDontTouch(op))
+    if (!AnnotationSet(op).empty() || hasDontTouch(op))
       return true;
     if (auto name = dyn_cast<FNamableOp>(op))
       if (!name.hasDroppableName())
@@ -112,7 +112,7 @@ class EffectInfo {
   }
 
   void update(FModuleLike moduleOp) {
-    if (!AnnotationSet(moduleOp).canBeDeleted())
+    if (!AnnotationSet(moduleOp).empty())
       return markEffectful(moduleOp);
     auto *op = moduleOp.getOperation();
     // Regular modules may be pure.

diff --git a/lib/Dialect/FIRRTL/Transforms/IMConstProp.cpp b/lib/Dialect/FIRRTL/Transforms/IMConstProp.cpp
@@ -65,7 +65,7 @@ static bool isDeletableWireOrRegOrNode(Operation *op) {
     return true;
 
   // Otherwise, don't delete if has anything keeping it around or unknown.
-  return AnnotationSet(op).canBeDeleted() && !hasDontTouch(op) &&
+  return AnnotationSet(op).empty() && !hasDontTouch(op) &&
          hasDroppableName(op) && !cast<Forceable>(op).isForceable();
 }
 

diff --git a/lib/Dialect/FIRRTL/Transforms/IMDeadCodeElim.cpp b/lib/Dialect/FIRRTL/Transforms/IMDeadCodeElim.cpp
@@ -50,7 +50,7 @@ static bool isDeletableDeclaration(Operation *op) {
   if (auto name = dyn_cast<FNamableOp>(op))
     if (!name.hasDroppableName())
       return false;
-  return !hasDontTouch(op) && AnnotationSet(op).canBeDeleted();
+  return !hasDontTouch(op) && AnnotationSet(op).empty();
 }
 
 namespace {
@@ -403,11 +403,6 @@ void IMDeadCodeElimPass::runOnOperation() {
         hierPathOp =
             symbolTable->template lookup<hw::HierPathOp>(hierPathSym.getAttr());
 
-      if (anno.canBeDeleted()) {
-        if (hierPathOp && portId >= 0)
-          hierPathToElements[hierPathOp].insert(module.getArgument(portId));
-        return false;
-      }
       if (hierPathOp)
         markAlive(hierPathOp);
       if (portId >= 0)
@@ -551,9 +546,7 @@ void IMDeadCodeElimPass::rewriteModuleBody(FModuleOp module) {
 
   auto removeDeadNonLocalAnnotations = [&](int _, Annotation anno) -> bool {
     auto hierPathSym = anno.getMember<FlatSymbolRefAttr>("circt.nonlocal");
-    // We only clean up non-local annotations here as local annotations will
-    // be deleted afterwards.
-    if (!anno.canBeDeleted() || !hierPathSym)
+    if (!hierPathSym)
       return false;
     auto hierPathOp =
         symbolTable->template lookup<hw::HierPathOp>(hierPathSym.getAttr());

diff --git a/lib/Dialect/FIRRTL/Transforms/RemoveUnusedPorts.cpp b/lib/Dialect/FIRRTL/Transforms/RemoveUnusedPorts.cpp
@@ -73,9 +73,8 @@ void RemoveUnusedPortsPass::removeUnusedModulePorts(
     auto arg = module.getArgument(index);
 
     // If the port is don't touch or has unprocessed annotations, we cannot
-    // remove the port. Maybe we can allow annotations though.
-    if ((hasDontTouch(arg) || !port.annotations.canBeDeleted()) &&
-        !ignoreDontTouch)
+    // remove the port.
+    if ((hasDontTouch(arg) || !port.annotations.empty()) && !ignoreDontTouch)
       continue;
 
     // TODO: Handle inout ports.