Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIRRTL] Use PRINTF_FD macro instead of 0x80000002 as printf fd #7983

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+33 −18
Open

[FIRRTL] Use PRINTF_FD macro instead of 0x80000002 as printf fd #7983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions lib/Conversion/FIRRTLToHW/LowerToHW.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ struct FIRRTLModuleLowering;
/// This is state shared across the parallel module lowering logic.
struct CircuitLoweringState {
// Flags indicating whether the circuit uses certain header fragments.
std::atomic<bool> usedPrintfCond{false};
std::atomic<bool> usedPrintf{false};
std::atomic<bool> usedAssertVerboseCond{false};
std::atomic<bool> usedStopCond{false};

Expand Down Expand Up @@ -809,7 +809,19 @@ void FIRRTLModuleLowering::lowerFileHeader(CircuitOp op,
guard, []() {}, body);
};

if (state.usedPrintfCond) {
if (state.usedPrintf) {
b.create<sv::MacroDeclOp>("PRINTF_FD");
b.create<sv::MacroDeclOp>("PRINTF_FD_");
b.create<emit::FragmentOp>("PRINTF_FD_FRAGMENT", [&] {
b.create<sv::VerbatimOp>(
"\n// Users can define 'PRINTF_FD' to add a specified fd to "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we add comments for other chisel macros? I didn't think we do. There should be a user friendly form of the chisel abi which outlines the macros created to control these things.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we add comments for other chisel macros?

These macro are not exposed in Chisel, I think Chisel doesn't have a clear ABI for now.

But instead we should add these to firrtl spec, and chisel users can be point to there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty consistent with how we currently handle other un-documented things. E.g., PRINTF_COND has the exact same language.

"prints.");
emitGuard("PRINTF_FD_", [&]() {
emitGuardedDefine("PRINTF_FD", "PRINTF_FD_", "(`PRINTF_FD)",
"32'h80000002");
});
});

b.create<sv::MacroDeclOp>("PRINTF_COND");
b.create<sv::MacroDeclOp>("PRINTF_COND_");
b.create<emit::FragmentOp>("PRINTF_COND_FRAGMENT", [&] {
Expand Down Expand Up @@ -4416,7 +4428,8 @@ LogicalResult FIRRTLLowering::visitStmt(PrintFOp op) {
circuitState.addMacroDecl(builder.getStringAttr("SYNTHESIS"));
addToIfDefBlock("SYNTHESIS", std::function<void()>(), [&]() {
addToAlwaysBlock(clock, [&]() {
circuitState.usedPrintfCond = true;
circuitState.usedPrintf = true;
circuitState.addFragment(theModule, "PRINTF_FD_FRAGMENT");
circuitState.addFragment(theModule, "PRINTF_COND_FRAGMENT");

// Emit an "sv.if '`PRINTF_COND_ & cond' into the #ifndef.
Expand All @@ -4425,9 +4438,10 @@ LogicalResult FIRRTLLowering::visitStmt(PrintFOp op) {
ifCond = builder.createOrFold<comb::AndOp>(ifCond, cond, true);

addIfProceduralBlock(ifCond, [&]() {
// Emit the sv.fwrite, writing to stderr by default.
Value fdStderr = builder.create<hw::ConstantOp>(APInt(32, 0x80000002));
builder.create<sv::FWriteOp>(fdStderr, op.getFormatString(), operands);
// Emit the sv.fwrite, writing to fd specified by `PRINTF_FD.
Value fd =
builder.create<sv::MacroRefExprOp>(builder.getIntegerType(32), "PRINTF_FD_");
builder.create<sv::FWriteOp>(fd, op.getFormatString(), operands);
});
});
});
Expand Down
6 changes: 3 additions & 3 deletions test/Conversion/FIRRTLToHW/lower-to-hw-module.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ firrtl.circuit "Simple" {
// CHECK: %myext.out = hw.instance "myext" @MyParameterizedExtModule<DEFAULT: i64 = 0, DEPTH: f64 = 3.242000e+01, FORMAT: none = "xyz_timeout=%d\0A", WIDTH: i8 = 32>(in: %reset: i1) -> (out: i8)
%myext:2 = firrtl.instance myext @MyParameterizedExtModule(in in: !firrtl.uint<1>, out out: !firrtl.uint<8>)

// CHECK: [[FD:%.*]] = hw.constant -2147483646 : i32
// CHECK: sv.fwrite [[FD]], "%x"(%xyz.out4) : i4
// CHECK: sv.fwrite [[FD]], "Something interesting! %x"(%myext.out) : i8
// 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

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

Expand Down
14 changes: 7 additions & 7 deletions test/Conversion/FIRRTLToHW/lower-to-hw.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class =
// printf(clock, reset, "Hi signed %d %d\n", add(c, c), d)

// CHECK-LABEL: hw.module private @Print
// CHECK-SAME: attributes {emit.fragments = [@PRINTF_COND_FRAGMENT]}
// CHECK-SAME: attributes {emit.fragments = [@PRINTF_FD_FRAGMENT, @PRINTF_COND_FRAGMENT]}
firrtl.module private @Print(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>,
in %a: !firrtl.uint<4>, in %b: !firrtl.uint<4>,
in %c: !firrtl.sint<4>, in %d: !firrtl.sint<4>) {
Expand All @@ -347,20 +347,20 @@ firrtl.circuit "Simple" attributes {annotations = [{class =
// CHECK-NEXT: %PRINTF_COND_ = sv.macro.ref.expr @PRINTF_COND_() : () -> i1
// CHECK-NEXT: [[AND:%.+]] = comb.and bin %PRINTF_COND_, %reset
// CHECK-NEXT: sv.if [[AND]] {
// CHECK-NEXT: [[FD:%.+]] = hw.constant -2147483646 : i32
// CHECK-NEXT: sv.fwrite [[FD]], "No operands!\0A"
// CHECK-NEXT: %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "No operands!\0A"
// CHECK-NEXT: }
// CHECK-NEXT: %PRINTF_COND__0 = sv.macro.ref.expr @PRINTF_COND_() : () -> i1
// CHECK-NEXT: [[AND:%.+]] = comb.and bin %PRINTF_COND__0, %reset : i1
// CHECK-NEXT: sv.if [[AND]] {
// CHECK-NEXT: [[FD:%.+]] = hw.constant -2147483646 : i32
// CHECK-NEXT: sv.fwrite [[FD]], "Hi %x %x\0A"([[ADD]], %b) : i5, i4
// 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: }
// 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: [[FD:%.+]] = hw.constant -2147483646 : i32
// CHECK-NEXT: sv.fwrite [[FD]], "Hi signed %d %d\0A"([[SUMSIGNED]], [[DSIGNED]]) : i5, i4
// CHECK-NEXT: %PRINTF_FD_ = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
// CHECK-NEXT: sv.fwrite %PRINTF_FD_, "Hi signed %d %d\0A"([[SUMSIGNED]], [[DSIGNED]]) : i5, i4
// CHECK-NEXT: }
// CHECK-NEXT: }
// CHECK-NEXT: }
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/lower-layers.fir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ circuit TestHarness:
; CHECK: `ifndef SYNTHESIS
; CHECK: always @(posedge clock) begin
; CHECK: if ((`PRINTF_COND_) & reset)
; CHECK: $fwrite(32'h80000002, "The last PC was: %x", dut_trace);
; CHECK: $fwrite(`PRINTF_FD_, "The last PC was: %x", dut_trace);
; CHECK: end // always @(posedge)
; CHECK: `endif // not def SYNTHESIS
; CHECK: endmodule
Expand Down
3 changes: 2 additions & 1 deletion test/firtool/print.fir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ circuit PrintTest:
; CHECK-NEXT: [[PRINTF_COND:%.+]] = sv.macro.ref.expr @PRINTF_COND_() : () -> i1
; CHECK-NEXT: [[COND:%.+]] = comb.and bin [[PRINTF_COND]], %cond : i1
; CHECK-NEXT: sv.if [[COND]] {
; CHECK-NEXT: sv.fwrite %c-2147483646_i32, "test %d\0A"(%var) : i32
; CHECK-NEXT: [[PRINTF_FD:%.+]] = sv.macro.ref.expr @PRINTF_FD_() : () -> i32
; CHECK-NEXT: sv.fwrite [[PRINTF_FD]], "test %d\0A"(%var) : i32
; CHECK-NEXT: }
; CHECK-NEXT: }
; CHECK-NEXT: }