Hello! I have a question about the semantics of concatenation operation. From the FIRRTL spec: cat (e1,e2). The result of the concatenate operation is the bits of e1 concatenated to the most significant end of the bits of e2. How should one interpret it? Should it be like that: e1[we1 - 1]e1[…]e1[0]e2[we2 - 1]e2[…]e2[0] or like that: e2[we2 - 1]e2[…]e2[0]e1[we1 - 1]e1[…]e1[0]? As far as I understand it should be the first option.
I’m asking this question because if the first option is true then it is likely that FIRRTL lowering logic contains a bug.
Yes, it should be the first option. Do you have an example that is showing a miscompilation?
This looks right to me:
FIRRTL version 4.0.0
circuit Foo:
public module Foo:
input e1: UInt<1>
input e2: UInt<1>
output out: UInt<2>
connect out, cat(e1, e2)
This produces:
// Generated by CIRCT firtool-1.70.0-24-g2b482b219
module Foo(
input e1,
e2,
output [1:0] out
);
assign out = {e1, e2};
endmodule
In the Verilog, this means that out[1] is e1 and out[0] is e2.
Thank you for your reply.
The whole example is very big. I can send it to you, but maybe I can describe you the part, where the strange behavior occurs (in my opinion). It is possible that I’m just misunderstanding something.
FIRRTL lowering produces the following structure:
...
%_GEN__317 = firrtl.wire : !firrtl.uint<32>
...
%_GEN__3609 = firrtl.wire : !firrtl.uint<1>
...
%_GEN__4214 = firrtl.wire : !firrtl.uint<32>
...
%1 = firrtl.cat %0, %_GEN__3609 : (!firrtl.uint<31>, !firrtl.uint<1>) -> !firrtl.uint<32>
firrtl.strictconnect %_GEN__4214, %1 : !firrtl.uint<32>
...
%23 = firrtl.constCast %c0_ui1_0 : (!firrtl.const.uint<1>) -> !firrtl.uint<1>
...
firrtl.strictconnect %_GEN__3609, %23 : !firrtl.uint<1>
...
firrtl.strictconnect %_GEN__317, %_GEN__4214 : !firrtl.uint<32>
...
%69 = firrtl.bits %_GEN__317 31 to 1 : (!firrtl.uint<32>) -> !firrtl.uint<31>
...
As far as I understand %1 concatenates the result of the %23 with %0:
%0[31…1]%23[0]. And after that %69 removes the result of the above concatenation which does not make any sense. Judging by firrtl.shr operation the enumeration of the bits is right. Maybe I am wrong somewhere?
Your interpretation seems correct to me.
Here is your example again with some of the holes filled in so the MLIR is legal:
firrtl.circuit "Test" {
firrtl.module @Test(in %0: !firrtl.uint<31>, out %out: !firrtl.uint<31>) {
%c0_ui1_0 = firrtl.constant 0 : !firrtl.const.uint<1>
%_GEN__317 = firrtl.wire : !firrtl.uint<32>
%_GEN__3609 = firrtl.wire : !firrtl.uint<1>
%_GEN__4214 = firrtl.wire : !firrtl.uint<32>
%1 = firrtl.cat %0, %_GEN__3609 : (!firrtl.uint<31>, !firrtl.uint<1>) -> !firrtl.uint<32>
firrtl.strictconnect %_GEN__4214, %1 : !firrtl.uint<32>
%23 = firrtl.constCast %c0_ui1_0 : (!firrtl.const.uint<1>) -> !firrtl.uint<1>
firrtl.strictconnect %_GEN__3609, %23 : !firrtl.uint<1>
firrtl.strictconnect %_GEN__317, %_GEN__4214 : !firrtl.uint<32>
%69 = firrtl.bits %_GEN__317 31 to 1 : (!firrtl.uint<32>) -> !firrtl.uint<31>
firrtl.strictconnect %out, %69 : !firrtl.uint<31>
}
}
And then later in lowering, the HW level canonicalizers see the pointless ops and remove them. I just fed the example in to firtool.
// -----// IR Dump Before Canonicalizer (canonicalize) //----- //
hw.module @Test(in %0 "" : i31, out out : i31) {
%false = hw.constant false
%_GEN__317 = hw.wire %_GEN__4214 : i32
%_GEN__4214 = hw.wire %1 : i32
%1 = comb.concat %0, %false : i31, i1
%2 = comb.extract %_GEN__317 from 1 : (i32) -> i31
hw.output %2 : i31
}
// -----// IR Dump After Canonicalizer (canonicalize) //----- //
hw.module @Test(in %0 "" : i31, out out : i31) {
hw.output %0 : i31
}
Producing this verilog:
// Generated by CIRCT unknown git version
module Test( // ./test.mlir:2:3
input [30:0] _GEN, // ./test.mlir:2:26
output [30:0] out // ./test.mlir:2:52
);
assign out = _GEN; // ./test.mlir:2:3
endmodule
I’m not entirely following the example, but it sounds like you have some code which looks like the following:
FIRRTL version 4.0.0
circuit Foo:
public module Foo:
input a: UInt<31>
input b: UInt<1>
output c: UInt<31>
connect c, shr(cat(a, b), 1)
This is initially parsed into:
module {
firrtl.circuit "Foo" {
firrtl.module @Foo(in %a: !firrtl.uint<31>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<31>) attributes {convention = #firrtl<convention scalarized>} {
%0 = firrtl.cat %a, %b : (!firrtl.uint<31>, !firrtl.uint<1>) -> !firrtl.uint<32>
%1 = firrtl.shr %0, 1 : (!firrtl.uint<32>) -> !firrtl.uint<31>
firrtl.strictconnect %c, %1 : !firrtl.uint<31>
}
}
}
After FIRRTL canonicalization, this converts the shr to bits:
module {
firrtl.circuit "Foo" {
firrtl.module @Foo(in %a: !firrtl.uint<31>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<31>) attributes {convention = #firrtl<convention scalarized>} {
%0 = firrtl.cat %a, %b : (!firrtl.uint<31>, !firrtl.uint<1>) -> !firrtl.uint<32>
%1 = firrtl.bits %0 31 to 1 : (!firrtl.uint<32>) -> !firrtl.uint<31>
firrtl.strictconnect %c, %1 : !firrtl.uint<31>
}
}
}
FIRRTL Dialect doesn’t seem to have any further canonicalization of cat(bits(...))). However, HW Dialect does. When this is converted to HW dialect this becomes:
module {
hw.module @Foo(in %a : i31, in %b : i1, out c : i31) {
%0 = comb.concat %a, %b : i31, i1
%1 = comb.extract %0 from 1 : (i32) -> i31
hw.output %1 : i31
}
}
When canonicalized in HW dialect, this becomes:
module {
hw.module @Foo(in %a : i31, in %b : i1, out c : i31) {
hw.output %a : i31
}
}
This is reading between the lines and trying to interpret a lot from your mention of shr. Is this where the confusion is coming from?
Thank you for your reply. I know that later in lowering to hw dialect it removes pointless ops. That’s not the problem. The problem that it produces this pointless chain of ops. What’s more, in the whole example this particular bit remains unconnected. If it was like this %69 = firrtl.bits %_GEN__317 30 to 0 : (!firrtl.uint<32>) -> !firrtl.uint<31> everything would be connected.
Thank you for your reply.
No. firrtl.shr isn’t present in this chain of ops. I mentioned it just to reference where I got bit enumeration. Please forget about it. The structure that I got is exactly like I mentioned before. The problem is that as a result it produces clearly pointless chain of ops. What’s more, when analyzing bit-to-bit connections, this results in this particular bit remains unconnected. I suspect that it should be like this: %69 = firrtl.bits %_GEN__317 30 to 0 : (!firrtl.uint<32>) -> !firrtl.uint<31> instead of %69 = firrtl.bits %_GEN__317 31 to 1 : (!firrtl.uint<32>) -> !firrtl.uint<31>. I will include the source .fir file (as .txt file)
picorv_processor1.txt (514.6 KB)
NOTE: CIRCT hash commit I’m on: 2d822ea
Here is some snippet of that fir file with the relevant pieces:
wire _GEN__4214 : UInt
wire _GEN__3609 : UInt<1>
wire _GEN__3567 : UInt
wire _GEN__317 : UInt<32>
_GEN__4214 <= cat(bits(_GEN__3567, 30, 0), bits(_GEN__3609, 0, 0))
_GEN__3609 <= UInt<1>(0b0)
_GEN__3567 <= mux(bits(latched_stalu, 0, 0), bits(alu_out_q, 31, 0), bits(reg_out, 31, 0))
_GEN__3565 <= cat(bits(_GEN__317, 31, 1), UInt<1>(0b0))
_GEN__3143 <= cat(bits(_GEN__317, 31, 1), bits(_GEN__826, 0, 0))
_GEN__317 <= _GEN__4214
Looking at that input .fir file I notice two things:
- The bottom bit of
_GEN__317is always0 - The bottom bit of
_GEN__317is never used in subsequent calculations
The FIRRTL file seem to have identical logic to the the IR you sent earlier. If there is a bug here, it seems like it would be in the tool that produced this .fir file, or it originates from the picorv32 project itself. Are you building GitHub - YosysHQ/picorv32: PicoRV32 - A Size-Optimized RISC-V CPU? Are you using the yosys FIRRTL backend to translate from System Verilog to FIRRTL?
- Yes.
- No, it’s ‘homemade’.