Update reported width from div/rem to match FIRRTL results (#1748)

* Update reported width from div/rem to match FIRRTL results

* Add tests for width of % and / on UInt and SInt

* Add loop-based test for known UInt/SInt op result widths

Co-authored-by: Jack Koenig <koenig@sifive.com>

5 files changed, 128 insertions, 3 deletions

diff --git a/core/src/main/scala/chisel3/Bits.scala b/core/src/main/scala/chisel3/Bits.scala
index 6bd5a07c..c86e9208 100644
--- a/core/src/main/scala/chisel3/Bits.scala
+++ b/core/src/main/scala/chisel3/Bits.scala
@@ -448,7 +448,7 @@ sealed class UInt private[chisel3] (width: Width) extends Bits(width) with Num[U
   override def do_/ (that: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt =
     binop(sourceInfo, UInt(this.width), DivideOp, that)
   override def do_% (that: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt =
-    binop(sourceInfo, UInt(this.width), RemOp, that)
+    binop(sourceInfo, UInt(this.width min that.width), RemOp, that)
   override def do_* (that: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt =
     binop(sourceInfo, UInt(this.width + that.width), TimesOp, that)
 
@@ -762,9 +762,9 @@ sealed class SInt private[chisel3] (width: Width) extends Bits(width) with Num[S
   override def do_* (that: SInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): SInt =
     binop(sourceInfo, SInt(this.width + that.width), TimesOp, that)
   override def do_/ (that: SInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): SInt =
-    binop(sourceInfo, SInt(this.width), DivideOp, that)
+    binop(sourceInfo, SInt(this.width + 1), DivideOp, that)
   override def do_% (that: SInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): SInt =
-    binop(sourceInfo, SInt(this.width), RemOp, that)
+    binop(sourceInfo, SInt(this.width min that.width), RemOp, that)
 
   /** Multiplication operator
     *
diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala
index 095c8a05..61f97ce6 100644
--- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala
+++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala
@@ -158,6 +158,7 @@ object Width {
 
 sealed abstract class Width {
   type W = Int
+  def min(that: Width): Width = this.op(that, _ min _)
   def max(that: Width): Width = this.op(that, _ max _)
   def + (that: Width): Width = this.op(that, _ + _)
   def + (that: Int): Width = this.op(this, (a, b) => a + that)
diff --git a/src/test/scala/chiselTests/SIntOps.scala b/src/test/scala/chiselTests/SIntOps.scala
index 9aacc378..f2e238e9 100644
--- a/src/test/scala/chiselTests/SIntOps.scala
+++ b/src/test/scala/chiselTests/SIntOps.scala
@@ -116,4 +116,36 @@ class SIntOpsSpec extends ChiselPropSpec with Utils {
     assertTesterPasses(new SIntLitExtractTester)
   }
 
+  // We use WireDefault with 2 arguments because of
+  // https://www.chisel-lang.org/api/3.4.1/chisel3/WireDefault$.html
+  //   Single Argument case 2
+  property("modulo divide should give min width of arguments") {
+    assertKnownWidth(4) {
+      val x = WireDefault(SInt(8.W), DontCare)
+      val y = WireDefault(SInt(4.W), DontCare)
+      val op = x % y
+      WireDefault(chiselTypeOf(op), op)
+    }
+    assertKnownWidth(4) {
+      val x = WireDefault(SInt(4.W), DontCare)
+      val y = WireDefault(SInt(8.W), DontCare)
+      val op = x % y
+      WireDefault(chiselTypeOf(op), op)
+    }
+  }
+
+  property("division should give the width of the numerator + 1") {
+    assertKnownWidth(9) {
+      val x = WireDefault(SInt(8.W), DontCare)
+      val y = WireDefault(SInt(4.W), DontCare)
+      val op = x / y
+      WireDefault(chiselTypeOf(op), op)
+    }
+    assertKnownWidth(5) {
+      val x = WireDefault(SInt(4.W), DontCare)
+      val y = WireDefault(SInt(8.W), DontCare)
+      val op = x / y
+      WireDefault(chiselTypeOf(op), op)
+    }
+  }
 }
diff --git a/src/test/scala/chiselTests/UIntOps.scala b/src/test/scala/chiselTests/UIntOps.scala
index bba06d11..62d00de2 100644
--- a/src/test/scala/chiselTests/UIntOps.scala
+++ b/src/test/scala/chiselTests/UIntOps.scala
@@ -153,4 +153,37 @@ class UIntOpsSpec extends ChiselPropSpec with Matchers with Utils {
       io.out := io.in.asBools()(2)
     })
   }
+
+  // We use WireDefault with 2 arguments because of
+  // https://www.chisel-lang.org/api/3.4.1/chisel3/WireDefault$.html
+  //   Single Argument case 2
+  property("modulo divide should give min width of arguments") {
+    assertKnownWidth(4) {
+      val x = WireDefault(UInt(8.W), DontCare)
+      val y = WireDefault(UInt(4.W), DontCare)
+      val op = x % y
+      WireDefault(chiselTypeOf(op), op)
+    }
+    assertKnownWidth(4) {
+      val x = WireDefault(UInt(4.W), DontCare)
+      val y = WireDefault(UInt(8.W), DontCare)
+      val op = x % y
+      WireDefault(chiselTypeOf(op), op)
+    }
+  }
+
+  property("division should give the width of the numerator") {
+    assertKnownWidth(8) {
+      val x = WireDefault(UInt(8.W), DontCare)
+      val y = WireDefault(UInt(4.W), DontCare)
+      val op = x / y
+      WireDefault(chiselTypeOf(op), op)
+    }
+    assertKnownWidth(4) {
+      val x = WireDefault(UInt(4.W), DontCare)
+      val y = WireDefault(UInt(8.W), DontCare)
+      val op = x / y
+      WireDefault(chiselTypeOf(op), op)
+    }
+  }
 }
diff --git a/src/test/scala/chiselTests/WidthSpec.scala b/src/test/scala/chiselTests/WidthSpec.scala
index 2a33c1d6..34159214 100644
--- a/src/test/scala/chiselTests/WidthSpec.scala
+++ b/src/test/scala/chiselTests/WidthSpec.scala
@@ -186,3 +186,62 @@ class RegInitWidthSpec extends WireDefaultRegInitSpecImpl {
   def builder2[T <: Data](x: T, y: T): T = RegInit(x, y)
 }
 
+class OpWidthSpec extends ChiselFlatSpec {
+  import firrtl._
+  import firrtl.ir._
+
+  val maxWidth = 5
+  val uIntOps: Seq[((UInt, UInt) => UInt, PrimOp)] =
+    Seq(
+      (_ +& _, PrimOps.Add),
+      (_ -& _, PrimOps.Sub),
+      (_ * _, PrimOps.Mul),
+      (_ / _, PrimOps.Div),
+      (_ % _, PrimOps.Rem),
+      (_ << _, PrimOps.Dshl),
+      (_ >> _, PrimOps.Dshr)
+    )
+
+  assertTesterPasses(new chisel3.testers.BasicTester {
+    for (i <- 0 to maxWidth) {
+      for (j <- 0 to maxWidth) {
+        for ((cOp, fOp) <- uIntOps) {
+          val args = Seq(i, j).map(w => Reference("", UIntType(IntWidth(w))))
+          fOp.propagateType(DoPrim(fOp, args, Nil, UnknownType)) match {
+            case UIntType(IntWidth(w)) =>
+              val x = 0.U(maxWidth.W).head(i)
+              val y = 0.U(maxWidth.W).head(j)
+              assert(w == cOp(x, y).getWidth)
+          }
+        }
+      }
+    }
+    stop()
+  })
+
+  val sIntOps: Seq[((SInt, SInt) => SInt, PrimOp)] =
+    Seq(
+      (_ +& _, PrimOps.Add),
+      (_ -& _, PrimOps.Sub),
+      (_ * _, PrimOps.Mul),
+      (_ / _, PrimOps.Div),
+      (_ % _, PrimOps.Rem)
+    )
+
+  assertTesterPasses(new chisel3.testers.BasicTester {
+    for (i <- 0 to maxWidth) {
+      for (j <- 0 to maxWidth) {
+        for ((cOp, fOp) <- sIntOps) {
+          val args = Seq(i, j).map(w => Reference("", SIntType(IntWidth(w))))
+          fOp.propagateType(DoPrim(fOp, args, Nil, UnknownType)) match {
+            case SIntType(IntWidth(w)) =>
+              val x = 0.U(maxWidth.W).head(i).asSInt
+              val y = 0.U(maxWidth.W).head(j).asSInt
+              assert(w == cOp(x, y).getWidth)
+          }
+        }
+      }
+    }
+    stop()
+  })
+}