search

alexforencich / verilog-axi

Verilog AXI components for FPGA implementation
MIT License
1.52k stars 454 forks source link

Fix AXI_ADDR_BIT_OFFSET and AXIL_ADDR_BIT_OFFSET part select #49

Open AlexLao512 opened 1 year ago

AlexLao512 commented 1 year ago

I think I fixed this correctly, the tests in your repo seem to pass.

AlexLao512 commented 1 year ago

I misunderstood some stuff. Let me work on this a bit. I think there are some reasonable workarounds that are a bit annoying but seeing https://github.com/alexforencich/verilog-axi/issues/28 I think they are worth doing.

I think we can get away with slightly more confusing code but avoid duplicating a lot of code with generate blocks.

For code paths we know are going to compile out, we use a different parameter or ternary operators to force the values to something the simulator is happy with.

I'll restrict this MR to axi_axil_adapter since that is what I have my testbenches for at the moment.

AlexLao512 commented 1 year ago

I have something working, I forced pushed over the commit. Sorry for the confusion earlier.

@alexforencich. Do you mind checking? I don't know if I am running your cocotb stuff properly. Quick sanity check in my much smaller testbenches shows they compile fine and functionality looks OK but my testbenches are not very complete since I just use them for testing SystemVerilog wrappers around many of your modules.

alexforencich commented 1 year ago

Terribly ugly macros. I liked one of the other suggestions better that added an intermediate var, but IMO the proper solution is to fix the simulator so it doesn't throw an error if the code is unreachable by parameter value.

AlexLao512 commented 1 year ago

I'll see if I can bypass the error or create intermediate parameters.

AlexLao512 commented 1 year ago

@alexforencich I think I've found a more reasonable workaround. ModelSim seems happy with [A -: B] relative part selects even when they end up backwards. If you are ok with this change, I can make the same changes to the other adapters with similar issues.

PedroAntunes178 commented 1 year ago

Hi, related to this issue, xcelium gives the following error:

xmelab: *E,RNGDIR (../src/axi_axil_adapter_rd.v,286|61): Reversed part-select index expression ordering.
                  s_axi_rdata_next = m_axil_rdata >> (addr_reg[AXIL_ADDR_BIT_OFFSET-1:AXI_ADDR_BIT_OFFSET] * AXI_DATA_WIDTH);

And Verilator gives a similar error.