From d912e079755e7ebb21fb149fe3fad996cbef7a55 Mon Sep 17 00:00:00 2001
From: Hideto Ueno <uenoku.tokotoko@gmail.com>
Date: Wed, 4 Dec 2024 16:14:52 -0800
Subject: [PATCH] [LowerTypes] Copy discardable attributes when cloning
 operations (#7948)

This PR changes to clone discardable attributes when operation is cloned.

Fix https://github.com/llvm/circt/issues/7934. AttributeAnnotation adds `sv.attributes` in LowerAnnotation as a discardable attribute which previously LowerTypes dropped.
---
 lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp | 14 ++++----------
 test/Dialect/FIRRTL/lower-types.mlir         | 15 +++++++++++++++
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp b/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp
index 3b0b095052ad..df93e2a40903 100644
--- a/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp
+++ b/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp
@@ -687,6 +687,9 @@ bool TypeLoweringVisitor::lowerProducer(
         newOp->setAttr(cache.nameAttr, StringAttr::get(context, loweredName));
       if (nameKindAttr)
         newOp->setAttr(cache.nameKindAttr, nameKindAttr);
+
+      // Clone discardable attributes as well.
+      newOp->setDiscardableAttrs(op->getDiscardableAttrDictionary());
     }
     lowered.push_back(newVal);
   }
@@ -1502,16 +1505,7 @@ bool TypeLoweringVisitor::visitDecl(InstanceOp op) {
       op.getLowerToBindAttr(),
       sym ? hw::InnerSymAttr::get(sym) : hw::InnerSymAttr());
 
-  // Copy over any attributes which have not already been copied over by
-  // arguments to the builder.
-  auto attrNames = InstanceOp::getAttributeNames();
-  DenseSet<StringRef> attrSet(attrNames.begin(), attrNames.end());
-  SmallVector<NamedAttribute> newAttrs(newInstance->getAttrs());
-  for (auto i : llvm::make_filter_range(op->getAttrs(), [&](auto namedAttr) {
-         return !attrSet.count(namedAttr.getName());
-       }))
-    newAttrs.push_back(i);
-  newInstance->setAttrs(newAttrs);
+  newInstance->setDiscardableAttrs(op->getDiscardableAttrDictionary());
 
   SmallVector<Value> lowered;
   for (size_t aggIndex = 0, eAgg = op.getNumResults(); aggIndex != eAgg;
diff --git a/test/Dialect/FIRRTL/lower-types.mlir b/test/Dialect/FIRRTL/lower-types.mlir
index 385b05085bb5..2de51c8d835f 100644
--- a/test/Dialect/FIRRTL/lower-types.mlir
+++ b/test/Dialect/FIRRTL/lower-types.mlir
@@ -1422,3 +1422,18 @@ firrtl.circuit "MemoryPrefixCopying" {
     } : !firrtl.bundle<addr: uint<6>, en: uint<1>, clk: clock, data flip: vector<uint<8>, 1>>
   }
 }
+
+
+firrtl.circuit "DiscardableAttributes" {
+  // COMMON-LABEL: firrtl.module @DiscardableAttributes
+  firrtl.module @DiscardableAttributes(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>, in %a: !firrtl.bundle<a: uint<1>>) {
+    // CHECK-NEXT: %node_a = firrtl.node %a_a {foo}
+    // CHECK-NEXT: %wire_a = firrtl.wire {foo = "bar"}
+    // CHECK-NEXT: %reg_a = firrtl.reg %clock {foo}
+    // CHECK-NEXT: %regreset_a = firrtl.regreset %clock, %reset, %a_a {foo}
+    %node = firrtl.node %a {foo}: !firrtl.bundle<a: uint<1>>
+    %wire = firrtl.wire {foo = "bar"} : !firrtl.bundle<a: uint<1>>
+    %reg = firrtl.reg %clock {foo}: !firrtl.clock, !firrtl.bundle<a: uint<1>>
+    %regreset = firrtl.regreset %clock, %reset, %a {foo}: !firrtl.clock, !firrtl.uint<1>, !firrtl.bundle<a: uint<1>>, !firrtl.bundle<a: uint<1>>
+  }
+}