calyx-py: Mark `std_bit_slice` as combinational

[abnerhexu]

Hey repo contributors,

I found that std_slice is combinational but std_bit_slice is not. However, from the verilog template, they should be the same.

I wonder why you have such design.

module std_bit_slice #(
    parameter IN_WIDTH = 32,
    parameter START_IDX = 0,
    parameter END_IDX = 31,
    parameter OUT_WIDTH = 32
)(
   input wire logic [IN_WIDTH-1:0] in,
   output logic [OUT_WIDTH-1:0] out
);
    assign out = in[END_IDX:START_IDX];
...
endmodule

and

module std_slice #(
    parameter IN_WIDTH  = 32,
    parameter OUT_WIDTH = 32
) (
   input wire logic [IN_WIDTH-1:0] in,
   output logic [OUT_WIDTH-1:0] out
);
  assign out = in[OUT_WIDTH-1:0];
...
endmodule

[rachitnigam]

Both implementations are combinational. What makes you think that std_bit_slice is not combinational?

[abnerhexu]

in calyx/calyx-py/calyx /builder.py, line 1440,

def is_comb(self) -> bool:
        return self._cell.comp.id in (
            "std_add",
            "std_sub",
            "std_lt",
            "std_le",
            "std_ge",
            "std_gt",
            "std_eq",
            "std_neq",
            "std_sgt",
            "std_slt",
            "std_fp_sgt",
            "std_fp_slt",
        )

Maybe it should be added with more classes?

[rachitnigam]

Yup, looks like an oversight! Do you want to open a pull request to fix this?

[abnerhexu]

Hey, there is one more possible bug, in primitives/core.sv, line 209

module std_bit_slice #(
    parameter IN_WIDTH = 32,
    parameter START_IDX = 0,
    parameter END_IDX = 31,
    parameter OUT_WIDTH = 32
)(
   input wire logic [IN_WIDTH-1:0] in,
   output logic [OUT_WIDTH-1:0] out
);
    assign out = in[END_IDX:START_IDX];
    ...
endmodule

In order to be consistent with the docs itself (https://docs.calyxir.org/libraries/core.html),

std_bit_slice<IN_WIDTH, START_IDX, END_IDX, OUT_WIDTH> Extract the bit-string starting at START_IDX and ending at END_IDX - 1 from in. This is computed as in[END_IDX:START_IDX].OUT_WIDTH must be specified to be END_WIDTH - START_WITH wide when instantiating the module.

it should be

assign out = in[END_IDX:START_IDX];

For example, when create a std_bit_slice by slicer = std_bit_slice(4, 2, 4, 2);, it cannot be synthesized because END_IDX is out of range in in signal.

To fixed it, the document should be changed like

std_bit_slice<IN_WIDTH, START_IDX, END_IDX, OUT_WIDTH> Extract the bit-string starting at START_IDX and ending at END_IDX - 1 from in. This is computed as in[END_IDX-1:START_IDX].OUT_WIDTH must be specified to be END_WIDTH - START_WITH wide when instantiating the module.

and the core.sv should changed like

assign out = in[END_IDX-1:START_IDX];

I can help fix these two issues.

[rachitnigam]

Sounds good, thanks!

[rachitnigam]

Closed by 8d242da68ad5cfb266cd622911beafe341921d3a