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

ABI cleanup #1024

Merged
merged 3 commits into from Sep 8, 2014
Merged

ABI cleanup #1024

merged 3 commits into from Sep 8, 2014

Conversation

comex
Copy link
Contributor

@comex comex commented Sep 7, 2014

Improves some code and removes a bunch of unnecessary code.

Someone should test on Windows.

@comex comex force-pushed the abi-cleanup branch 2 times, most recently from a3fcaee to fd7a71b Compare September 7, 2014 20:39
@skidau
Copy link
Contributor

skidau commented Sep 8, 2014

Tested this in Windows with DSP LLE JIT enabled

Animal Crossing (GC/non-MMU) - WORKING
NFS: HP2 (GC/non-MMU) - CRASH
MGS: TS (GC/mmu speedhack) - WORKING
F-ZERO GX (GC/mmu speedhack) - CRASH
GUN (GC/mmu DSI) - WORKING
Rogue Leader (GC/mmu ISI) - CRASH
Ultimate Spider-man (GC/mmu ISI) - WORKING
DKCR (Wii/AX ucode) - WORKING
NSMBW (Wii/AX ucode) - WORKING
SMG (Wii/zelda ucode) - CRASH
ZTP (Wii/zelda ucode) - WORKING

Code looks good by a cursory glance. Will in-depth review after bugs fixed.

@comex
Copy link
Contributor Author

comex commented Sep 8, 2014

This is the one PR I thought wouldn't have any bugs.

@skidau
Copy link
Contributor

skidau commented Sep 8, 2014

}
int size = ((noProlog ? -regSize : 0) - (count * regSize)) & 0xf;
rsp_alignment -= count * 8;

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@comex
Copy link
Contributor Author

comex commented Sep 8, 2014

Ah, there was one missing conversion. fixed.

@phire
Copy link
Member

phire commented Sep 8, 2014

Code looks good to me.

Does @skidau want to make sure those broken games are working now?

…Stack.

- Factor common work into a helper function.
- Replace confusingly named "noProlog" with "rsp_alignment".  Now that
x86 is not supported, we can just specify it explicitly as 8 for
clarity.
- Add the option to include more frame size, which I'll need later.
- Revert a change by magumagu in March which replaced MOVAPD with MOVUPD
on account of 32-bit Windows, since it's no longer supported.  True,
apparently recent processors don't execute the former any faster if the
pointer is, in fact, aligned, but there's no point using MOVUPD for
something that's guaranteed to be aligned...

(I discovered that GenFrsqrte and GenFres were incorrectly passing false
to noProlog - they were, in fact, functions without prologs, the
original meaning of the parameter - which caused the previous change to
break.  This is now fixed.)
…k (it didn't preserve FPRs!) and replace with ABI_PushRegistersAndAdjustStack.

To avoid FPRs being pushed unnecessarily, I checked the uses: DSPEmitter
doesn't use FPRs, and VertexLoader doesn't use anything but RAX, so I
specified the register list accordingly.  The regular JIT, however, does
use FPRs, and as far as I can tell, it was incorrect not to save them in
the outer routine.  Since the dispatcher loop is only exited when
pausing or stopping, this should have no noticeable performance impact.
…_CallFunctionRR.

The latter being true was the only case where the former would do
anything, and it was never true.  They became obsolete with x86's
removal.
comex added a commit that referenced this pull request Sep 8, 2014
@comex comex merged commit 7fb6628 into dolphin-emu:master Sep 8, 2014
@comex comex deleted the abi-cleanup branch September 23, 2014 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants