[FIRRTL][InferWidths] Restore back-prop constraint for strictconnect. (…

…#5409) Fix #5408. Unfix #5391. This partially reverts commit 6c01320.
llvm · Jun 15, 2023 · 5f9d04e · 5f9d04e
1 parent b0d0a22
commit 5f9d04e
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 29 deletions.
diff --git a/lib/Dialect/FIRRTL/Transforms/InferWidths.cpp b/lib/Dialect/FIRRTL/Transforms/InferWidths.cpp
@@ -1550,11 +1550,20 @@ LogicalResult InferenceMapping::mapOperation(Operation *op) {
       })
       .Case<ConnectOp>(
           [&](auto op) { constrainTypes(op.getDest(), op.getSrc()); })
-      .Case<RefDefineOp, StrictConnectOp>([&](auto op) {
+      .Case<RefDefineOp>([&](auto op) {
         // Dest >= Src, but also check Src <= Dest for correctness
         // (but don't solve to make this true, don't back-propagate)
         constrainTypes(op.getDest(), op.getSrc(), true);
       })
+      // StrictConnect is an identify constraint
+      .Case<StrictConnectOp>([&](auto op) {
+        // This back-propagates width from destination to source,
+        // causing source to sometimes be inferred wider than
+        // it should be (https://github.com/llvm/circt/issues/5391).
+        // This is needed to push the width into feeding widthCast?
+        constrainTypes(op.getDest(), op.getSrc());
+        constrainTypes(op.getSrc(), op.getDest());
+      })
       .Case<AttachOp>([&](auto op) {
         // Attach connects multiple analog signals together. All signals must
         // have the same bit width. Signals without bit width inherit from the

diff --git a/test/Dialect/FIRRTL/infer-widths-errors.mlir b/test/Dialect/FIRRTL/infer-widths-errors.mlir
@@ -148,34 +148,6 @@ firrtl.circuit "Foo" {
   }
 }
 
-// -----
-// https://github.com/llvm/circt/issues/5391
-// Unclear if widthCast is/should be allowed to be CSE'd.
-// This IR was generated from test on that issue before InferWidths.
-
-// Don't back-propagate widths.
-firrtl.circuit "Issue5391" {
-  firrtl.module @Issue5391(in %x: !firrtl.uint<1>,
-                           in %y: !firrtl.uint<2>,
-                           out %out1: !firrtl.uint,
-                           out %out2: !firrtl.uint) {
-    // expected-error @below {{uninferred width: wire "w" cannot satisfy all width requirements}}
-    %w = firrtl.wire : !firrtl.uint
-    %0 = firrtl.widthCast %x : (!firrtl.uint<1>) -> !firrtl.uint
-    firrtl.strictconnect %w, %0 : !firrtl.uint
-    %1 = firrtl.widthCast %y : (!firrtl.uint<2>) -> !firrtl.uint
-    // expected-note @below {{width is constrained to be at most 1 here:}}
-    // expected-note @below {{width is constrained to be at least 2 here:}}
-    firrtl.strictconnect %w, %1 : !firrtl.uint
-    %2 = firrtl.widthCast %w : (!firrtl.uint) -> !firrtl.uint
-    firrtl.strictconnect %out1, %2 : !firrtl.uint
-    %wx = firrtl.wire : !firrtl.uint
-    firrtl.strictconnect %wx, %0 : !firrtl.uint
-    %3 = firrtl.widthCast %wx : (!firrtl.uint) -> !firrtl.uint
-    firrtl.strictconnect %out2, %3 : !firrtl.uint
-  }
-}
-
 // -----
 // https://github.com/llvm/circt/issues/5002
 

diff --git a/test/Dialect/FIRRTL/infer-widths.mlir b/test/Dialect/FIRRTL/infer-widths.mlir
@@ -935,4 +935,29 @@ firrtl.circuit "Foo" {
   // Should not crash when encountering property types.
   // CHECK: firrtl.module @Property(in %a: !firrtl.string)
   firrtl.module @Property(in %a: !firrtl.string) { }
+
+  // Check some strictconnect + widthCast inferences.
+  // See https://github.com/llvm/circt/issues/5408 .
+  // CHECK-LABEL: module @StrictConnectBackIntoWidthCast1
+  firrtl.module @StrictConnectBackIntoWidthCast1(in %y: !firrtl.uint<2>, out %out1: !firrtl.uint) attributes {convention = #firrtl<convention scalarized>} {
+    %w = firrtl.wire : !firrtl.uint
+    %c0_ui = firrtl.constant 0 : !firrtl.const.uint
+    %0 = firrtl.widthCast %c0_ui : (!firrtl.const.uint) -> !firrtl.const.uint
+    %1 = firrtl.constCast %0 : (!firrtl.const.uint) -> !firrtl.uint
+    firrtl.strictconnect %w, %1 : !firrtl.uint
+    %2 = firrtl.widthCast %y : (!firrtl.uint<2>) -> !firrtl.uint
+    firrtl.strictconnect %w, %2 : !firrtl.uint
+    %3 = firrtl.widthCast %w : (!firrtl.uint) -> !firrtl.uint
+    firrtl.strictconnect %out1, %3 : !firrtl.uint
+  }
+  // CHECK-LABEL: module @StrictConnectBackIntoWidthCast2
+  firrtl.module @StrictConnectBackIntoWidthCast2(in %x: !firrtl.uint<1>, in %y: !firrtl.uint<2>, out %out1: !firrtl.uint) attributes {convention = #firrtl<convention scalarized>} {
+    %w = firrtl.wire : !firrtl.uint
+    %0 = firrtl.widthCast %x : (!firrtl.uint<1>) -> !firrtl.uint
+    firrtl.strictconnect %w, %0 : !firrtl.uint
+    %1 = firrtl.widthCast %y : (!firrtl.uint<2>) -> !firrtl.uint
+    firrtl.strictconnect %w, %1 : !firrtl.uint
+    %2 = firrtl.widthCast %w : (!firrtl.uint) -> !firrtl.uint
+    firrtl.strictconnect %out1, %2 : !firrtl.uint
+  }
 }