Skip to content
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

Additional cleanup and improvements #3

Merged
merged 3 commits into from Dec 18, 2022
Merged

Conversation

mumbel
Copy link
Contributor

@mumbel mumbel commented Nov 27, 2022

  • Added comments to each instruction
  • Cleaned up coprocessor instructions
  • Included instructions found in MB91301
  • usignedi_4 and signedi_4 can be handled with "signed" tokens
  • shifted bits instead of multiply
  • Cleaned up +16 bit instructions
  • Carry flag was calculated from register with operation applied twice
  • show store from pcodeop stres

Bumps ldef version due to instruction decoding changes

* Added comments to each instruction
* Cleaned up coprocessor instructions
* Included instructions found in MB91301
* usignedi_4 and signedi_4 can be handled with "signed" tokens
* shifted bits instead of multiply
* Cleaned up +16 bit instructions
* Carry flag was calculated from register with operation applied twice
* show store from pcodeop stres

Bumps ldef version due to instruction decoding changes
@mumbel
Copy link
Contributor Author

mumbel commented Nov 27, 2022

@DiscoStarslayer I can try to break this down if its too much touching in 1 commit, but did a better comparison of my first implementation and this one and noticed a few things, and just gave it a deeper read through in general.

The C flag changes are the only things I'd consider a bug though, everything else you could consider preference really (wouldn't affect anything in disassembly really, and nothing significant in decompilation)

@DiscoStarslayer
Copy link
Contributor

@mumbel Thank you, these changes look great.

All in one PR is no problem. It looks like there is a bit of a regression in the decomp output. Not from a functionality perspective (they still pass the ghidra test suites) but decomp output perspective.

See this commit contributed by a user:
c1c1db4

It appears ghidra has some strange heuristics when using XOR's. For instance in decomp you will get output like this for BLT:
CC: "LT" is cc=0xA { local tmp:1 = ((V ^ N) == 1); export *[const]:1 tmp; }
if ((SBORROW4(a,b) ^ (a - b) < 0) == 1) {
converting logic with XOR into equivalent logic avoiding XOR cleans up the decomp quite a bit:
CC: "LT" is cc=0xA { local tmp:1 = (V != N); export *[const]:1 tmp; }
if (a < b) {

I will merge these changes as they are correct, however I will take a deeper look into what is causing the decomp to fall though. I'd like to keep the XOR based logic in the sleigh code is it's written now as it more closely follows the datasheet, but practicality may win over

@DiscoStarslayer DiscoStarslayer merged commit a8ecd56 into desrdev:main Dec 18, 2022
@mumbel
Copy link
Contributor Author

mumbel commented Dec 18, 2022

oh. yeah that's a significant readability regression. I mostly just followed the manual, but the math can definitely be handled differently. V != N is an obvious equation now that its pointed out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants