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

[rtl] Trim rf_reg addresses for RV32E #1829

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

Conversation

ANurmi
Copy link

@ANurmi ANurmi commented Oct 3, 2022

Use trimmed address values to match the 16 registers in RV32E configuration. This fixes top level IbexDataReqPayloadX assert that was being triggered during LW instruction execution with IMM value 'h10 in rs2 register.

Fix for array index out of range with rv32e configuration.
Copy link
Contributor

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ANurmi for your PR!

This commit obviously fixes an issue when using RV32E but I think it would actually be better to just parameterize the regfile address widths. However this will be much more invasive. I am not sure what's better. I need to discuss this with the team.

assign rdata_a_o = rf_reg[raddr_a_i];
assign rdata_b_o = rf_reg[raddr_b_i];

if(RV32E) begin : rv32e_regfile
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the missing whitespace between if and (RV32E here causes lint to fail. Can you please run

fusesoc --cores-root . run --target=lint --tool=veriblelint lowrisc:ibex:ibex_top_tracing  --RV32E=0 --RV32M=ibex_pkg::RV32MFast --RV32B=ibex_pkg::RV32BNone --RegFile=ibex_pkg::RegFileFF --BranchTargetALU=0 --WritebackStage=0 --ICache=0 --ICacheECC=0 --ICacheScramble=0 --BranchPredictor=0 --DbgTriggerEn=0 --SecureIbex=0 --PMPEnable=0 --PMPGranularity=0 --PMPNumRegions=4 --MHPMCounterNum=0 --MHPMCounterWidth=40

to check?

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.

None yet

2 participants