Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix AXI_ADDR_BIT_OFFSET and AXIL_ADDR_BIT_OFFSET part select #49

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AlexLao512
Copy link

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

@AlexLao512
Copy link
Author

AlexLao512 commented Mar 6, 2023

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 #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 AlexLao512 force-pushed the fix_axi_axil_adapter_rd_and_wr_part_select branch from a3b4a34 to f7eeb4e Compare March 6, 2023 07:37
@AlexLao512 AlexLao512 force-pushed the fix_axi_axil_adapter_rd_and_wr_part_select branch from f7eeb4e to c4e502d Compare March 6, 2023 08:05
@AlexLao512
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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

@AlexLao512
Copy link
Author

AlexLao512 commented Mar 10, 2023

@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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants