SystemRDL / PeakRDL-regblock

Generate SystemVerilog RTL that implements a register block from compiled SystemRDL input.
http://peakrdl-regblock.readthedocs.io
GNU General Public License v3.0
52 stars 42 forks source link

[BUG] Incorred bit extraction on external component address assignment #116

Open apstrike opened 4 months ago

apstrike commented 4 months ago

Hello,

I think there's an improper bit select within the external memory template when it comes to setting the external bus's address.

For the following RDL file:

addrmap test {
    default regwidth = 32;
    default accesswidth = 32;

    external mem {
        mementries = 256;
        memwidth = 32;
        sw=rw;

    } my_mem;

};

The generated code gives the external memory interface struct as:

    typedef struct {
        logic req;
        logic [9:0] addr;
        logic req_is_wr;
        logic [31:0] wr_data;
        logic [31:0] wr_biten;
    } test__my_mem__external__out_t;

in where the addr field has the correct address width.

However in the module file we see:

logic [9:0] decoded_addr;

...

assign hwif_out.my_mem.addr = decoded_addr[10:0];

The bit select on decoded_addr is too wide and should be [9:0].

The root of the issue seems to lie in src/peakrdl_regblock/field_logic/generators.py's FieldLogicGenerator::assign_external_block_outputs:

prefix = "hwif_out." + get_indexed_path(self.exp.ds.top_node, node)
strb = self.exp.dereferencer.get_external_block_access_strobe(node)
addr_width = node.size.bit_length()

in which I think addr_width = (node.size-1).bit_length() is correct.

I see that under utils.py you have a clog2 function and I think it would be most appropriate to use that where possible.

Additionally, I'd think it would be appropriate to use clog2 in peakrdl_regblock/hwif/generators.py, under OutputStructGenerator_Hier::_add_external_block_members: self.add_member("addr", (node.size - 1).bit_length()) should be (I think) self.add_member("addr", clog2(node.size)).

Thanks, Aylon