Skip to content

Commit bfdda70

Browse files
seldridgeseldridge
committed
[LowerToHW] Revert a1b7ca6 '%0d' default
Revert a change introduced in a1b7ca6 that would cause all FIRRTL radix substitutions to get an automatic `%0` width specifier. While this is a sane default, this exposed situations where users were relying on this behavior as an actual interface. I.e., while this was brought up as a stylistic concern, it was also a functional breakge. Signed-off-by: Schuyler Eldridge <[email protected]>
1 parent 25b6e03 commit bfdda70

File tree

6 files changed

+16
-20
lines changed

6 files changed

+16
-20
lines changed

‎lib/Conversion/FIRRTLToHW/LowerToHW.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4575,16 +4575,12 @@ LogicalResult FIRRTLLowering::visitStmt(PrintFOp op) {
45754575

45764576
// Parse the radix.
45774577
switch (c) {
4578-
// A normal substitution. If this is a radix specifier, add an explicit
4579-
// `0` width. E.g., lower `%x` to `%0x`. Almost nobody wants the Verilog
4580-
// width behavior and this is a much saner lowering. Do not do this for
4581-
// `%c` as the spec is unclear if this is allowed.
4578+
// A normal substitution. If this is a radix specifier, include the width
4579+
// if one exists.
45824580
case 'b':
45834581
case 'd':
45844582
case 'x':
4585-
if (width.empty())
4586-
formatString.push_back('0');
4587-
else
4583+
if (!width.empty())
45884584
formatString.append(width);
45894585
[[fallthrough]];
45904586
case 'c':

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ firrtl.circuit "Simple" {
7272
%myext:2 = firrtl.instance myext @MyParameterizedExtModule(in in: !firrtl.uint<1>, out out: !firrtl.uint<8>)
7373

7474
// CHECK: %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
75-
// CHECK: sv.fwrite %PRINTF_FD_, "%0x"(%xyz.out4) : i4
76-
// CHECK: sv.fwrite %PRINTF_FD_, "Something interesting! %0x"(%myext.out) : i8
75+
// CHECK: sv.fwrite %PRINTF_FD_, "%x"(%xyz.out4) : i4
76+
// CHECK: sv.fwrite %PRINTF_FD_, "Something interesting! %x"(%myext.out) : i8
7777

7878
firrtl.connect %myext#0, %reset : !firrtl.uint<1>, !firrtl.uint<1>
7979

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class =
327327
// input d: SInt<4>
328328
// printf(clock, reset, "No operands!\n")
329329
// printf(clock, reset, "Hi %0x %0x\n", add(a, a), b)
330-
// printf(clock, reset, "Hi signed %0d %0d\n", add(c, c), d)
330+
// printf(clock, reset, "Hi signed %d %0d\n", add(c, c), d)
331331

332332
// CHECK-LABEL: hw.module private @Print
333333
// CHECK-SAME: attributes {emit.fragments = [@PRINTF_FD_FRAGMENT, @PRINTF_COND_FRAGMENT]}
@@ -352,19 +352,19 @@ firrtl.circuit "Simple" attributes {annotations = [{class =
352352
// CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1
353353
// CHECK-NEXT: sv.if [[AND]] {
354354
// CHECK-NEXT: %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
355-
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "Binary: %0b %0b %4b\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5
355+
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "Binary: %b %0b %4b\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5
356356
// CHECK-NEXT: }
357357
// CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1
358358
// CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1
359359
// CHECK-NEXT: sv.if [[AND]] {
360360
// CHECK-NEXT: %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
361-
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "Decimal: %0d %0d %4d\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5
361+
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "Decimal: %d %0d %4d\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5
362362
// CHECK-NEXT: }
363363
// CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1
364364
// CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1
365365
// CHECK-NEXT: sv.if [[AND]] {
366366
// CHECK-NEXT: %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
367-
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "Hexadecimal: %0x %0x %4x\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5
367+
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "Hexadecimal: %x %0x %4x\0A"([[ADD]], %b, [[ADD]]) : i5, i4, i5
368368
// CHECK-NEXT: }
369369
// CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1
370370
// CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1
@@ -378,14 +378,14 @@ firrtl.circuit "Simple" attributes {annotations = [{class =
378378
// CHECK-NEXT: %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
379379
// CHECK-NEXT: [[SUMSIGNED:%.+]] = sv.system "signed"([[ADDSIGNED]])
380380
// CHECK-NEXT: [[DSIGNED:%.+]] = sv.system "signed"(%d)
381-
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "Hi signed %0d %0d\0A"([[SUMSIGNED]], [[DSIGNED]]) : i5, i4
381+
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "Hi signed %d %d\0A"([[SUMSIGNED]], [[DSIGNED]]) : i5, i4
382382
// CHECK-NEXT: }
383383
// CHECK-NEXT: %[[PRINTF_COND_:.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1
384384
// CHECK-NEXT: [[AND:%.+]] = comb.and bin %[[PRINTF_COND_]], %reset : i1
385385
// CHECK-NEXT: sv.if [[AND]] {
386386
// CHECK-NEXT: %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
387387
// CHECK-NEXT: [[TIME:%.+]] = sv.system.time : i64
388-
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "[%0t]: %0d %m"([[TIME]], %a) : i64, i4
388+
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "[%0t]: %d %m"([[TIME]], %a) : i64, i4
389389
// CHECK-NEXT: }
390390
// CHECK-NEXT: }
391391
// CHECK-NEXT: }

‎test/firtool/import-ref.fir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ circuit TestHarness:
1717
connect dut.clock, clock
1818

1919
; CHECK: fwrite
20-
; CHECK-SAME: "%0x", TestHarness.dut.`ref_DUT_read)
20+
; CHECK-SAME: "%x", TestHarness.dut.`ref_DUT_read)
2121
printf(clock, UInt<1>(1), "%x", read(dut.read))
2222
; CHECK: initial
2323
; CHECK: force TestHarness.dut.`ref_DUT_write = 32'hDEADBEEF;

‎test/firtool/lower-layers.fir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ circuit TestHarness:
132132
; CHECK: `ifndef SYNTHESIS
133133
; CHECK: always @(posedge clock) begin
134134
; CHECK: if ((`PRINTF_COND_) & reset)
135-
; CHECK: $fwrite(`PRINTF_FD_, "The last PC was: %0x", dut_trace);
135+
; CHECK: $fwrite(`PRINTF_FD_, "The last PC was: %x", dut_trace);
136136
; CHECK: end // always @(posedge)
137137
; CHECK: `endif // not def SYNTHESIS
138138
; CHECK: endmodule

‎test/firtool/print.fir

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ circuit PrintTest:
99
input a : UInt<8>
1010

1111

12-
; CHECK: $fwrite(`PRINTF_FD_, "binary: %0b %0b %8b\n", a, a, a);
12+
; CHECK: $fwrite(`PRINTF_FD_, "binary: %b %0b %8b\n", a, a, a);
1313
printf(clock, cond, "binary: %b %0b %8b\n", a, a, a)
1414

15-
; CHECK-NEXT: $fwrite(`PRINTF_FD_, "decimal: %0d %0d %3d\n", a, a, a);
15+
; CHECK-NEXT: $fwrite(`PRINTF_FD_, "decimal: %d %0d %3d\n", a, a, a);
1616
printf(clock, cond, "decimal: %d %0d %3d\n", a, a, a)
1717

18-
; CHECK-NEXT: $fwrite(`PRINTF_FD_, "hexadecimal: %0x %0x %2x\n", a, a, a);
18+
; CHECK-NEXT: $fwrite(`PRINTF_FD_, "hexadecimal: %x %0x %2x\n", a, a, a);
1919
printf(clock, cond, "hexadecimal: %x %0x %2x\n", a, a, a)
2020

2121
; CHECK-NEXT: $fwrite(`PRINTF_FD_, "ASCII character: %c\n", a);

0 commit comments

Comments
 (0)