From e5f0c5065e6af2e07f23ba87a16edd385bbdf17e Mon Sep 17 00:00:00 2001
From: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Date: Mon, 6 Dec 2021 17:03:11 -0500
Subject: [PATCH] [FIRRTL] Parse params < 32-bit as 32-bit (#2300)

Change FIRRTL parsing to convert all parameters less than 32-bit to
32-bit APInts.  This fixes downstream issues with Verilog emission,
where negative parameters may have too few bits in their APInt
representation and print as shorter-than-expected literals.

Fixes #2299.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
---
 lib/Dialect/FIRRTL/Import/FIRParser.cpp     |  5 +++++
 test/Dialect/FIRRTL/SFCTests/parameters.fir | 18 ++++++++++++++++++
 test/Dialect/FIRRTL/parse-basic.fir         |  4 ++--
 3 files changed, 25 insertions(+), 2 deletions(-)
 create mode 100644 test/Dialect/FIRRTL/SFCTests/parameters.fir

diff --git a/lib/Dialect/FIRRTL/Import/FIRParser.cpp b/lib/Dialect/FIRRTL/Import/FIRParser.cpp
index 342d83fa21..e2b52ee50a 100644
--- a/lib/Dialect/FIRRTL/Import/FIRParser.cpp
+++ b/lib/Dialect/FIRRTL/Import/FIRParser.cpp
@@ -3404,6 +3404,11 @@ ParseResult FIRCircuitParser::parseModule(CircuitOp circuit,
       if (parseIntLit(result, "invalid integer parameter"))
         return failure();
 
+      // If the integer parameter is less than 32-bits, sign extend this to a
+      // 32-bit value.  This needs to eventually emit as a 32-bit value in
+      // Verilog and we want to get the size correct immediately.
+      result = result.sextOrSelf(32);
+
       value = builder.getIntegerAttr(
           builder.getIntegerType(result.getBitWidth()), result);
       break;
diff --git a/test/Dialect/FIRRTL/SFCTests/parameters.fir b/test/Dialect/FIRRTL/SFCTests/parameters.fir
new file mode 100644
index 0000000000..11639ef274
--- /dev/null
+++ b/test/Dialect/FIRRTL/SFCTests/parameters.fir
@@ -0,0 +1,18 @@
+; RUN: firtool -split-input-file -verilog %s | FileCheck %s
+
+; Test that a negative parameter prints out as a 32-bit parameter.  It is fine
+; to change this test to print as "-1" in the output Verilog, but not as a
+; non-32-bit "-1" like "0xF".
+circuit NegativeParameter:
+  extmodule Foo:
+    output a: UInt<1>
+    parameter x = -1
+  module NegativeParameter:
+    output a: UInt<1>
+
+    inst foo of Foo
+    a <= foo.a
+
+; CHECK-LABEL: module NegativeParameter
+; CHECK:         Foo #(
+; CHECK-NEXT:      .x(4294967295)
diff --git a/test/Dialect/FIRRTL/parse-basic.fir b/test/Dialect/FIRRTL/parse-basic.fir
index b690fbee1e..ea49e2b787 100644
--- a/test/Dialect/FIRRTL/parse-basic.fir
+++ b/test/Dialect/FIRRTL/parse-basic.fir
@@ -32,7 +32,7 @@ circuit MyModule :     ; CHECK: firrtl.circuit "MyModule" {
   ; CHECK: parameters = {DEFAULT = 0 : i32,
   ; CHECK:               DEPTH = 3.242000e+01 : f64,
   ; CHECK:               FORMAT = "xyz_timeout=%d\0A",
-  ; CHECK:               WIDTH = 32 : i8}}
+  ; CHECK:               WIDTH = 32 : i32}}
   ; CHECK-NOT: {
   extmodule MyParameterizedExtModule :
     input in: UInt
@@ -576,7 +576,7 @@ circuit MyModule :     ; CHECK: firrtl.circuit "MyModule" {
     ; CHECK: %c-4_si4 = firrtl.constant -4 : !firrtl.sint<4>
 
   ; CHECK-LABEL: firrtl.extmodule @issue183()
-  ; CHECK: attributes {parameters = {A = -1 : i4}}
+  ; CHECK: attributes {parameters = {A = -1 : i32}}
   extmodule issue183:
      parameter A = -1