-
Notifications
You must be signed in to change notification settings - Fork 12
Implement 64-bit instructions #73
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
Conversation
AngheloAlf
left a comment
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.
I think every comment I left can be answered with "the codebase is already doing this weird thing".
Either way, the PR looks good!
| case rabbitizer::InstrId::UniqueId::cpu_lbu: | ||
| case rabbitizer::InstrId::UniqueId::cpu_ld: | ||
| case rabbitizer::InstrId::UniqueId::cpu_ldl: | ||
| // case rabbitizer::InstrId::UniqueId::cpu_ldr: |
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 is this commented out?
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.
Same treatment as lwr, gotcha
| case rabbitizer::InstrId::UniqueId::cpu_sb: | ||
| case rabbitizer::InstrId::UniqueId::cpu_sd: | ||
| case rabbitizer::InstrId::UniqueId::cpu_sdl: | ||
| // case rabbitizer::InstrId::UniqueId::cpu_sdr: |
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.
same
| case rabbitizer::InstrId::UniqueId::cpu_dsub: | ||
| case rabbitizer::InstrId::UniqueId::cpu_dsubu: | ||
| if (insn.instruction.GetO32_rs() == rabbitizer::Registers::Cpu::GprO32::GPR_O32_zero) { | ||
| printf("%s = -%s;\n", r((int)insn.instruction.GetO32_rd()), r((int)insn.instruction.GetO32_rt())); |
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.
Manually implementing negu, I see
| printf("%s = %s + %d; ", reg, r((int)insn.instruction.GetO32_rs()), imm); | ||
| printf("%s = ((uint64_t)MEM_U8(%s) << 56) | ((uint64_t)MEM_U8(%s + 1) << 48) | " | ||
| "((uint64_t)MEM_U8(%s + 2) << 40) | ((uint64_t)MEM_U8(%s + 3) << 32) | " | ||
| "((uint64_t)MEM_U8(%s + 4) << 24) | ((uint64_t)MEM_U8(%s + 5) << 16) | " | ||
| "((uint64_t)MEM_U8(%s + 6) << 8) | (uint64_t)MEM_U8(%s + 7);\n", | ||
| reg, reg, reg, reg, reg, reg, reg, reg, reg); |
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.
This is an interesting implementation.
So just assume every ldl will have a corresponding ldr?
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.
... Oh, that's what recomp is already doing for lwl/lwr... huh. This should be fine I guess
| for (int j = 0; j < 8; j++) { | ||
| printf("MEM_U8(%s + %d + %d) = (uint8_t)(%s >> %d); ", r((int)insn.instruction.GetO32_rs()), imm, j, | ||
| r((int)insn.instruction.GetO32_rt()), (7 - j) * 8); | ||
| } |
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.
This codebase definitely has some interesting pieces of code. Inconsistent, but code
This reverts commit d26c890.
Note that there can't be
MEM_S64/MEM_U64macros because the two halves of the 64-bit value would be in the wrong order.(also fixed
swlto be one line because that's what// fdeadand// bdeadexpect)