Change the width of static shift right

core/src/main/scala/chisel3/Bits.scala

@@ -17,6 +17,7 @@ import chisel3.internal.firrtl.ir.PrimOp._
 import _root_.firrtl.{ir => firrtlir}
 import chisel3.internal.{castToInt, Builder, Warning, WarningID}
 import chisel3.util.simpleClassName
+import scala.annotation.nowarn
 
 /** Exists to unify common interfaces of [[Bits]] and [[Reset]].
   *
@@ -676,8 +677,26 @@ sealed class UInt private[chisel3] (width: Width) extends Bits(width) with Num[U
     this << castToInt(that, "Shift amount")
   override def do_<<(that: UInt)(implicit sourceInfo: SourceInfo): UInt =
     binop(sourceInfo, UInt(this.width.dynamicShiftLeft(that.width)), DynamicShiftLeftOp, that)
-  override def do_>>(that: Int)(implicit sourceInfo: SourceInfo): UInt =
-    binop(sourceInfo, UInt(this.width.shiftRight(that)), ShiftRightOp, validateShiftAmount(that))
+
+  // Implement legacy [buggy] UInt shr behavior for both Chisel and FIRRTL
+  @nowarn("msg=method shiftRight in class Width is deprecated")
+  private def legacyShiftRight(that: Int)(implicit sourceInfo: SourceInfo): UInt = {
+    val resultWidth = this.width.shiftRight(that)
+    val op = binop(sourceInfo, UInt(resultWidth), ShiftRightOp, validateShiftAmount(that))
+    resultWidth match {
+      // If result width is known and 0, wrap in pad(_, 1) to emulate old FIRRTL behavior,
+      //   but lie and say width = 0 to emulate old Chisel behavior.
+      case w @ KnownWidth(0) => op.binop(sourceInfo, UInt(w), PadOp, 1)
+      // If result width is unknown, still wrap in pad(_, 1) to emulate old FIRRTL behavior.
+      case UnknownWidth() => op.binop(sourceInfo, UInt(UnknownWidth()), PadOp, 1)
+      case _              => op
+    }
+  }
+
+  override def do_>>(that: Int)(implicit sourceInfo: SourceInfo): UInt = {
+    if (Builder.legacyShiftRightWidth) legacyShiftRight(that)
+    else binop(sourceInfo, UInt(this.width.unsignedShiftRight(that)), ShiftRightOp, validateShiftAmount(that))
+  }
   override def do_>>(that: BigInt)(implicit sourceInfo: SourceInfo): UInt =
     this >> castToInt(that, "Shift amount")
   override def do_>>(that: UInt)(implicit sourceInfo: SourceInfo): UInt =
@@ -989,8 +1008,14 @@ sealed class SInt private[chisel3] (width: Width) extends Bits(width) with Num[S
     this << castToInt(that, "Shift amount")
   override def do_<<(that: UInt)(implicit sourceInfo: SourceInfo): SInt =
     binop(sourceInfo, SInt(this.width.dynamicShiftLeft(that.width)), DynamicShiftLeftOp, that)
-  override def do_>>(that: Int)(implicit sourceInfo: SourceInfo): SInt =
-    binop(sourceInfo, SInt(this.width.shiftRight(that)), ShiftRightOp, validateShiftAmount(that))
+
+  @nowarn("msg=method shiftRight in class Width is deprecated")
+  override def do_>>(that: Int)(implicit sourceInfo: SourceInfo): SInt = {
+    // We don't need to pad to emulate old behavior for SInt, just emulate old Chisel behavior with reported width,
+    // FIRRTL will give a minimum of 1 bit for SInt
+    val newWidth = if (Builder.legacyShiftRightWidth) this.width.shiftRight(that) else this.width.signedShiftRight(that)
+    binop(sourceInfo, SInt(newWidth), ShiftRightOp, validateShiftAmount(that))
+  }
   override def do_>>(that: BigInt)(implicit sourceInfo: SourceInfo): SInt =
     this >> castToInt(that, "Shift amount")
   override def do_>>(that: UInt)(implicit sourceInfo: SourceInfo): SInt =

core/src/main/scala/chisel3/Width.scala

@@ -9,12 +9,18 @@ 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)
-  def shiftRight(that:       Int): Width = this.op(this, (a, b) => 0.max(a - that))
-  def dynamicShiftLeft(that: Width): Width =
+  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)
+  @deprecated(
+    "The width of shift-right now differs by type, use unsignedShiftRight and signedShiftRight",
+    "Chisel 7.0.0"
+  )
+  def shiftRight(that:         Int): Width = this.op(this, (a, b) => 0.max(a - that))
+  def unsignedShiftRight(that: Int): Width = this.op(this, (a, b) => 0.max(a - that))
+  def signedShiftRight(that:   Int): Width = this.op(this, (a, b) => 1.max(a - that))
+  def dynamicShiftLeft(that:   Width): Width =
     this.op(that, (a, b) => a + (1 << b) - 1)
 
   def known: Boolean

core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala

@@ -104,6 +104,7 @@ object Definition extends SourceInfoDoc {
       new DynamicContext(
         Nil,
         context.throwOnFirstError,
+        context.legacyShiftRightWidth,
         context.warningFilters,
         context.sourceRoots,
         Some(context.globalNamespace),

core/src/main/scala/chisel3/internal/Builder.scala

@@ -446,11 +446,12 @@ private[chisel3] class ChiselContext() {
 }
 
 private[chisel3] class DynamicContext(
-  val annotationSeq:     AnnotationSeq,
-  val throwOnFirstError: Boolean,
-  val warningFilters:    Seq[WarningFilter],
-  val sourceRoots:       Seq[File],
-  val defaultNamespace:  Option[Namespace],
+  val annotationSeq:         AnnotationSeq,
+  val throwOnFirstError:     Boolean,
+  val legacyShiftRightWidth: Boolean,
+  val warningFilters:        Seq[WarningFilter],
+  val sourceRoots:           Seq[File],
+  val defaultNamespace:      Option[Namespace],
   // Definitions from other scopes in the same elaboration, use allDefinitions below
   val outerScopeDefinitions: List[Iterable[Definition[_]]]) {
   val importedDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a }
@@ -926,6 +927,8 @@ private[chisel3] object Builder extends LazyLogging {
     major.toInt
   }
 
+  def legacyShiftRightWidth: Boolean = dynamicContextVar.value.map(_.legacyShiftRightWidth).getOrElse(false)
+
   // Builds a RenameMap for all Views that do not correspond to a single Data
   // These Data give a fake ReferenceTarget for .toTarget and .toReferenceTarget that the returned
   // RenameMap can split into the constituent parts

src/main/scala/chisel3/aop/injecting/InjectingAspect.scala

@@ -61,6 +61,7 @@ abstract class InjectorAspect[T <: RawModule, M <: RawModule](
         new DynamicContext(
           annotationsInAspect,
           chiselOptions.throwOnFirstError,
+          chiselOptions.legacyShiftRightWidth,
           chiselOptions.warningFilters,
           chiselOptions.sourceRoots,
           None,

src/main/scala/chisel3/stage/ChiselAnnotations.scala

@@ -361,3 +361,29 @@ object ChiselOutputFileAnnotation extends HasShellOptions {
   * @tparam DUT Type of the top-level Chisel design
   */
 case class DesignAnnotation[DUT <: RawModule](design: DUT) extends NoTargetAnnotation with Unserializable
+
+/** Use legacy Chisel shift-right behavior
+  *
+  * '''This should only be used for checking for unexpected semantic changes when bumping to Chisel 7.0.0'''
+  *
+  * Use as CLI option `--legacy-shift-right-width`.
+  *
+  * This behavior is inconsistent between Chisel and FIRRTL
+  * - Chisel will report the width of a UInt or SInt shifted right by a number >= its width as a 0-bit value
+  * - FIRRTL will implement the width for these UInts and SInts as 1-bit
+  */
+case object UseLegacyShiftRightWidthBehavior
+    extends NoTargetAnnotation
+    with ChiselOption
+    with HasShellOptions
+    with Unserializable {
+
+  val options = Seq(
+    new ShellOption[Unit](
+      longOption = "legacy-shift-right-width",
+      toAnnotationSeq = _ => Seq(UseLegacyShiftRightWidthBehavior),
+      helpText = "Use legacy (buggy) shift right width behavior (pre Chisel 7.0.0)"
+    )
+  )
+
+}

src/main/scala/chisel3/stage/ChiselOptions.scala

@@ -7,20 +7,22 @@ import chisel3.internal.WarningFilter
 import java.io.File
 
 class ChiselOptions private[stage] (
-  val printFullStackTrace: Boolean = false,
-  val throwOnFirstError:   Boolean = false,
-  val outputFile:          Option[String] = None,
-  val chiselCircuit:       Option[Circuit] = None,
-  val sourceRoots:         Vector[File] = Vector.empty,
-  val warningFilters:      Vector[WarningFilter] = Vector.empty) {
+  val printFullStackTrace:   Boolean = false,
+  val throwOnFirstError:     Boolean = false,
+  val outputFile:            Option[String] = None,
+  val chiselCircuit:         Option[Circuit] = None,
+  val sourceRoots:           Vector[File] = Vector.empty,
+  val warningFilters:        Vector[WarningFilter] = Vector.empty,
+  val legacyShiftRightWidth: Boolean = false) {
 
   private[stage] def copy(
-    printFullStackTrace: Boolean = printFullStackTrace,
-    throwOnFirstError:   Boolean = throwOnFirstError,
-    outputFile:          Option[String] = outputFile,
-    chiselCircuit:       Option[Circuit] = chiselCircuit,
-    sourceRoots:         Vector[File] = sourceRoots,
-    warningFilters:      Vector[WarningFilter] = warningFilters
+    printFullStackTrace:   Boolean = printFullStackTrace,
+    throwOnFirstError:     Boolean = throwOnFirstError,
+    outputFile:            Option[String] = outputFile,
+    chiselCircuit:         Option[Circuit] = chiselCircuit,
+    sourceRoots:           Vector[File] = sourceRoots,
+    warningFilters:        Vector[WarningFilter] = warningFilters,
+    legacyShiftRightWidth: Boolean = legacyShiftRightWidth
   ): ChiselOptions = {
 
     new ChiselOptions(
@@ -29,7 +31,8 @@ class ChiselOptions private[stage] (
       outputFile = outputFile,
       chiselCircuit = chiselCircuit,
       sourceRoots = sourceRoots,
-      warningFilters = warningFilters
+      warningFilters = warningFilters,
+      legacyShiftRightWidth = legacyShiftRightWidth
     )
 
   }

src/main/scala/chisel3/stage/package.scala

@@ -29,6 +29,7 @@ package object stage {
           case SourceRootAnnotation(s)       => c.copy(sourceRoots = c.sourceRoots :+ s)
           case a: WarningConfigurationAnnotation     => c.copy(warningFilters = c.warningFilters ++ a.filters)
           case a: WarningConfigurationFileAnnotation => c.copy(warningFilters = c.warningFilters ++ a.filters)
+          case UseLegacyShiftRightWidthBehavior => c.copy(legacyShiftRightWidth = true)
         }
       }
 

src/main/scala/chisel3/stage/phases/Elaborate.scala

@@ -33,6 +33,7 @@ class Elaborate extends Phase {
           new DynamicContext(
             annotations,
             chiselOptions.throwOnFirstError,
+            chiselOptions.legacyShiftRightWidth,
             chiselOptions.warningFilters,
             chiselOptions.sourceRoots,
             None,

src/main/scala/circt/stage/ChiselStage.scala

@@ -10,6 +10,7 @@ import chisel3.stage.{
   PrintFullStackTraceAnnotation,
   SourceRootAnnotation,
   ThrowOnFirstErrorAnnotation,
+  UseLegacyShiftRightWidthBehavior,
   WarningConfigurationAnnotation,
   WarningConfigurationFileAnnotation,
   WarningsAsErrorsAnnotation
@@ -37,6 +38,7 @@ trait CLI { this: BareShell =>
     ChiselGeneratorAnnotation,
     PrintFullStackTraceAnnotation,
     ThrowOnFirstErrorAnnotation,
+    UseLegacyShiftRightWidthBehavior,
     WarningsAsErrorsAnnotation,
     WarningConfigurationAnnotation,
     WarningConfigurationFileAnnotation,

src/test/scala/chiselTests/ChiselSpec.scala

@@ -23,12 +23,14 @@ import org.scalatest.funspec.AnyFunSpec
 import org.scalatest.matchers.should.Matchers
 import org.scalatest.propspec.AnyPropSpec
 import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks
+import org.scalactic.source.Position
 
 import java.io.{ByteArrayOutputStream, PrintStream}
 import java.security.Permission
 import scala.reflect.ClassTag
 import java.text.SimpleDateFormat
 import java.util.Calendar
+import chisel3.reflect.DataMirror
 
 /** Common utility functions for Chisel unit tests. */
 trait ChiselRunners extends Assertions {
@@ -124,38 +126,50 @@ trait ChiselRunners extends Assertions {
     assert(!runTester(t, additionalVResources, annotations))
   }
 
-  def assertKnownWidth(expected: Int)(gen: => Data): Unit = {
+  def assertKnownWidth(expected: Int, args: Iterable[String] = Nil)(gen: => Data)(implicit pos: Position): Unit = {
     class TestModule extends Module {
       val testPoint = gen
       assert(testPoint.getWidth === expected)
+      val out = IO(Output(chiselTypeOf(testPoint)))
       // Sanity check that firrtl doesn't change the width
-      testPoint := 0.U(0.W).asTypeOf(chiselTypeOf(testPoint))
-      dontTouch(testPoint)
+      val zero = 0.U(0.W).asTypeOf(chiselTypeOf(testPoint))
+      if (DataMirror.isWire(testPoint)) {
+        testPoint := zero
+      }
+      out := zero
+      out := testPoint
     }
-    val verilog = ChiselStage.emitSystemVerilog(new TestModule, Array.empty, Array("-disable-all-randomization"))
+    val verilog = ChiselStage.emitSystemVerilog(new TestModule, args.toArray, Array("-disable-all-randomization"))
     expected match {
-      case 0 => assert(!verilog.contains("testPoint"))
+      case 0 => assert(!verilog.contains("out"))
       case 1 =>
-        assert(verilog.contains(s"testPoint"))
-        assert(!verilog.contains(s"0] testPoint"))
-      case _ => assert(verilog.contains(s"[${expected - 1}:0] testPoint"))
+        assert(verilog.contains(s"out"))
+        assert(!verilog.contains(s"0] out"))
+      case _ => assert(verilog.contains(s"[${expected - 1}:0] out"))
     }
   }
 
-  def assertInferredWidth(expected: Int)(gen: => Data): Unit = {
+  def assertInferredWidth(expected: Int, args: Iterable[String] = Nil)(gen: => Data)(implicit pos: Position): Unit = {
     class TestModule extends Module {
       val testPoint = gen
       assert(!testPoint.isWidthKnown, s"Asserting that width should be inferred yet width is known to Chisel!")
-      testPoint := 0.U(0.W).asTypeOf(chiselTypeOf(testPoint))
-      dontTouch(testPoint)
+      // Sanity check that firrtl doesn't change the width
+      val out = IO(Output(chiselTypeOf(testPoint)))
+      val zero = 0.U(0.W).asTypeOf(chiselTypeOf(testPoint))
+      if (DataMirror.isWire(testPoint)) {
+        testPoint := zero
+      }
+      out := zero
+      out := testPoint
     }
-    val verilog = ChiselStage.emitSystemVerilog(new TestModule, Array.empty, Array("-disable-all-randomization"))
+    val verilog =
+      ChiselStage.emitSystemVerilog(new TestModule, args.toArray :+ "--dump-fir", Array("-disable-all-randomization"))
     expected match {
-      case 0 => assert(!verilog.contains("testPoint"))
+      case 0 => assert(!verilog.contains("out"))
       case 1 =>
-        assert(verilog.contains(s"testPoint"))
-        assert(!verilog.contains(s"0] testPoint"))
-      case _ => assert(verilog.contains(s"[${expected - 1}:0] testPoint"))
+        assert(verilog.contains(s"out"))
+        assert(!verilog.contains(s"0] out"))
+      case _ => assert(verilog.contains(s"[${expected - 1}:0] out"))
     }
   }