[RISCV][Lifting] Implement RzIL lifting for the rest of the M extension#6364
[RISCV][Lifting] Implement RzIL lifting for the rest of the M extension#6364moste00 wants to merge 4 commits into
Conversation
notxvilka
left a comment
There was a problem hiding this comment.
Since there is no tracetesting yet, please start from creating "emulateme" test and implement instructions first that in that particular test binary(ies). Then we can make sure those are more or less correct.
Do you mean there are no tracetesting at all or that there is no tracetesting for the M-extension ? Because there is tracetesting (BinaryAnalysisPlatform/qemu#44, rizinorg/rz-tracetest#29), I ran a very minimal local binary for both 32-bit and 64-bit already, and I made sure that all non-floating-point instructions in it run correctly, this is how a lot of instructions in #6287 are verified indeed. I didn't run any binary yet for instructions added uniquely in this PR, but I will soon. (If you meant that I should commit all the binaries that I trace to the binaries repo, I will, but I can't make them target a specific set of extensions without a lot of headache, so I can't make a binary specifically for the M extension, it will contain a lot from other MAFD extensions. I'm seperating the changes by extension for ease of review, but when I run binaries I will always run against IMAFD) |
|
@notxvilka Did you actually mean I shouldn't structure the lifting as extension-by-extension PRs at all, instead proceeding binary-by-binary on real-world binaries ? Good idea but I naturally think in terms of extensions, I can switch gears if that's what you meant. |
No, if you already started with the tracetesting, it's fine to keep going as is. It's just priority should be towards more popular instructions compared to less popular, so that it covers all use cases in real binaries. |
3c7c292 to
cf4a5d4
Compare
cf4a5d4 to
3267190
Compare
…riants of the instructions in it for 32-bit arch variant only, and add some tests
3267190 to
47ff84a
Compare
Your checklist for this pull request
RZ_APIfunction and struct this PR changes.RZ_API).Detailed description
Continues #6287 by adding more multiplication instructions.
Test plan
...
Closing issues
PR Stack Structure
This PR is the second of a 4-PR stack that lifts the I-M-A-FD RISC-V archiecture plus some privileged instructions.
The following is a table for ease of navigation from any PR to any other, please filter by the commit(s) mentioned in the table when reviewing the diff in the "Files Changed" tab to avoid the noise of the base branches.
Because it's painful to maintain this stack manually with no Github or other tooling support, I won't grow it any further until one of those 4 PRs gets merged.