[LowerToHW] Add '0' to format string width (#8354)

Add a zero width to all format string substitutions when lowering from
FIRRTL to HW/SV.  E.g., this will lower `%x` into `%0x`.  This is almost
always preferred and avoids the need for (right now) adding width
specifiers to Chisel and FIRRTL.  While we would like to do this, we don't
want to do it as passthrough from Chisel where Chisel users write Verilog
format substitutions.

As @jackkoenig and I got to bottom of, there are non-standard format
specifiers that people might want (e.g., `%-` for left justified) that
tools support differently and we would like to figure out how to actually
lower these if they matter.

Signed-off-by: Schuyler Eldridge <[email protected]>

Author: seldridge

lib/Conversion/FIRRTLToHW/LowerToHW.cpp

@@ -4544,7 +4544,7 @@ LogicalResult FIRRTLLowering::visitStmt(PrintFOp op) {
     return failure();
 
   // Update the format string to replace "special" substitutions based on
-  // substitution type.
+  // substitution type and lower normal substitusion.
   SmallString<32> formatString;
   for (size_t i = 0, e = op.getFormatString().size(), subIdx = 0; i != e; ++i) {
     char c = op.getFormatString()[i];
@@ -4554,17 +4554,21 @@ LogicalResult FIRRTLLowering::visitStmt(PrintFOp op) {
       formatString.push_back(c);
       c = op.getFormatString()[++i];
       switch (c) {
-      // A normal substitution.  Update the substitution index.
+      // A normal substitution.  If this is a radix specifier, add an explicit
+      // `0` width.  E.g., lower `%x` to `%0x`.  Almost nobody wants the Verilog
+      // width behavior and this is a much saner lowering.  Do not do this for
+      // `%c` as the spec is unclear if this is allowed.
       case 'b':
-      case 'c':
       case 'd':
       case 'x':
+        formatString.push_back('0');
+        [[fallthrough]];
+      case 'c':
         ++subIdx;
-        break;
+        [[fallthrough]];
       default:
-        break;
+        formatString.push_back(c);
       }
-      formatString.push_back(c);
       break;
     // Maybe a "{{}}" special substitution.
     case '{': {

test/Conversion/FIRRTLToHW/lower-to-hw-module.mlir

@@ -72,8 +72,8 @@ firrtl.circuit "Simple" {
     %myext:2 = firrtl.instance myext @MyParameterizedExtModule(in in: !firrtl.uint<1>, out out: !firrtl.uint<8>)
 
     // CHECK: %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
-    // CHECK: sv.fwrite %PRINTF_FD_, "%x"(%xyz.out4) : i4
-    // CHECK: sv.fwrite %PRINTF_FD_, "Something interesting! %x"(%myext.out) : i8
+    // CHECK: sv.fwrite %PRINTF_FD_, "%0x"(%xyz.out4) : i4
+    // CHECK: sv.fwrite %PRINTF_FD_, "Something interesting! %0x"(%myext.out) : i8
 
     firrtl.connect %myext#0, %reset : !firrtl.uint<1>, !firrtl.uint<1>
 

test/Conversion/FIRRTLToHW/lower-to-hw.mlir

@@ -326,8 +326,8 @@ firrtl.circuit "Simple"   attributes {annotations = [{class =
 //    input c: SInt<4>
 //    input d: SInt<4>
 //    printf(clock, reset, "No operands!\n")
-//    printf(clock, reset, "Hi %x %x\n", add(a, a), b)
-//    printf(clock, reset, "Hi signed %d %d\n", add(c, c), d)
+//    printf(clock, reset, "Hi %0x %0x\n", add(a, a), b)
+//    printf(clock, reset, "Hi signed %0d %0d\n", add(c, c), d)
 
   // CHECK-LABEL: hw.module private @Print
   // CHECK-SAME: attributes {emit.fragments = [@PRINTF_FD_FRAGMENT, @PRINTF_COND_FRAGMENT]}
@@ -352,22 +352,22 @@ firrtl.circuit "Simple"   attributes {annotations = [{class =
     // CHECK-NEXT:     [[AND:%.+]] = comb.and bin %PRINTF_COND__0, %reset : i1
     // CHECK-NEXT:     sv.if [[AND]] {
     // CHECK-NEXT:       %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
-    // CHECK-NEXT:       sv.fwrite %PRINTF_FD_, "Hi %x %x\0A"([[ADD]], %b) : i5, i4
+    // CHECK-NEXT:       sv.fwrite %PRINTF_FD_, "Hi %0x %0x\0A"([[ADD]], %b) : i5, i4
     // CHECK-NEXT:     }
     // CHECK-NEXT:     %PRINTF_COND__1 = sv.macro.ref.expr @PRINTF_COND_() : () -> i1
     // CHECK-NEXT:     [[AND:%.+]] = comb.and bin %PRINTF_COND__1, %reset : i1
     // CHECK-NEXT:     sv.if [[AND]] {
     // CHECK-NEXT:       %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
     // CHECK-NEXT:       [[SUMSIGNED:%.+]] = sv.system "signed"([[ADDSIGNED]])
     // CHECK-NEXT:       [[DSIGNED:%.+]] = sv.system "signed"(%d)
-    // CHECK-NEXT:       sv.fwrite %PRINTF_FD_, "Hi signed %d %d\0A"([[SUMSIGNED]], [[DSIGNED]]) : i5, i4
+    // CHECK-NEXT:       sv.fwrite %PRINTF_FD_, "Hi signed %0d %0d\0A"([[SUMSIGNED]], [[DSIGNED]]) : i5, i4
     // CHECK-NEXT:     }
     // CHECK-NEXT:     %PRINTF_COND__2 = sv.macro.ref.expr @PRINTF_COND_() : () -> i1
     // CHECK-NEXT:     [[AND:%.+]] = comb.and bin %PRINTF_COND__2, %reset : i1
     // CHECK-NEXT:     sv.if [[AND]] {
     // CHECK-NEXT:       %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
     // CHECK-NEXT:       [[TIME:%.+]] = sv.system.time : i64
-    // CHECK-NEXT:       sv.fwrite %PRINTF_FD_, "[%0t]: %d"([[TIME]], %a) : i64, i4
+    // CHECK-NEXT:       sv.fwrite %PRINTF_FD_, "[%0t]: %0d"([[TIME]], %a) : i64, i4
     // CHECK-NEXT:     }
     // CHECK-NEXT:   }
     // CHECK-NEXT: }

test/firtool/import-ref.fir

@@ -17,7 +17,7 @@ circuit TestHarness:
     connect dut.clock, clock
 
     ; CHECK: fwrite
-    ; CHECK-SAME: "%x", TestHarness.dut.`ref_DUT_read)
+    ; CHECK-SAME: "%0x", TestHarness.dut.`ref_DUT_read)
     printf(clock, UInt<1>(1), "%x", read(dut.read))
     ; CHECK: initial
     ; CHECK: force TestHarness.dut.`ref_DUT_write = 32'hDEADBEEF;

test/firtool/lower-layers.fir

@@ -132,7 +132,7 @@ circuit TestHarness:
   ; CHECK:   `ifndef SYNTHESIS
   ; CHECK:     always @(posedge clock) begin
   ; CHECK:       if ((`PRINTF_COND_) & reset)
-  ; CHECK:         $fwrite(`PRINTF_FD_, "The last PC was: %x", dut_trace);
+  ; CHECK:         $fwrite(`PRINTF_FD_, "The last PC was: %0x", dut_trace);
   ; CHECK:     end // always @(posedge)
   ; CHECK:   `endif // not def SYNTHESIS
   ; CHECK: endmodule

test/firtool/print.fir

@@ -7,7 +7,7 @@ circuit PrintTest:
     input clock : Clock
     input cond : UInt<1>
     input var : UInt<32>
-    printf(clock, cond, "test %d\n", var)
+    printf(clock, cond, "test %b %c %d %x\n", var, var, var, var)
 
     ; CHECK:      sv.ifdef  @SYNTHESIS {
     ; CHECK-NEXT: } else {
@@ -16,7 +16,7 @@ circuit PrintTest:
     ; CHECK-NEXT:     [[COND:%.+]] = comb.and bin [[PRINTF_COND]], %cond : i1
     ; CHECK-NEXT:     sv.if [[COND]] {
     ; CHECK-NEXT:       [[PRINTF_FD:%.+]] = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
-    ; CHECK-NEXT:       sv.fwrite [[PRINTF_FD]], "test %d\0A"(%var) : i32
+    ; CHECK-NEXT:       sv.fwrite [[PRINTF_FD]], "test %0b %c %0d %0x\0A"(%var, %var, %var, %var) : i32
     ; CHECK-NEXT:     }
     ; CHECK-NEXT:   }
     ; CHECK-NEXT: }