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

JitArm64: Implement HLE function hooking #9218

Merged
merged 1 commit into from
Nov 4, 2020

Conversation

JosJuice
Copy link
Member

@JosJuice JosJuice commented Nov 2, 2020

Closely based on the x86-64 implementation. Fixes https://bugs.dolphin-emu.org/issues/10965.

@JosJuice JosJuice requested a review from degasus November 2, 2020 21:29
Copy link
Member

@degasus degasus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the FlushCarry, it looks good to me.

@@ -209,20 +209,15 @@ void JitArm64::FallBackToInterpreter(UGeckoInstruction inst)
}
}

void JitArm64::HLEFunction(UGeckoInstruction inst)
void JitArm64::HLEFunction(u32 hook_index)
{
gpr.Flush(FlushMode::FLUSH_ALL);
fpr.Flush(FlushMode::FLUSH_ALL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a chance that the first instruction is an integer instruction? If so, "FlushCarry();" might need to be called as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FlushCarry added.

return false;

LDR(INDEX_UNSIGNED, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(npc));
js.downcountAmount += js.st.numCycles;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I do not get the point why we add the cycles of this single instruction. I assume te HLE functions shall decrease downCount on their own, so this single instruction likely has no effect. However it is the same in the x64 JIT, so I'm fine with it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

js.st.numCycles is for the whole block, not the current instruction. Though, implementing it that way also seems weird, considering that we might hit the HLE case halfway through a block after we've already added some cycles to js.downcountAmount the normal way...

@degasus degasus merged commit 069840f into dolphin-emu:master Nov 4, 2020
@JosJuice JosJuice deleted the aarch64-hle-hooks branch November 4, 2020 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants