[calyx] fix calyx canonicalization.

lib/Dialect/Calyx/CMakeLists.txt

@@ -21,6 +21,7 @@ add_circt_dialect_library(CIRCTCalyx
   CIRCTComb
   CIRCTSV
   CIRCTHW
+  CIRCTFSM
   MLIRArithDialect
   MLIRIR
   MLIRMemRefDialect

lib/Dialect/Calyx/CalyxOps.cpp

@@ -12,6 +12,7 @@
 
 #include "circt/Dialect/Calyx/CalyxOps.h"
 #include "circt/Dialect/Comb/CombOps.h"
+#include "circt/Dialect/FSM/FSMOps.h"
 #include "circt/Dialect/HW/HWAttributes.h"
 #include "circt/Dialect/HW/HWOps.h"
 #include "circt/Dialect/HW/HWTypes.h"
@@ -745,16 +746,25 @@ LogicalResult ComponentOp::verify() {
 
   // Verify the component actually does something: has a non-empty Control
   // region, or continuous assignments.
-  bool hasNoControlConstructs =
-      getControlOp().getBodyBlock()->getOperations().empty();
+  bool hasNoControlConstructs = true;
+  getControlOp().walk<WalkOrder::PreOrder>([&](Operation *op) {
+    if (isa<EnableOp, InvokeOp, fsm::MachineOp>(op)) {
+      hasNoControlConstructs = false;
+      return WalkResult::interrupt();
+    }
+    return WalkResult::advance();
+  });
   bool hasNoAssignments =
       getWiresOp().getBodyBlock()->getOps<AssignOp>().empty();
   if (hasNoControlConstructs && hasNoAssignments)
     return emitOpError(
-        "The component currently does nothing. It needs to either have "
-        "continuous assignments in the Wires region or control constructs in "
-        "the Control region.");
-
+               "The component currently does nothing. It needs to either have "
+               "continuous assignments in the Wires region or control "
+               "constructs in the Control region. The Control region "
+               "should contain at least one of ")
+           << "'" << EnableOp::getOperationName() << "' , "
+           << "'" << InvokeOp::getOperationName() << "' or "
+           << "'" << fsm::MachineOp::getOperationName() << "'.";
   return success();
 }
 
@@ -843,8 +853,7 @@ LogicalResult CombComponentOp::verify() {
   if (hasNoAssignments)
     return emitOpError(
         "The component currently does nothing. It needs to either have "
-        "continuous assignments in the Wires region or control constructs in "
-        "the Control region.");
+        "continuous assignments in the Wires region.");
 
   // Check that all cells are combinational
   auto cells = getOps<CellInterface>();
@@ -2236,6 +2245,8 @@ template <typename OpTy>
 static std::optional<EnableOp> getLastEnableOp(OpTy parent) {
   static_assert(IsAny<OpTy, SeqOp, StaticSeqOp>(),
                 "Should be a StaticSeqOp or SeqOp.");
+  if (parent.getBodyBlock()->empty())
+    return std::nullopt;
   auto &lastOp = parent.getBodyBlock()->back();
   if (auto enableOp = dyn_cast<EnableOp>(lastOp))
     return enableOp;

test/Conversion/CalyxToFSM/materialize-errors.mlir

@@ -1,18 +1,16 @@
 // RUN: circt-opt -pass-pipeline='builtin.module(calyx.component(materialize-calyx-to-fsm))' -split-input-file -verify-diagnostics %s
-
 calyx.component @main(%go: i1 {go}, %reset: i1 {reset}, %clk: i1 {clk}) -> (%done: i1 {done}) {
+  %r.in, %r.write_en, %r.clk, %r.reset, %r.out, %r.done = calyx.register @r : i32, i1, i1, i1, i32, i1
   calyx.wires {
+    calyx.assign %r.clk = %clk : i1
   }
 // expected-error @+1 {{'calyx.control' op expected an 'fsm.machine' operation as the top-level operation within the control region of this component.}}
   calyx.control {
     calyx.seq {}
   }
 }
 
-
-
 // -----
-
 calyx.component @main(%go: i1 {go}, %reset: i1 {reset}, %clk: i1 {clk}) -> (%done: i1 {done}) {
   calyx.wires {
   }

test/Dialect/Calyx/clk-insertion.mlir

@@ -6,7 +6,7 @@ module attributes {calyx.entrypoint = "main"} {
     calyx.wires { calyx.assign %done = %c1_1 : i1 }
     calyx.control {}
   }
-  calyx.component @main(%go: i1 {go}, %clk: i1 {clk}, %reset: i1 {reset}) -> (%done: i1 {done}) {
+  calyx.component @main(%in: i8, %go: i1 {go}, %clk: i1 {clk}, %reset: i1 {reset}) -> (%out: i8, %done: i1 {done}) {
     %c0.in, %c0.go, %c0.clk, %c0.reset, %c0.out, %c0.done = calyx.instance @c0 of @A : i8, i1, i1, i1, i8, i1
     %r.in, %r.write_en, %r.clk, %r.reset, %r.out, %r.done = calyx.register @r : i1, i1, i1, i1, i1, i1
 // CHECK: calyx.wires {
@@ -16,6 +16,8 @@ module attributes {calyx.entrypoint = "main"} {
 // CHECK:   calyx.assign %r.clk = %clk : i1
 // CHECK: }
     calyx.wires {
+      calyx.assign %c0.in = %in : i8
+      calyx.assign %out = %c0.out : i8
     }
     calyx.control {
       calyx.seq { }

test/Dialect/Calyx/errors.mlir

@@ -886,7 +886,7 @@ module attributes {calyx.entrypoint = "main"} {
 // -----
 
 module attributes {calyx.entrypoint = "main"} {
-  // expected-error @+1 {{'calyx.component' op The component currently does nothing. It needs to either have continuous assignments in the Wires region or control constructs in the Control region.}}
+  // expected-error @+1 {{'calyx.component' op The component currently does nothing. It needs to either have continuous assignments in the Wires region or control constructs in the Control region. The Control region should contain at least one of 'calyx.enable' , 'calyx.invoke' or 'fsm.machine'.}}
   calyx.component @main(%go: i1 {go}, %clk: i1 {clk}, %reset: i1 {reset}) -> (%done: i1 {done}) {
     %std_lt_0.left, %std_lt_0.right, %std_lt_0.out = calyx.std_lt @std_lt_0 : i32, i32, i1
     %c64_i32 = hw.constant 64 : i32
@@ -899,6 +899,54 @@ module attributes {calyx.entrypoint = "main"} {
   }
 }
 
+// -----
+
+module attributes {calyx.entrypoint = "main"} {
+   // expected-error @+1 {{'calyx.component' op The component currently does nothing. It needs to either have continuous assignments in the Wires region or control constructs in the Control region. The Control region should contain at least one of 'calyx.enable' , 'calyx.invoke' or 'fsm.machine'.}}
+  calyx.component @main(%go: i1 {go}, %clk: i1 {clk}, %reset: i1 {reset}) -> (%done: i1 {done}) {
+    %r.in, %r.write_en, %r.clk, %r.reset, %r.out, %r.done = calyx.register @r : i1, i1, i1, i1, i1, i1
+    %eq.left, %eq.right, %eq.out = calyx.std_eq @eq : i1, i1, i1
+    %c1_1 = hw.constant 1 : i1
+    calyx.wires {
+    }
+    calyx.control {
+      calyx.seq {
+        calyx.if %eq.out {
+          calyx.seq {
+          }
+        } else {
+          calyx.seq {
+          }
+        }
+      }
+    }
+  }
+}
+
+// -----
+
+module attributes {calyx.entrypoint = "main"} {
+   // expected-error @+1 {{'calyx.component' op The component currently does nothing. It needs to either have continuous assignments in the Wires region or control constructs in the Control region. The Control region should contain at least one of 'calyx.enable' , 'calyx.invoke' or 'fsm.machine'.}}
+  calyx.component @main(%go: i1 {go}, %clk: i1 {clk}, %reset: i1 {reset}) -> (%done: i1 {done}) {
+    %r.in, %r.write_en, %r.clk, %r.reset, %r.out, %r.done = calyx.register @r : i1, i1, i1, i1, i1, i1
+    %eq.left, %eq.right, %eq.out = calyx.std_eq @eq : i1, i1, i1
+    %c1_1 = hw.constant 1 : i1
+    calyx.wires {
+    }
+    calyx.control {
+      calyx.par {
+        calyx.if %eq.out {
+          calyx.par {
+          }
+        } else {
+          calyx.par {
+          }
+        }
+      }
+    }
+  }
+}
+
 // -----
 module attributes {calyx.entrypoint = "A"} {
   hw.module.extern @params<WIDTH: i32>(in %in: !hw.int<#hw.param.decl.ref<"WIDTH">>, in %clk: i1 {calyx.clk}, in %go: i1 {calyx.go = 1}, out out: !hw.int<#hw.param.decl.ref<"WIDTH">>, out done: i1 {calyx.done}) attributes {filename = "test.v"}
@@ -988,7 +1036,7 @@ module attributes {calyx.entrypoint = "main"} {
 // -----
 
 module attributes {calyx.entrypoint = "main"} {
-  // expected-error @+1 {{'calyx.comb_component' op The component currently does nothing. It needs to either have continuous assignments in the Wires region or control constructs in the Control region.}}
+  // expected-error @+1 {{'calyx.comb_component' op The component currently does nothing. It needs to either have continuous assignments in the Wires region.}}
   calyx.comb_component @main() -> () {
     %std_lt_0.left, %std_lt_0.right, %std_lt_0.out = calyx.std_lt @std_lt_0 : i32, i32, i1
     %c64_i32 = hw.constant 64 : i32