Fix/tc sram - Githubissues

pulp-platform / tech_cells_generic

Technology dependent cells instantiated in the design for generic process (simulation, FPGA)

Other

28 stars 29 forks source link

Fix/tc sram #20

Closed JeanRochCoulon closed 2 years ago

JeanRochCoulon commented 2 years ago

Hello @micprog, I would be pleased to use the project to model the cva6 memories, as seen with @zarubaf. The cva6 regression was failing, I have made some fixes to make it PASS:

2 fixes to suit Verilator (Verilator errors are in the commit message)
1 fix detected by VCS
1 improvement to speed-up init time Tell me !

JeanRochCoulon commented 2 years ago

Thanks a lot @niwis for the feedback, I modified (with push --force) the PR. Note: The timing optimization has not been pushed again, maybe I will submit another PR with it.

JeanRochCoulon commented 2 years ago

Hello @niwis, I have updated the model. This tc_sram model has been used and verified in a coming CVA6 PR, to submit this CVA6 PR I need that this tc_sram PR merged. Thanks for your review !!

meggiman commented 2 years ago

hi @JeanRochCoulon , I just pushed and alternative PR (#23) which should fix your issue yet doesn't introduce lint warnings due to the usage of blocking assignments in sequential blocks. I would be glad if you could give it a try and provide feedback if this would solve your issue. Preferably we do not want to deviate from the default rule of thumb (only non-blocking assignment in always_ff blocks) since a lot of Lint tools (including verilator) would mark flag this.

niwis commented 2 years ago

Superseded by #23