ThinkOpenly / sail-riscv

Sail RISC-V model
Other
11 stars 14 forks source link

Make extension discriminator clear #8

Closed Bakugo90 closed 6 months ago

Bakugo90 commented 7 months ago

Implement #4 For this pull request i work on "A" and "M" extension. I've cloned your JSON branch and worked from that so I can continue what you've already done.

I have already implemented all of the necessary changes. Look at the "JSON" branch in this repository. Further work here should probably be on top of that branch.

Of course, I've noticed that you've already worked on the "V" and "Zbkb" extension. And that really helped me to recognise the "M" and "A" extensions.

ThinkOpenly commented 7 months ago

Greetings, and thanks for this submission!

While the changes you have implemented are not incorrect, they do not address the issue described in #4.

If you look at most of the changes in the existing code where the extension function is used, you'll see it mostly is appended to (one or both sides of) mapping clause encdec definitions. Issue #4 is asking that the same type of conditional expressions used for some extensions be used for all extensions. For example, the current code has:

mapping clause encdec = LOADRES(aq, rl, rs1, size, rd) if amo_width_valid(size)

and it would need to be something like:

mapping clause encdec = LOADRES(aq, rl, rs1, size, rd) if extension("A") & amo_width_valid(size)
Bakugo90 commented 6 months ago

Done ! @ThinkOpenly

ThinkOpenly commented 1 month ago

FYI, this commit and other related "extension tagging" commits have been merged upstream. https://github.com/riscv/sail-riscv/commit/ebf57cc79459bdba3e8d15e84941b6dcd088f5f4. Congratulations, and thank you for your efforts! :tada: