Update more

include/circt/Dialect/FIRRTL/FIRRTLOpInterfaces.td

@@ -49,6 +49,12 @@ def FModuleLike : OpInterface<"FModuleLike", [Symbol, PortList, InstanceGraphMod
     InterfaceMethod<"Get the module's enabled layers.",
       "ArrayRef<Attribute>", "getLayers">,
 
+    InterfaceMethod<"Set the module's enabled layers.",
+      "void", "setLayersAttr", (ins "::mlir::ArrayAttr":$layers)>,
+
+    InterfaceMethod<"Set the module's enabled layers",
+      "void", "setLayers", (ins "::llvm::ArrayRef<Attribute>":$layers)>,
+
     //===------------------------------------------------------------------===//
     // Port Directions
     //===------------------------------------------------------------------===//

include/circt/Dialect/FIRRTL/FIRRTLStructure.td

@@ -305,7 +305,10 @@ def ClassOp : FIRRTLModuleLike<"class", [
     // Class Ops do not support port annotations or enabled layers.
     // Override these methods to return an empty array attr.
     "getPortAnnotationsAttr",
-    "getLayersAttr"]>,
+    "getLayersAttr",
+    "getLayers",
+    "setLayersAttr",
+    "setLayers"]>,
   DeclareOpInterfaceMethods<ClassLike>,
   DeclareOpInterfaceMethods<Symbol, ["canDiscardOnUseEmpty"]>]> {
   let summary = "FIRRTL Class";
@@ -362,7 +365,10 @@ def ExtClassOp : FIRRTLModuleLike<"extclass", [
     // ExtClassOps do not support port annotations or enabled layers.
     // Override these methods to return an empty array attr.
     "getPortAnnotationsAttr",
-    "getLayersAttr"]>,
+    "getLayersAttr",
+    "getLayers",
+    "setLayersAttr",
+    "setLayers"]>,
   DeclareOpInterfaceMethods<ClassLike>,
   DeclareOpInterfaceMethods<Symbol, ["canDiscardOnUseEmpty"]>]> {
   let summary = "FIRRTL external class";

lib/Dialect/FIRRTL/FIRRTLOps.cpp

@@ -1985,6 +1985,14 @@ ArrayAttr ClassOp::getLayersAttr() { return ArrayAttr::get(getContext(), {}); }
 
 ArrayRef<Attribute> ClassOp::getLayers() { return getLayersAttr(); }
 
+void ClassOp::setLayersAttr(ArrayAttr layers) {
+  assert(layers.empty() && "classes may not enable layers");
+}
+
+void ClassOp::setLayers(ArrayRef<Attribute> layers) {
+  assert(layers.empty() && "classes may not enable layers");
+}
+
 SmallVector<::circt::hw::PortInfo> ClassOp::getPortList() {
   return ::getPortListImpl(*this);
 }
@@ -2061,6 +2069,14 @@ ArrayAttr ExtClassOp::getLayersAttr() {
 
 ArrayRef<Attribute> ExtClassOp::getLayers() { return getLayersAttr(); }
 
+void ExtClassOp::setLayersAttr(ArrayAttr layers) {
+  assert(layers.empty() && "classes may not enable layers");
+}
+
+void ExtClassOp::setLayers(ArrayRef<Attribute> layers) {
+  assert(layers.empty() && "classes may not enable layers");
+}
+
 ArrayAttr ExtClassOp::getParameters() { return {}; }
 
 ArrayAttr ExtClassOp::getPortAnnotationsAttr() {

lib/Dialect/FIRRTL/Transforms/LowerLayers.cpp

@@ -55,15 +55,15 @@ class LowerLayersPass : public LowerLayersBase<LowerLayersPass> {
 
   /// Extract layerblocks and strip probe colors from all ops under the module.
   /// Returns true if the module ports were modified.
-  bool runOnModule(FModuleOp moduleOp);
+  void runOnModule(FModuleLike moduleOp);
 
   /// Update the module's port types to remove any explicit layer requirements
   /// from any probe types. Returns true if the port types were updated.
-  bool removeLayersFromPorts(FModuleOp moduleOp);
+  void removeLayersFromPorts(FModuleLike moduleOp);
 
   /// Update the value's type to remove any layers from any probe types.
   /// Returns true if the type changed.
-  bool removeLayersFromValue(Value value);
+  void removeLayersFromValue(Value value);
 
   /// Remove any layers from the result of the cast. If the cast becomes a nop,
   /// remove the cast itself from the IR.
@@ -95,22 +95,20 @@ FModuleOp LowerLayersPass::buildNewModule(OpBuilder &builder, Location location,
   return newModule;
 }
 
-bool LowerLayersPass::removeLayersFromValue(Value value) {
+void LowerLayersPass::removeLayersFromValue(Value value) {
   auto type = dyn_cast<RefType>(value.getType());
   if (!type || !type.getLayer())
-    return false;
+    return;
   value.setType(type.removeLayer());
-  return true;
 }
 
-bool LowerLayersPass::removeLayersFromPorts(FModuleOp moduleOp) {
-  bool changed = false;
-  for (auto arg : moduleOp.getBodyBlock()->getArguments())
-    changed |= removeLayersFromValue(arg);
-  if (!changed)
-    return false;
+void LowerLayersPass::removeLayersFromPorts(FModuleLike moduleLike) {
+  if (auto moduleOp = dyn_cast<FModuleOp>(moduleLike.getOperation())) {
+    for (auto arg : moduleOp.getBodyBlock()->getArguments())
+      removeLayersFromValue(arg);
+  }
 
-  auto oldTypeAttrs = moduleOp.getPortTypesAttr();
+  auto oldTypeAttrs = moduleLike.getPortTypesAttr();
   SmallVector<Attribute> newTypeAttrs;
   newTypeAttrs.reserve(oldTypeAttrs.size());
   for (auto typeAttr : oldTypeAttrs.getAsRange<TypeAttr>()) {
@@ -119,10 +117,8 @@ bool LowerLayersPass::removeLayersFromPorts(FModuleOp moduleOp) {
         typeAttr = TypeAttr::get(refType.removeLayer());
     newTypeAttrs.push_back(typeAttr);
   }
-  moduleOp->setAttr(FModuleLike::getPortTypesAttrName(),
-                    ArrayAttr::get(moduleOp.getContext(), newTypeAttrs));
-
-  return true;
+  moduleLike->setAttr(FModuleLike::getPortTypesAttrName(),
+                      ArrayAttr::get(moduleLike.getContext(), newTypeAttrs));
 }
 
 void LowerLayersPass::removeLayersFromRefCast(RefCastOp cast) {
@@ -141,18 +137,24 @@ void LowerLayersPass::removeLayersFromRefCast(RefCastOp cast) {
   }
 }
 
-bool LowerLayersPass::runOnModule(FModuleOp moduleOp) {
+void LowerLayersPass::runOnModule(FModuleLike moduleLike) {
   LLVM_DEBUG({
-    llvm::dbgs() << "Module: " << moduleOp.getModuleName() << "\n";
+    llvm::dbgs() << "Module: " << moduleLike.getModuleName() << "\n";
     llvm::dbgs() << "  Examining Layer Blocks:\n";
   });
 
+  // Strip away layers from the interface of the module-like op.
+  moduleLike.setLayers({});
+  removeLayersFromPorts(moduleLike);
+
+  // If the module-like op is a real module, remove layers from its body.
+  auto moduleOp = dyn_cast<FModuleOp>(moduleLike.getOperation());
+  if (!moduleOp)
+    return;
+
   CircuitOp circuitOp = moduleOp->getParentOfType<CircuitOp>();
   StringRef circuitName = circuitOp.getName();
 
-  moduleOp.setLayers({});
-  removeLayersFromPorts(moduleOp);
-
   // A map of instance ops to modules that this pass creates.  This is used to
   // check if this was an instance that we created and to do fast module
   // dereferencing (avoiding a symbol table).
@@ -474,8 +476,6 @@ bool LowerLayersPass::runOnModule(FModuleOp moduleOp) {
 
     return WalkResult::advance();
   });
-
-  return true;
 }
 
 /// Process a circuit to remove all layer blocks in each module and top-level
@@ -506,8 +506,9 @@ void LowerLayersPass::runOnOperation() {
   });
 
   // Lower the layer blocks of each module.
-  SmallVector<FModuleOp> modules(circuitOp.getBodyBlock()->getOps<FModuleOp>());
-  parallelForEach(modules, [this](FModuleOp module) { runOnModule(module); });
+  SmallVector<FModuleLike> modules(
+      circuitOp.getBodyBlock()->getOps<FModuleLike>());
+  parallelForEach(modules, [this](FModuleLike module) { runOnModule(module); });
 
   // Generate the header and footer of each bindings file.  The body will be
   // populated later when binds are exported to Verilog.  This produces text
@@ -568,6 +569,11 @@ void LowerLayersPass::runOnOperation() {
       layers.push_back(
           {layerOp, builder.getStringAttr("`include \"" + prefix + ".sv\"")});
   });
+
+  // All layers definitions can now be deleted.
+  for (auto layerOp : llvm::make_early_inc_range(
+           getOperation().getBodyBlock()->getOps<LayerOp>()))
+    layerOp.erase();
 }
 
 std::unique_ptr<mlir::Pass> circt::firrtl::createLowerLayersPass() {

test/Dialect/FIRRTL/lower-layers.mlir

@@ -6,7 +6,6 @@ firrtl.circuit "Simple" {
       firrtl.layer @C bind {}
     }
   }
-  firrtl.layer @B bind {}
 
   firrtl.module @Simple() {
     %a = firrtl.wire : !firrtl.uint<1>
@@ -23,6 +22,7 @@ firrtl.circuit "Simple" {
       }
     }
   }
+}
 
 // CHECK-LABEL: firrtl.circuit "Simple"
 //
@@ -119,6 +119,7 @@ firrtl.circuit "ModuleNameConflict" {
 // CHECK-SAME:      @[[groupModule]](
 
 // -----
+
 firrtl.circuit "Test" {
   firrtl.module @Test() {}
 
@@ -133,42 +134,52 @@ firrtl.circuit "Test" {
   // Removal of Probe Colors
   //===--------------------------------------------------------------------===//
 
-  // CHECK-LABEL: @ColoredPorts(out %o : !firrtl.probe<uint<1>>, in %i : !firrtl.probe<uint<1>>)
-  firrtl.module @ColoredPorts(out %o : !firrtl.probe<uint<1>, @A>, in %i : !firrtl.probe<uint<1>, @A>) {}
+  // CHECK-LABEL: @ColoredPorts(out %o: !firrtl.probe<uint<1>>)
+  firrtl.module @ColoredPorts(out %o: !firrtl.probe<uint<1>, @A>) {}
 
-  // CHECK-LABEL: @ExtColoredPorts(out o : !firrtl.probe<uint<1>>, in i : !firrtl.probe<uint<1>>)
-  firrtl.extmodule @ExtColoredPorts(out o : !firrtl.probe<uint<1>, @A>, in i : !firrtl.probe<uint<1>, @A>) {}
+  // CHECK-LABEL: @ExtColoredPorts(out o: !firrtl.probe<uint<1>>)
+  firrtl.extmodule @ExtColoredPorts(out o: !firrtl.probe<uint<1>, @A>)
 
   // CHECK-LABEL: @ColoredPortsOnInstances
-  firrtl.module @ColoredPortsOnInstances(out o : !firrtl.probe<uint<1>, @A>, in i : !firrtl.probe<uint<1>, @A>) {
-     // CHECK: %foo_o, %foo_i = firrtl.instance @ColoredPorts()
-   %foo_o, %foo_i = firrtl.instance foo {layers = [@A]} @ColoredPorts(out o : !firrtl.probe<uint<1>, @A>, in i : !firrtl.probe<uint<1>, @A>)
+  firrtl.module @ColoredPortsOnInstances() {
+    // CHECK: %foo_o = firrtl.instance foo @ColoredPorts(out o: !firrtl.probe<uint<1>>)
+   %foo_o = firrtl.instance foo @ColoredPorts(out o: !firrtl.probe<uint<1>, @A>)
   }
 
   // CHECK-LABEL: @ColoredThings
   firrtl.module @ColoredThings() {
     // CHECK: %0 = firrtl.wire : !firrtl.probe<bundle<f: uint<1>>>
     %0 = firrtl.wire : !firrtl.probe<bundle<f: uint<1>>, @A>
-    // CHECK: %1 = firrtl.sub %0[0] : !firrtl.probe<bundle<f: uint<1>>>
-    %1 = firrtl.sub %0[0] : !firrtl.probe<bundle<f: uint<1>>, @A>
+    // CHECK: %1 = firrtl.ref.sub %0[0] : !firrtl.probe<bundle<f: uint<1>>>
+    %1 = firrtl.ref.sub %0[0] : !firrtl.probe<bundle<f: uint<1>>, @A>
     // CHECK-NOT: firrtl.cast
-    %2 = firrtl.cast %1 : (!firrtl.probe<uint<1>, @A>) -> !firrtl.probe<uint<1>, @A::@B>
+    %2 = firrtl.ref.cast %1 : (!firrtl.probe<uint<1>, @A>) -> !firrtl.probe<uint<1>, @A::@B>
   }
 
   //===--------------------------------------------------------------------===//
   // Removal of Enabled Layers
   //===--------------------------------------------------------------------===//
 
-  // CHECK-LABEL: @EnabledLayers() {}
+  // CHECK-LABEL: @EnabledLayers() {
   firrtl.module @EnabledLayers() attributes {layers = [@A]} {}
 
   // CHECK-LABEL: @EnabledLayersOnInstance()
-  firrtl.module @EnabledLayersOnInstance() {
+  firrtl.module @EnabledLayersOnInstance() attributes {layers = [@A]} {
     // CHECK: firrtl.instance enabledLayers @EnabledLayers()
     firrtl.instance enabledLayers {layers = [@A]} @EnabledLayers()
   }
 
   //===--------------------------------------------------------------------===//
-  // Removal of Layerblocks
+  // Removal of Layerblocks and Layers
   //===--------------------------------------------------------------------===//
+
+  // CHECK-NOT: firrtl.layer @GoodbyeCruelWorld
+  firrtl.layer @GoodbyeCruelWorld bind {}
+
+  // CHECK-LABEL @WithLayerBlock
+  firrtl.module @WithLayerBlock() {
+    // CHECK-NOT firrtl.layerblock @GoodbyeCruelWorld
+    firrtl.layerblock @GoodbyeCruelWorld {
+    }
+  }
 }