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

Add remaining possible uses of MOVBE #303

Merged
merged 2 commits into from Apr 24, 2014
Merged

Conversation

Tilka
Copy link
Member

@Tilka Tilka commented Apr 23, 2014

Ceterum autem censeo x86-32 esse delendam.

@delroth
Copy link
Member

delroth commented Apr 23, 2014

LGTM if user testing does not find any weird bug. @JMC47

@Sonicadvance1
Copy link
Contributor

I instantly get a Backpatch error with this.

@Tilka
Copy link
Member Author

Tilka commented Apr 23, 2014

Fixed 32 bit, haven't looked into the Backpatch error yet.

@Tilka
Copy link
Member Author

Tilka commented Apr 23, 2014

Fixed.

@Sonicadvance1
Copy link
Contributor

Works for me and LGTM.

@delroth
Copy link
Member

delroth commented Apr 24, 2014

Will you be available in the next few days to fix crashes on a short notice?

If not, would appreciate some basic user testing from both people who have Haswell and people who have not. Leaving a forum thread for a day should probably be enough.

@Sonicadvance1
Copy link
Contributor

I have tested a few games for a few minutes each and couldn't find a noticeable issue on my end for now.
Haswell CPU of course.

@comex
Copy link
Contributor

comex commented Apr 24, 2014

So... backpatching assumes there are at least 5 bytes to replace with a call instruction. Usually the BSWAP was enough for this, but for 8-bit loads and stores two NOP bytes are added. However, some MOVBE instructions seem to be 4 bytes:

 0: 0f 38 f0 3f             movbe  (%rdi),%edi

Is this taken care of somewhere?

@Tilka
Copy link
Member Author

Tilka commented Apr 24, 2014

@comex: No, but all our backpatched MOVBEs use memory references. Good catch though.

EDIT: OK, what I just said doesn't make any sense. I'll have a look.

@Tilka
Copy link
Member Author

Tilka commented Apr 24, 2014

Should still be fine, we always have a SIB byte or an offset.

Also fixes a missing 'break' statement in DisassembleMov().
@Tilka
Copy link
Member Author

Tilka commented Apr 24, 2014

Squashed.

delroth added a commit that referenced this pull request Apr 24, 2014
Add remaining possible uses of MOVBE
@delroth delroth merged commit 47373af into dolphin-emu:master Apr 24, 2014
@delroth
Copy link
Member

delroth commented Apr 24, 2014

Please try and monitor forums for issues that could be caused by this :)

@Tilka Tilka deleted the movbe branch April 24, 2014 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants