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

SPU LLVM: Add relaxed xfloat option #11461

Merged
merged 1 commit into from
Jan 31, 2022
Merged

Conversation

Whatcookie
Copy link
Member

This new setting is enabled by default and enables "approximate" accuracy for the FCGT and FNMS instructions. The relaxed setting is not exposed to the GUI, but the approximate option is now available on the CPU tab.

To test simply disable approximate xfloat.

Provides around a 10% performance boost in games which don't need the full approximate accuracy (GOW3, Wipeout HD)

closes #11420

@@ -52,6 +52,7 @@ struct cfg_root : cfg::node
cfg::_enum<tsx_usage> enable_TSX{ this, "Enable TSX", enable_tsx_by_default() ? tsx_usage::enabled : tsx_usage::disabled }; // Enable TSX. Forcing this on Haswell/Broadwell CPUs should be used carefully
cfg::_bool spu_accurate_xfloat{ this, "Accurate xfloat", false };
cfg::_bool spu_approx_xfloat{ this, "Approximate xfloat", true };
cfg::_bool spu_relaxed_xfloat{ this, "Relaxed xfloat", true }; // Approximate accuracy for only the "FCGT" and "FNMS" instructions
Copy link
Contributor

Choose a reason for hiding this comment

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

we now have 3 xfloat settings.
wouldn't it make more sense to use an enum?

Copy link
Member Author

Choose a reason for hiding this comment

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

Neko wanted accurate and approximate to be separate settings, but maybe I can merge relaxed and approx.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine,

- This new setting is on by default
- It's active when approximate default is disabled
- Approximate xfloat is now exposed to the gui
@psennermann
Copy link

psennermann commented Jan 31, 2022

I think the description in the CPU tab is somewhat misleading, because if you write that the option "fixes bugs in various games at the cost of performance" most people would think that disabling (unticking) it would bring a speed up, instead it is the opposite, or have I missed something?

ok, so the fastest setting must be the option unticked...and the previous default (accurate?) option is gone?

I give up, there's a second option for the same thing :-/ But what happens if I enable both? Shouldn't be grayed out?

@Linear524
Copy link

Yeah, it is still a bit confusing - how SPU float vectors processing will behave if "Accurate Xfloat" will be enabled with "Approximate Xfloat" being disabled ?
Will it be similar to both options enabled ?

P.S. - for my rig Relaxed Xfloat just gain only + 2~4 fps in GoW3, so it might be not so good to sacrifice accuracy of other instructions... I mean - maybe it might hit overall stability ?

@Nekotekina
Copy link
Member

Yeah, approximate should be grayed out since it's overridden.

@marcoluc97
Copy link
Contributor

Maybe I'm wrong but can we have a slider for this 3 levels of accuracy or something similar?

@Nekotekina
Copy link
Member

There are other accuracy options which may be added in future so it may look awkward this way.

@HerrHulaHoop
Copy link

HerrHulaHoop commented Feb 1, 2022

The UI presentation of these options needs work. All these options are mutually exclusive so we should have them in a drop-down or radio button layout.

Slider wouldn't work with our current UI language (we use sliders strictly for numbers).

Just realised this is the PR thread, let's move this discussion to a separate issue.

@Linear524
Copy link

Maybe it would be better to use good old debug tab with the list of Instructions accuracy drop down list?
1705
1706

@Darkhost1999
Copy link
Contributor

We don't want the setting to be hidden by default. Debug is a no go.
But I don't see what the problem is. Approximate being enabled means Approximate xfloat, accurate being enabled means accurate xfloat, both disabled means relaxed.
True a drop down would be beneficial yet if there's plans for more options maybe it should wait for those changes to be added to the drop down and keep them exactly where they are in the cpu tab because it's a cpu instruction.

@AniLeo
Copy link
Member

AniLeo commented Feb 1, 2022

Maybe it would be better to use good old debug tab with the list of Instructions accuracy drop down list?
1705
1706

RPCS3 is not a nuclear power plant managing software

@legend800
Copy link

legend800 commented Feb 6, 2022

I've read through this and the associate feature request but it's still super confusing to have 2 boxes for 3 options, with no greying out. Like is this a valid option? If not, why are we allowing it and giving users more configs to test that don't do anything.

2022-02-05 18_16_46-Window

@Xcedf
Copy link

Xcedf commented Feb 6, 2022

@legend800 dude, almost nothing changed, Approximate Xfloat was always there, just wasn't in emu's Ui, Relaxed mode also was always there, if both Approx and Accurate are inactive, so if none checked here goes Relaxed mode. If Accurate is checked it overrides Approximate even if it's checked too, but yea graying out would be nice for end users.

@legend800
Copy link

Dude....that's what we're saying. It looks like there's 4 possible configs - Both Off, Both On, Accurate On/Approx Off, Accurate Off/Approx On. When really it's just 3: Both off (using secret Relaxed), Accurate On (Approx should be grey), Approx On (Accurate grey).

Really the dropdown above is the only way to fix. There's 3 options. Let them select it from the list instead of a mini-puzzle. Not sure why there's push back on that when there's dropdowns for almost everything.

Keep in mind this is the first thing that users see - center in first tab. It should make sense and not mislead them to think Both On is using Approx for example which I literally thought today (been using emu for 4yrs) would override Accurate - tooltip doesn't explain this at all and users will be split on what they think will happen with both checked.

The team keeps adding options to the UI, here's a change to reduce it by 1 AND simply the CX for users.

@Emogop
Copy link

Emogop commented Feb 6, 2022

Relaxed Xfloat(new):
Screenshot_1
Approximate Xfloat:
Screenshot_2

Persona 5
Relaxed Xfloat(new):
Screenshot_3
Approximate Xfloat:
Screenshot_4

@HerrHulaHoop
Copy link

Move this discussion to a separate issue thread. There's no point spamming the PR thread with this.

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.

[Feature Request] SPU Instructions Accuracy Levels.