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

Misc: CMake tidy-up, replace non-constant shift immediates in GSVector #10501

Merged
merged 4 commits into from
Dec 30, 2023
Merged

Misc: CMake tidy-up, replace non-constant shift immediates in GSVector #10501

merged 4 commits into from
Dec 30, 2023

Conversation

stenzek
Copy link
Contributor

@stenzek stenzek commented Dec 29, 2023

Description of Changes

Works around the requirement for the offsetof parameter to be constant.

Moves GSVector shift immediates over to a template parameter, since you can't use non-constant shift immediates for NEON, so this needs to happen anyway. And, less importantly, apparently GCC 13 doesn't like it.

Also removes the __m128 overloads - it's too easy to mistake for a variable shift (which doesn't exist in SSE4), instead it takes the shift amount from the lowest 32-bits.

Removes a bunch of redundant checks from the CMake scripts, and uses standardized platform names.

Adds a warning in caps for anyone trying to build with GCC (we only support Clang/MSVC).

Rationale behind Changes

Janitor part 3?
Closes #10500, although GCC 13 is still broken on Ubuntu 23.10.

Suggested Testing Steps

Make sure software renderer still works.
Note to self: do a dump run before merging.

Also removes the __m128 overloads - it's too easy to mistake for a
variable shift (which doesn't exist in SSE4), instead it takes the shift
amount from the lowest 32-bits.
@Ziemas
Copy link
Contributor

Ziemas commented Dec 29, 2023

Compiles on gcc 13.2.1 for me, software renderer seems to work. 👍

@stenzek
Copy link
Contributor Author

stenzek commented Dec 29, 2023

It doesn't build on Ubuntu 23.10, or Fedora 39:

In function ‘__m128i _mm_srli_si128(__m128i, int)’,
    inlined from ‘GSVector4i GSVector4i::srl() const [with int i = 4]’ at /home/user/dev/pcsx2/pcsx2/GS/GSVector4i.h:694:41,
    inlined from ‘void GSState::GIFPackedRegHandlerSTQRGBAXYZ2(const GIFPackedReg*, u32) [with unsigned int prim = 5; bool auto_flush = false; bool index_swap = false]’ at /home/user/dev/pcsx2/pcsx2/GS/GSState.cpp:690:20:
/usr/lib/gcc/x86_64-linux-gnu/13/include/emmintrin.h:1229:10: error: the last argument must be an 8-bit immediate
 1229 |   return (__m128i)__builtin_ia32_psrldqi128 (__A, __N * 8);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Could be a bug, I'm not sure, need to dig further.
But not really a blocker since we don't support gcc anyway.

@stenzek stenzek merged commit 47f8d8c into PCSX2:master Dec 30, 2023
12 checks passed
@stenzek stenzek deleted the cmake branch December 30, 2023 04:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: build failure with gcc-13 in offsetof macro
3 participants