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

Prefetch MFC list elements #5345

Merged
merged 6 commits into from Jul 23, 2019
Merged

Prefetch MFC list elements #5345

merged 6 commits into from Jul 23, 2019

Conversation

elad335
Copy link
Contributor

@elad335 elad335 commented Nov 15, 2018

Prefetch elements to protect them from destruction by the previous elements processing in the GET list. That is happening when the destination of the command overlaps the list entries.
Seen in Calling All Cars, Brings him from intro to ingame (could be playable).
It seems logical that the mfc in realhw caches chunks of entries rather than one after another each element execution.
Should fix a few other access violations caused by poor commands management, apparently some games are self destructive.
I did only prefetch up to the list end or the list stall, whatever comes first. becuase prefetching more than that seems overkill.

image

@elad335 elad335 changed the title Prefetch mfc list elemets to protect from overwriting Prefetch mfc list elements to protect them from overwriting Nov 15, 2018
@elad335
Copy link
Contributor Author

elad335 commented Dec 30, 2018

Can I get a review on this? seems like a forgotten pr @Nekotekina
tl;dr the game was overwriting its list elements in a GET list command in the first element, on a real ps3 it works since mfc has already fetched a good amount of list elements in one round before tranfers begun.
And it wont reduce perf since we already copy list elements to temp buffer from LS.

@Nekotekina
Copy link
Member

Copying potentially huge list will always cause the overhead with memcpy. It's also unrealistic that PS3 is able to fetch the huge list. I'd expect something rather small, like aligned 16-byte reads.

@elad335
Copy link
Contributor Author

elad335 commented Dec 31, 2018

I've written a testcase specifically for that - https://github.com/elad335/myps3tests/tree/master/spu_tests/Max%20list%20elements%20prefetch%20count
I've executed a GET list where the first element transfers 128 bytes from a "bad" elements list pointing to nulptrs from vm to the list elements itself, other elements in the OG list were filled with empty transfers.
The highest list cmd size this testcase would go is 0x30 bytes (6 elements) without crashing.

@Nekotekina
Copy link
Member

That seems more realistic. This amount should fit into registers instead of memcpying it.

@elad335
Copy link
Contributor Author

elad335 commented Jan 1, 2019

This amount should fit into registers instead of memcpying it.

What do you mean?

@uaqlover
Copy link

any update for this pr

@elad335
Copy link
Contributor Author

elad335 commented Mar 27, 2019

Its pending for review.

@elad335 elad335 force-pushed the spu2 branch 4 times, most recently from 228622c to 21d5c09 Compare March 27, 2019 11:07
@elad335 elad335 changed the title Prefetch mfc list elements to protect them from overwriting Prefetch MFC list elements Mar 29, 2019
@AniLeo AniLeo requested review from Nekotekina and removed request for Nekotekina March 31, 2019 23:32
@uaqlover
Copy link

@elad335 update this pr with Latest master Build want it for testing only

@elad335
Copy link
Contributor Author

elad335 commented Jun 10, 2019

Rewritten to be 100% UB-free.

@elad335 elad335 force-pushed the spu2 branch 2 times, most recently from de23946 to a395765 Compare July 3, 2019 19:51
Also move some stuff away from command processing such as a few constant arguments setup
@uaqlover
Copy link

Waiting for merge 👍👍

if (index == fetch_size)
{
const auto src = _ptr<void>(args.eal & 0x3fff8);
if ((uptr)src % alignof(v128))
Copy link
Member

Choose a reason for hiding this comment

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

Can union be replaced with simple alignas(16) list_element items[fetch_size];, and the whole alignment check with two pathes with single memcpy?

@Nekotekina Nekotekina merged commit 49aefc0 into RPCS3:master Jul 23, 2019
@elad335 elad335 deleted the spu2 branch July 25, 2019 08:44
kd-11 pushed a commit to kd-11/rpcs3 that referenced this pull request Nov 2, 2019
* Prefetch mfc list elemets to protect from overwriting

Also move some stuff away from command processing such as a few constant arguments setup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants