uwsampl / churchroad

MIT License
4 stars 1 forks source link

Yosys plugin generating incorrect `ZeroExtend`s #43

Closed ninehusky closed 1 week ago

ninehusky commented 1 week ago

When using the Churchroad Yosys plugin, I'm seeing that there are multiple ZeroExtends that aren't correct. Consider the following module:

module my_module(
    input a,
    input b,
    output[1:0] o,
);
    assign o = {a, b};
endmodule

Running yosys -m $CHURCHROAD_DIR/yosys-plugin/churchroad.so -q -p "read_verilog -sv my-module.v; prep -top my_module; pmuxtree; write_lakeroad" yields:

; wire declarations
; a
(let v0 (Wire "v0" 1))
; b
(let v1 (Wire "v1" 1))
; o
(let v2 (Wire "v2" 2))

; cells
; TODO not handling signedness
(let v3 (Op1 (ZeroExtend 2) v1))
; TODO not handling signedness
(let v4 (Op1 (ZeroExtend 2) v0))
(union v2 (Op2 (Concat) v3 v4))

; inputs
(let a (Var "a" 1))
(IsPort "" "a" (Input) a)
(union v0 a)
(let b (Var "b" 1))
(IsPort "" "b" (Input) b)
(union v1 b)

; outputs
(let o v2)
(IsPort "" "o" (Output) o)

; delete wire expressions
(delete (Wire "v0" 1))
(delete (Wire "v1" 1))
(delete (Wire "v2" 2))

v2, which is the output of the module, is being assigned the concatenation of v3 and v4, each of which are respectively a and b zero-extended to be of bitwidth 2. This means that we're unioning two things of different bitwidth!

ninehusky commented 1 week ago

I'll try to dig into this a little bit tonight!

gussmith23 commented 1 week ago

The bug is here: https://github.com/uwsampl/churchroad/blob/cb9195421c4c58070a4b964cd5636b8198250750/yosys-plugin/churchroad.cc#L1558-L1559

Basically, we're extending the inputs before the concat, which is useful e.g. when implementing things like Add and And (i.e. ops where the inputs and output are all the same bitwidth), but Concat does not have the same behavior. It's output bitwidth is the sum of its input bitwidths. thus, the inputs don't need to be extended to match the output bitwidth.