Check sign of primop constants where appropriate (#1421)

* Avoid IndexOutOfBoundsException when Bits has too few consts

* Check for negative consts in all relevant primops

* Use BigInt for all checks on primop constants

2 files changed, 31 insertions, 21 deletions

diff --git a/src/main/scala/firrtl/passes/Checks.scala b/src/main/scala/firrtl/passes/Checks.scala
index 934aaed3..a5f66a55 100644
--- a/src/main/scala/firrtl/passes/Checks.scala
+++ b/src/main/scala/firrtl/passes/Checks.scala
@@ -54,9 +54,9 @@ trait CheckHighFormLike {
     s"$info: [module $mname] Has instance loop $loop")
   class NoTopModuleException(info: Info, name: String) extends PassException(
     s"$info: A single module must be named $name.")
-  class NegArgException(info: Info, mname: String, op: String, value: Int) extends PassException(
+  class NegArgException(info: Info, mname: String, op: String, value: BigInt) extends PassException(
     s"$info: [module $mname] Primop $op argument $value < 0.")
-  class LsbLargerThanMsbException(info: Info, mname: String, op: String, lsb: Int, msb: Int) extends PassException(
+  class LsbLargerThanMsbException(info: Info, mname: String, op: String, lsb: BigInt, msb: BigInt) extends PassException(
     s"$info: [module $mname] Primop $op lsb $lsb > $msb.")
   class ResetInputException(info: Info, mname: String, expr: Expression) extends PassException(
     s"$info: [module $mname] Abstract Reset not allowed as top-level input: ${expr.serialize}")
@@ -83,24 +83,31 @@ trait CheckHighFormLike {
           errors.append(new IncorrectNumConstsException(info, mname, e.op.toString, nc))
       }
 
+      def nonNegativeConsts(): Unit = {
+        e.consts.filter(_ < 0).foreach {
+          negC => errors.append(new NegArgException(info, mname, e.op.toString, negC))
+        }
+      }
+
       e.op match {
         case Add | Sub | Mul | Div | Rem | Lt | Leq | Gt | Geq |
              Eq | Neq | Dshl | Dshr | And | Or | Xor | Cat | Dshlw | Clip | Wrap | Squeeze =>
           correctNum(Option(2), 0)
         case AsUInt | AsSInt | AsClock | AsAsyncReset | Cvt | Neq | Not =>
           correctNum(Option(1), 0)
-        case AsFixedPoint | Pad | Head | Tail | IncP | DecP | SetP =>
+        case AsFixedPoint | SetP =>
           correctNum(Option(1), 1)
-        case Shl | Shr =>
+        case Shl | Shr | Pad | Head | Tail | IncP | DecP =>
           correctNum(Option(1), 1)
-          val amount = e.consts.map(_.toInt).filter(_ < 0).foreach {
-            c => errors.append(new NegArgException(info, mname, e.op.toString, c))
-          }
+          nonNegativeConsts()
         case Bits =>
           correctNum(Option(1), 2)
-          val (msb, lsb) = (e.consts(0).toInt, e.consts(1).toInt)
-          if (lsb > msb) {
-            errors.append(new LsbLargerThanMsbException(info, mname, e.op.toString, lsb, msb))
+          nonNegativeConsts()
+          if (e.consts.length == 2) {
+            val (msb, lsb) = (e.consts(0), e.consts(1))
+            if (lsb > msb) {
+              errors.append(new LsbLargerThanMsbException(info, mname, e.op.toString, lsb, msb))
+            }
           }
         case AsInterval =>
           correctNum(Option(1), 3)
diff --git a/src/test/scala/firrtlTests/CheckSpec.scala b/src/test/scala/firrtlTests/CheckSpec.scala
index 96275003..9a384d21 100644
--- a/src/test/scala/firrtlTests/CheckSpec.scala
+++ b/src/test/scala/firrtlTests/CheckSpec.scala
@@ -267,7 +267,7 @@ class CheckSpec extends FlatSpec with Matchers {
     }
   }
 
-  for (op <- List("shl", "shr")) {
+  for (op <- List("shl", "shr", "pad", "head", "tail", "incp", "decp")) {
     s"$op by negative amount" should "result in an error" in {
       val amount = -1
       val input =
@@ -283,16 +283,19 @@ class CheckSpec extends FlatSpec with Matchers {
     }
   }
 
-  "LSB larger than MSB in bits" should "throw an exception" in {
-    val input =
-      """|circuit bar :
-         |  module bar :
-         |    input in : UInt<8>
-         |    output foo : UInt
-         |    foo <= bits(in, 3, 4)
-         |      """.stripMargin
-    val exception = intercept[PassException] {
-      checkHighInput(input)
+  // Check negative bits constant, too
+  for (args <- List((3, 4), (0, -1))) {
+    val opExp = s"bits(in, ${args._1}, ${args._2})"
+    s"Illegal bit extract ${opExp}" should "throw an exception" in {
+      val input =
+        s"""|circuit bar :
+            |  module bar :
+            |    input in : UInt<8>
+            |    output foo : UInt
+            |    foo <= ${opExp}""".stripMargin
+      val exception = intercept[PassException] {
+        checkHighInput(input)
+      }
     }
   }