simulation-driver's scanHexBits() handles the case where a negati…

…ve number `-1<<(bitWidth-1)` with `bitWidth % 8 == 1` is supplied.
chipsalliance · Jan 8, 2025 · b92be13 · b92be13
1 parent add78df
commit b92be13
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 13 deletions.
diff --git a/svsim/src/main/resources/simulation-driver.cpp b/svsim/src/main/resources/simulation-driver.cpp
@@ -440,21 +440,18 @@ static uint8_t *scanHexBits(const char **scanCursor, const char *scanEnd,
   }
 
   bool isNegative;
-  int valueBitCount;
   if (**scanCursor == '-') {
     (*scanCursor)++;
     if (reverseScanCursor < *scanCursor) {
       failWithError("Unexpected end of negative value when %s.", description);
     }
     isNegative = true;
-    if (bitCount <= 1) {
-      failWithError("Cannot scan 1-bit-wide negative value when %s.",
+    if (bitCount < 1) {
+      failWithError("Cannot scan 0-bit-wide negative value when %s.",
                     description);
     }
-    valueBitCount = bitCount - 1;
   } else {
     isNegative = false;
-    valueBitCount = bitCount;
   }
 
   int byteCount = (bitCount + 7) / 8;
@@ -464,7 +461,7 @@ static uint8_t *scanHexBits(const char **scanCursor, const char *scanEnd,
   const char *firstCharacterOfValue = *scanCursor;
   int carry = 1; // Only used when `isNegative` is true
   int scannedByteCount = 0;
-  int valueByteCount = (valueBitCount + 7) / 8;
+  int valueByteCount = (bitCount + 7) / 8;
   while (scannedByteCount < valueByteCount) {
     int scannedByte = scanHexByteReverse(&reverseScanCursor,
                                          firstCharacterOfValue, description);
@@ -490,11 +487,10 @@ static uint8_t *scanHexBits(const char **scanCursor, const char *scanEnd,
   // A mask of the "inapplicable" bits in the high order byte, used to determine
   // if we received too many bits for the value we are trying to scan. This
   // value could be calculated with bitwise operations, but I find a table to be
-  // cleaner and easier to understand. We use `valueBitCount` instead of
-  // `bitCount` because the sign bit should be `1` for negative numbers along
-  // with all of the other leading bits.
+  // cleaner and easier to understand. There's no sign bit in Scala's `BigInt.toString(16)`,
+  // instead a minus sign will be present.
   uint8_t highOrderByteMask;
-  switch (valueBitCount % 8) {
+  switch (bitCount % 8) {
   case 1:
     highOrderByteMask = 0b11111110;
     break;

diff --git a/svsim/src/test/scala/BackendSpec.scala b/svsim/src/test/scala/BackendSpec.scala
@@ -230,8 +230,7 @@ trait BackendSpec extends AnyFunSpec with Matchers {
           verbose = false,
           executionScriptLimit = None
         ) { controller =>
-          // When bitWidth % 8 == 1, scanHexBits() will infer the wrong size
-          val bitWidths: Seq[Int] = List(8, 31, 32)
+          val bitWidths: Seq[Int] = List(8, 31, 32, 33)
           val inPorts = bitWidths.map(b => controller.port(s"in_${b}"))
           val outPorts = bitWidths.map(b => controller.port(s"out_${b}"))
 
@@ -241,7 +240,7 @@ trait BackendSpec extends AnyFunSpec with Matchers {
           def boundValues(bitWidth: Int): Seq[BigInt] = {
             val minVal = BigInt(-1) << (bitWidth - 1)
             val maxVal = (BigInt(1) << (bitWidth - 1)) - 1
-            val deltaRange = maxVal.min(BigInt(3))
+            val deltaRange = maxVal.min(BigInt(257))
             val valueNearZero = for { v <- -deltaRange to deltaRange} yield v
             val valueNearMax = for {delta <- BigInt(0) to deltaRange} yield maxVal-delta
             val valueNearMin = for {delta <- BigInt(0) to deltaRange} yield minVal+delta