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

Don't use the fast path when there are watchpoints enabled, for writes too #4078

Merged
merged 1 commit into from Sep 5, 2016

Conversation

comex
Copy link
Contributor

@comex comex commented Aug 1, 2016

This change is Reviewable

…s too.

Also fold the check in both functionss into 'slowmem' rather than having
a separate test.  (jo.alwaysUseMemFuncs implies jo.memcheck anyway, as
makes sense.)
@comex comex force-pushed the alwaysUseMemFuncs-for-write branch from 32c3a9a to c51faa4 Compare August 2, 2016 00:41
@aldelaro5
Copy link
Member

aldelaro5 commented Aug 2, 2016

After the edit you made, the write memory checks now work on my end!

thank you :D

EDIT: however, I just noticed it now slows down by half compared to using a memory check without this commit applied...

@comex
Copy link
Contributor Author

comex commented Aug 3, 2016

EDIT: however, I just noticed it now slows down by half compared to using a memory check without this commit applied...

Makes sense, since a whole lot of memory accesses are no longer using the fast path...

Still, this change is correct, and optimization (which is very possible) should come separately.

@comex comex changed the title WIP: Don't use the fast path when there are watchpoints enabled, for writes too Don't use the fast path when there are watchpoints enabled, for writes too Aug 3, 2016
@phire
Copy link
Member

phire commented Aug 8, 2016

Apart from that, LGTM


Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.


Source/Core/Core/PowerPC/JitCommon/Jit_Util.cpp, line 353 [r1] (raw file):

  }

  if (!jit->jo.alwaysUseMemFuncs && !slowmem)

This if condition can be simplified too.


Comments from Reviewable

@phire phire merged commit d29dae2 into dolphin-emu:master Sep 5, 2016
aldelaro5 pushed a commit to aldelaro5/dolphin that referenced this pull request Sep 5, 2016
phire added a commit that referenced this pull request Sep 6, 2016
OrN pushed a commit to OrN/dolphin that referenced this pull request Sep 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants