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.

Sign up for GitHub

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

Jump to bottom

PPU LLVM: Add FMA accuracy setting #7874

Merged
merged 5 commits into from Mar 31, 2020
Merged

PPU LLVM: Add FMA accuracy setting #7874

merged 5 commits into from Mar 31, 2020

Conversation

elad335
Copy link
Contributor

@elad335 elad335 commented Mar 27, 2020 •
edited

Followup for #7135, implements as "PPU Accurate FMA" setting in config file.
When this option enabled it changes the PPU cache version changes and if you do not have cache for x option state it will generate new PPU LLVM cache.

@elad335 elad335 force-pushed the cure-for-covid-19 branch 2 times, most recently from 6cce131 to 0d4627d Compare March 27, 2020 08:21
@elad335 elad335 force-pushed the cure-for-covid-19 branch 4 times, most recently from 4f4abb8 to fb50f45 Compare March 27, 2020 09:06
@elad335 elad335 changed the title PPU LLVM: Add FMA accuracy setting [WIP] PPU LLVM: Add FMA accuracy setting Mar 27, 2020
@MSuih MSuih mentioned this pull request Mar 27, 2020
Closed
@AniLeo
Copy link
Member

AniLeo commented Mar 27, 2020

Is the performance penalty that big to warrant an accuracy setting?

@elad335
Copy link
Contributor Author

elad335 commented Mar 27, 2020

It depends on the cpu really, on cpus from the last ~5 years it may result in a slight performance increase actually.
On cpus which lack support for native FMA it can murder framerates.

@AniLeo
Copy link
Member

AniLeo commented Mar 27, 2020

I think we should avoid the extra setting then. All CPUs from Recommended Specs have FMA as far as I'm aware
If anything, we can make a CPU check to avoid older CPUs having the huge performance hog

@doesthisusername @elad335
PPU LLVM : Match PS3 for the instructions fmadd, fmadds, fmsub, fmsub…
5ddd716 
…s, fnmadd, fnmadds, fnmsub, fnmsubs

Co-Authored-By: Eladash <elad3356p@gmail.com>
@JohnHolmesII
Copy link
Contributor

At the very least, it should default to on. But I also agreed with Ani, I don't think a setting is warranted. Maybe make it an option in the other direction? That is, make it "PPU fast fma" and false by default. If someone really needs to they can enable inaccuracy.

@elad335 elad335 changed the title [WIP] PPU LLVM: Add FMA accuracy setting PPU LLVM: Add FMA accuracy setting Mar 27, 2020
@Whatcookie Whatcookie mentioned this pull request Mar 30, 2020
Closed
@Nekotekina
Copy link
Member

Nekotekina commented Mar 31, 2020 •
edited

I think it shouldn't check for m_use_fma. This would make setting work consistently.

@AniLeo
Copy link
Member

AniLeo commented Mar 31, 2020

The point is for the setting to be forced on fma enabled CPUs

@elad335
Copy link
Contributor Author

elad335 commented Mar 31, 2020

Cured Fixed.

@Nekotekina Nekotekina merged commit 92f821a into RPCS3:master Mar 31, 2020
@elad335 elad335 deleted the cure-for-covid-19 branch April 1, 2020 06:54
@illusion0001 illusion0001 mentioned this pull request Apr 1, 2020
Merged
@AniLeo AniLeo mentioned this pull request Apr 15, 2020
Closed
@web1018 web1018 mentioned this pull request May 3, 2020
Closed
@jobs-git jobs-git mentioned this pull request May 17, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Megamouse Megamouse Megamouse left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@elad335 @AniLeo @JohnHolmesII @Nekotekina @Megamouse @doesthisusername