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

Purge SSE2/SSE3 #4329

Merged
merged 7 commits into from
Apr 10, 2021
Merged

Purge SSE2/SSE3 #4329

merged 7 commits into from
Apr 10, 2021

Conversation

GovanifY
Copy link
Member

People are going to hate me for this one.

This removes all the dead code used to support SSE2/SSE3 accelerations.

@GovanifY GovanifY force-pushed the purge-sse2 branch 13 times, most recently from 1a46a43 to 8b864e1 Compare March 26, 2021 23:43
@GovanifY GovanifY added this to the Release 1.8 milestone Mar 27, 2021
@TellowKrinkle
Copy link
Member

Not sure what happened but GSVector4.h has a bunch of random spaces added

@TellowKrinkle
Copy link
Member

Also check the recompilers for checks to x86caps.hasStreamingSIMD*

@GovanifY
Copy link
Member Author

Also check the recompilers for checks to x86caps.hasStreamingSIMD*

Did that and looked at non GS source too, I kept the SSE2 checks for the "features detected" log though

@GovanifY
Copy link
Member Author

Not sure what happened but GSVector4.h has a bunch of random spaces added

Went over the patch and fixed the spacing issues, there were some on GSVector4i.h too

pcsx2/gui/AppInit.cpp Outdated Show resolved Hide resolved
pcsx2/x86/microVU_Misc.inl Outdated Show resolved Hide resolved
@GovanifY GovanifY force-pushed the purge-sse2 branch 2 times, most recently from 930def7 to fdd0f06 Compare March 27, 2021 08:12
@lightningterror
Copy link
Contributor

Keep solution, spu, core changes in separate commits.
If we want to completely wipe sse2 I think I saw a bunch in x86emitter and ipu for example.

@GovanifY
Copy link
Member Author

Keep solution, spu, core changes in separate commits.

Ack

If we want to completely wipe sse2 I think I saw a bunch in x86emitter and ipu for example.

I didn't find any relevant use of x86caps or _M_SSE, you'll have to point me to them

@TellowKrinkle
Copy link
Member

TellowKrinkle commented Mar 27, 2021

Also GSDrawScanlineCodeGenerator.x86.cpp and GSDrawScanlineCodeGenerator.cpp have some m_cpu.has(util::Cpu::tSSE41)s including a massive disgusting thing in ReadTexel_SSE

@GovanifY
Copy link
Member Author

Keep solution, spu, core changes in separate commits.

split everything into different commits

@xddxd
Copy link
Contributor

xddxd commented Mar 27, 2021

You’re welcome. Old hardware should not block progress from being made. What modern Pentiums don’t support SSE4? Pentiums support it since Sandy Bridge.

@TellowKrinkle
Copy link
Member

There's 2 main issues

  1. The plan for the GSdx merge involves disabling multiply-built GSdx as we carry out the merge. GSdx benefits most from SSE4 (AVX/AVX2 affect it much less), so the officially built PCSX2 will definitely drop SSE3 for that period whether we keep it as a build-time option or not
  2. We keep accidentally breaking SSE3. SSE4 has a lot of useful instructions (min/max of integers, pinsr/pextr, movsx/movzx) that we end up adding to new code and not noticing until someone comes in with a sigill later. Most recently, SSE3 was broken for two months from ee07f86 to f9d96f5

I'll probably try to maintain SSE2 on my fork through the next official release as I'd like to have a fallback for Mac users (on Windows we can say "use 1.6.0" but there is no 1.6.0 for Mac), and since that'll include the plugin merge, going from there will probably not have too many merge conflicts to deal with for anyone else who wants to have SSE2 support, and anyone who is trying to keep SSE2 alive probably actually has an SSE2 computer they can test on to make sure new changes actually work

@asturur
Copy link

asturur commented Mar 27, 2021

Again if the project is hard to mantain, you loose the ability to run newer versions entirely, because developers may stop doing the emulator at all. Have developers do what they have to do.

@ghost
Copy link

ghost commented Mar 27, 2021

Again if the project is hard to mantain, you loose the ability to run newer versions entirely, because developers may stop doing the emulator at all. Have developers do what they have to do.

I am talking with the devs as you are seeing here. Let the conversation run, please.

We keep accidentally breaking SSE3. SSE4 has a lot of useful instructions (min/max of integers, pinsr/pextr, movsx/movzx) that we end up adding to new code and not noticing until someone comes in with a sigill later. Most recently, SSE3 was broken for two months.

I see... Well, that is understandable. But isn't it possible to shield all the SSE4 stuff from SSE2/SSE3 stuffs? Excuse me if I am talking nonsense but I am just asking because you can do this very thing to shield x86 code from x86_64 code.

I'll probably try to maintain SSE2 on my fork through the next official release as I'd like to have a fallback for Mac users

My fork will also keep SSE2 support. But all of the backwards and legacy support aside, I am currently trying to find a way to continue #4199 because I, again, do not have a SSE4-compatible system on hand. If it was not for the GSdx merge this would have been simple, just edit the plugin itself, but it will be much more difficult after the merge.

Perhaps I can find a workaround to this but we will see.

@fantesykikachu
Copy link

I see... Well, that is understandable. But isn't it possible to shield all the SSE4 stuff from SSE2/SSE3 stuffs? Excuse me if I am talking nonsense but I am just asking because you can do this very thing to shield x86 code from x86_64 code.

By shield you mean duplicate, and yes in theory they could keep it but it would be horrible to maintain. It is the same way with x86 and x86_64, right now there is a lot of duplication but as soon as x86_64 is fully working the plan is to drop x86 (32bit) support, just like many other emulators did like five years ago.

@tadanokojin
Copy link
Member

Steam Surveys cannot be applied to emulation

Then nothing can be used because there are no stats that applies specifically to ps2 emulation (at least without telepathy, which is non-starter afaic) and we effectively have no data to rely on. Did you see the qualifier "potentially" in that sentence? I don't mean to be condescending but I'm not sure you realized that word is doing the work of telling you that this isn't meant to be interpreted as 1:1 with emulation users.

It's a decent sampling of gamers more broadly but even so I'm accounting for the fact that the sample might be slightly bias and I still don't think SSE2 is worth it. I think it's fair to recognize the overlap between this two groups of computer users and account for any potential bias in the data rather than just dismiss it outright.

Additionally, you're welcome to present other stats you feel are more accurately represent out user-base.

Not everyone who emulated will be rockin' a 1080 Ti or a Ryzen 7 or somethin',

I didn't suggest everyone does and neither does steam. AMD is barely a quarter of the market let alone a Ryzen 7 and a 1080ti isn't even in the top 10. I'm assuming you're exaggerating for effect but I'm not really sure what you're trying to suggest here.

the great deal with emulation is accessibility. And removing SSE2 will just stand against this.

You need to draw the line somewhere and It's clearly balancing act. Bring more than an absolutest stance about accessibility. Where is the line and why?

KH and Soulcalibur are NOT the only games I run

Nothing about that suggested they were the only games you run, just that they were games I'm aware you run which I preemptively addressed before they came up. KH in particular I wanted to address preemptively because it's an exceptional case that I suspected would come up given the nature of the conversation.

I wasn't even aware that you played SC specifically, I said "2D fighting games" because that seemed more charitable than the other 2D games I could think of as an example (visual novels). SC is a 3D fighting game last I checked.

Quoting one guy who said 'If you still use a CPU without SSE4 at this point it's likely under the minimum regardless so it wouldn't be supported anyway.' above.

That's not saying you can't use it. It's saying we likely won't provide support for it because it's too under-powered, which is true. I'm going to be charitable and say you probably just didn't understand what they were saying.

I can understand cutting support on deprecated APIs (DX9) or unsupported OSes (Win7, Win8) even though I do not support those decisions. But it seems baffling to e that you would cut out a supported architecture (All modern OSes will still run on SSE2) and in fact one that many of your peers still use.

We're not developing an OS or anything close to it. This is a confused point, d3d9 isn't deprecated afaik and I'm pretty sure there are operating systems that require at least SSE4 (MacOS and some linux distros I'm pretty sure).

@seta-san
Copy link
Contributor

seta-san commented Mar 28, 2021

I feel for you. But we're talking about computers 10 to 15 years old. We dump computers by the dozens half as old.. I think you said you had a 775 socket computer. Don't core 2 quads supports sse4?

@TellowKrinkle
Copy link
Member

TellowKrinkle commented Mar 28, 2021

My fork will also keep SSE2 support. But all of the backwards and legacy support aside, I am currently trying to find a way to continue #4199 because I, again, do not have a SSE4-compatible system on hand. If it was not for the GSdx merge this would have been simple, just edit the plugin itself, but it will be much more difficult after the merge.

I usually have two branches for PRs, one with MacOS patches and one without

I work on the one with patches, and when I'm done I git cherry-pick the commits onto the one I actually PR

You can git cherry-pick start..end to cherry-pick a range of commits at once

@asturur
Copy link

asturur commented Mar 28, 2021

Again if the project is hard to mantain, you loose the ability to run newer versions entirely, because developers may stop doing the emulator at all. Have developers do what they have to do.

I am talking with the devs as you are seeing here. Let the conversation run, please.

I do not think this is the way to facilitate any running conversation. You should use any private channel with developers if you can't stand other points of view.

Copy link
Contributor

@lightningterror lightningterror left a comment

Choose a reason for hiding this comment

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

I think it looks good, could use some testing just to make sure nothing broke.
Maybe Tellow or Li can double check if something got left out.

@GovanifY
Copy link
Member Author

@Mrlinkwii tested the PR and it seemed fine, @TellowKrinkle care to double check the PR in case?

@RedDevilus
Copy link
Contributor

RedDevilus commented Mar 29, 2021

@TellowKrinkle
Copy link
Member

TellowKrinkle commented Mar 29, 2021

Looks good to me, though I'd prefer if you reordered the commits so that they all build (build system changes first)

Edit: Also rebase the part where you un-delete things you deleted together so it remains unchanged by any of the commits

Copy link
Member

@tadanokojin tadanokojin left a comment

Choose a reason for hiding this comment

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

GS stuff looks okay to me, I think you should merge the two GS commits.

@GovanifY
Copy link
Member Author

GovanifY commented Mar 29, 2021

Looks good to me, though I'd prefer if you reordered the commits so that they all build (build system changes first)

Edit: Also rebase the part where you un-delete things you deleted together so it remains unchanged by any of the commits

done

GS stuff looks okay to me, I think you should merge the two GS commits.

done

Copy link
Contributor

@RedDevilus RedDevilus left a comment

Choose a reason for hiding this comment

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

LGTM.

@lightningterror lightningterror merged commit 12e9ddf into master Apr 10, 2021
@lightningterror lightningterror deleted the purge-sse2 branch April 10, 2021 21:19
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.