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

[DYNAREC] Fix x87cache issues #1025

Merged
merged 6 commits into from
Oct 14, 2023
Merged

[DYNAREC] Fix x87cache issues #1025

merged 6 commits into from
Oct 14, 2023

Conversation

ksco
Copy link
Collaborator

@ksco ksco commented Oct 13, 2023

Reported by @fan-wenjie

@ksco ksco marked this pull request as draft October 13, 2023 14:29
@ksco ksco changed the title [DYNAREC][WIP] Fix x87cache issues [DYNAREC] Fix x87cache issues Oct 13, 2023
@ksco ksco marked this pull request as ready for review October 14, 2023 10:48
@ksco
Copy link
Collaborator Author

ksco commented Oct 14, 2023

@ptitSeb Seems working, but I can't test real programs right now.

@ptitSeb
Copy link
Owner

ptitSeb commented Oct 14, 2023

Looks ok. I would have created new macro for PUSH/PUSHEMPTY/POP for easier changes, but this one works too.

I can wait if you can test a bit later.

@ksco
Copy link
Collaborator Author

ksco commented Oct 14, 2023

I can wait if you can test a bit later.

Are you able to test it for arm64? Like running some games (UT2004 maybe?) or WOW64 apps that use x87, I want to make sure it doesn't break things that are already working. I'll do some tests for rv64 later today.

I would have created new macro for PUSH/PUSHEMPTY/POP

Yeah, this looks better, but we'll only need to create new macros for PUSH/PUSHEMPTY, POP empty is harder to detect as we assume the x87cache is empty at the entry of a block IIUC.

@ptitSeb
Copy link
Owner

ptitSeb commented Oct 14, 2023

I would have created new macro for PUSH/PUSHEMPTY/POP

Yeah, this looks better, but we'll only need to create new macros for PUSH/PUSHEMPTY, POP empty is harder to detect as we assume the x87cache is empty at the entry of a block IIUC.

Pop empty is the same: you need to test if the stack is -8 (because it can be negative, if you start a block and just pop po pop...)

@@ -974,6 +974,16 @@ void x87_do_pop(dynarec_arm_t* dyn, int ninst, int s1)
}
}

int x87_cachecount(dynarec_arm_t* dyn, int ninst)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, I see, we already have dyn->n.stack for this, so this function is unnecessary then...

@ksco
Copy link
Collaborator Author

ksco commented Oct 14, 2023

Ok, I've tested some games on rv64, seems all good ;)

@ptitSeb
Copy link
Owner

ptitSeb commented Oct 14, 2023

Ok, I'll push then.

@ptitSeb ptitSeb merged commit 9191c99 into ptitSeb:main Oct 14, 2023
28 checks passed
@ksco ksco deleted the x87cache branch October 14, 2023 16:50
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.

2 participants