TLDR; To make ExportVerilog less monolithic and to do pretty printing in the future, I think ExportVerilog would need some refactoring. As a first step, I’d like to move the AST legalization logic of ExportVerilog into PrepareForEmisson so that ExportVerilog doesn’t have to care about the legality of expressions.
Introduction
ExportVerilog is emitting production-quality verilog but one major missing feature is prettify-printing like clang-format or verible. Towards prettify-formating, I think we first need to fix the situation where ExportVerilog legalizes verilog expressions while emitting them. Let me first describe the current emission way.
hw.module @Foo(%a: i4, %b: i4) -> (c: i1) {
%0 = comb.add %a, %b : i4
%1 = comb.extract %0 from 3 : (i4) -> i1
hw.output %1 : i1
}
ExportVerilog emits the following verilog:
wire [3:0] _GEN = a + b;
assign c = _GEN[3];
%0 requires a temporary wire because (a+b)[3] is illegal in verilog. ExportVerilog dynamically checks the legality of expressions with isExpressionUnableToInline and spills a temporary wire if it is illegal. This procedure makes pretty-print difficult because it is hard to predict outputs from the IR. Hence, I want to perform the legalization in PrepareForEmission so that ExportVerilog doesn’t have to worry about legality, but can focus on prettify-printing of IR.
Proposal
Here, I’d like to describe how to legalize IR at PrepareForEmission.
Input:
hw.module @Foo(%a: i4, %b: i4) -> (c: i1) {
%0 = comb.add %a, %b : i4
%1 = comb.extract %0 from 3 : (i4) -> i1
hw.output %1 : i1
}
After PrepareForEmission (expected):
hw.module @Foo(%a: i4, %b: i4) -> (c: i1) {
%0 = comb.add %a, %b : i4
// Explicitly create a wire op for a temporary wire
%1 = sv.wire : !hw.inout<i4>
sv.assign %1, %0 : i4
%2 = sv.read_inout %1 : !hw.inout<i4>
%3 = comb.extract %2 from 3 : (i4) -> i1
hw.output %3 : i1
}
PrepareForEmission performs legalization by introducing a wire op for %0. sv.wire/sv.assign/sv.read_inout will be used here. One concern is that this legalization will create at least three operations per temporary wire so it might dramatically increase the size of IR.
ExportVerilog (now):
wire [3:0] _GEN;
assign _GEN = a + b;
assign c = _GEN[3];
This is the current output for the IR above. Currently, wire ops are forwardly declared so it would be necessary to stop the forward declaration of wire ops.
ExportVerilog (expected):
wire [3:0] _GEN = a + b;
assign c = _GEN[3];
Once we can stop forward declarations of wire ops, the output should be exactly the same as now.
Plan
- Add
spillTemporaryAtPrepareto the lowering option to selectively enable the new lowering. - Expose
isExpessionUnableToInlineto PrepareForEmission and callisExpressionUnableToInlinefromshouldSpillWirein Prepare. - Remove
outOfLineExpressionsfromModuleEmitterto stop unnecessary forward declaration of wire/regs. This part will require a lot of changes to ExportVerilog. - Make
spillTemporaryAtPreparedefault and remove the option.
Any feedback and comment are highly appreciated!
