-
Notifications
You must be signed in to change notification settings - Fork 45
feat: integrate Rabbitizer via FetchContent #30
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
base: main
Are you sure you want to change the base?
Conversation
|
Nice work @Lucasnribeiro |
4d2caa3 to
a35f585
Compare
|
at first the idea was to just introduce rabbitizer, but i realized that we have the opportunity to just drop R5900_decoder entirely in favor of using Rabbitizer. Now the pr is a little bigger but more complete |
| std::cerr << "Error decoding instruction at " << formatAddress(addr) | ||
| << ": " << e.what() << std::endl; | ||
| } | ||
| RabbitizerInstruction instr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use decodeInstruction function here ?
| } | ||
|
|
||
| Instruction inst = m_decoder->decodeInstruction(address, rawInstruction); | ||
| Instruction inst = decodeInstruction(address, rawInstruction); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
create a namespace for decoder so de can call decoder::decodeInstruction or some other namespace, insted of just calling decodeInstruction
| @@ -0,0 +1,651 @@ | |||
| #include "ps2recomp/decoder.h" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
about this file: r5900_decoder.cpp functions lilke decodeIType, decodeRegimm and other handle way more that what we have here. why remove the other cases ?
implementing this issue #17