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

JIT: support immediate stores #805

Merged
merged 1 commit into from Sep 16, 2014
Merged

Conversation

FioraAeterna
Copy link
Contributor

This is a bit of a WIP given that it.... turned out to be a lot bigger than I expected it to be, and I still need to bench it, but I'd love comments.

Basically I noticed there were tons of examples of code like this:

mov eax, 0x8
bswap eax, eax
mov [rdx+rbx], eax

This was clearly quite silly, so I decided to add support for immediate stores, a'la:

mov [rdx+rbx], 0x08000000

How hard could this be? Well, after a journey into the backpatcher, I can say "much harder than I originally thought..."

@@ -195,7 +215,13 @@ const u8 *Jitx86Base::BackPatch(u8 *codePtr, u32 emAddress, void *ctx_void)
}

u32 registersInUse = it->second;

/*if (info.hasImmediate)

This comment was marked as off-topic.

@phire
Copy link
Member

phire commented Aug 16, 2014

Would be nice to fix SafeWriteRegToReg so it doesn't clobber both its argument registers. Looks like it only clobbers source if it needs to do a BSWAP instruction and it only needs to clobber addr if it needs to call the external write function.
Currently it always clobbers addr on the slow path, doing an extra add instruction.

But probably out of scope of this PR.

LGTM.

@FioraAeterna FioraAeterna force-pushed the storerefactor branch 3 times, most recently from c428ae4 to 768c957 Compare August 23, 2014 07:26
@FioraAeterna FioraAeterna changed the title JIT: support immediate stores WIP: JIT: support immediate stores Aug 23, 2014
@FioraAeterna FioraAeterna changed the title WIP: JIT: support immediate stores JIT: support immediate stores Aug 28, 2014
@FioraAeterna FioraAeterna changed the title JIT: support immediate stores WIP: JIT: support immediate stores Aug 28, 2014
@FioraAeterna
Copy link
Contributor Author

I've rewritten this to work with all of comex's latest changes, but I'm not sure what I should do about the register issue. If the fast path fails, we need to allocate a register for the immediate reg_value for the slow path, but it's not clear which register we should pick (RSCRATCH or RSCRATCH2 might already be used by addr, and might be requested as callee-save by the caller?).

This should work with MMU games now.

@comex
Copy link
Contributor

comex commented Sep 11, 2014

You've already saved registers that are in use (other than RSCRATCHes, but those were never going to be saved across a function call anyway - SAFE_LOADSTORE_CLOBBER_RSCRATCH_INSTEAD_OF_ADDR is badly named - it's more about whether to assume RSCRATCH is free; frankly, I'm an insane programmer). Accordingly, you can use any register that's not going to conflict with something else, but keep in in that the result is going to be wasteful code as ABI_CallFunctionRR copies your register into ABI_PARAM2 - not that it matters for a slow case.

@FioraAeterna
Copy link
Contributor Author

Comex, is FioraAeterna@df2fafe#diff-9aa48ec8e209a074e590018a52b78eb6R70 bad? should I not be doing that?

@comex
Copy link
Contributor

comex commented Sep 11, 2014

I think it's fine...

@FioraAeterna
Copy link
Contributor Author

I thought I should be avoiding using RSCRATCH across function calls though?

@comex
Copy link
Contributor

comex commented Sep 11, 2014

It'll work if you ask for it to be saved.

@FioraAeterna
Copy link
Contributor Author

Okay, makes sense!

@FioraAeterna FioraAeterna changed the title WIP: JIT: support immediate stores JIT: support immediate stores Sep 13, 2014
@FioraAeterna FioraAeterna force-pushed the storerefactor branch 2 times, most recently from 56cca4a to 929fc8a Compare September 14, 2014 00:26
skidau added a commit that referenced this pull request Sep 16, 2014
@skidau skidau merged commit 8361d2b into dolphin-emu:master Sep 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants