Skip to content

cpuinfo: Use auxv for AltiVec on Linux if possible #12955

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

Merged
merged 1 commit into from
May 4, 2025

Conversation

awilfox
Copy link
Contributor

@awilfox awilfox commented May 4, 2025

Description

The SIGILL handler is not very reliable and can cause crashes.

Linux provides the CPU's AltiVec support status in getauxval.

Existing Issue(s)

None seem to be reported to SDL itself. A few distro-specific bugs have been submitted. We were notified by a user trying to run SuperTux on Wii U, which has a PPC 750 (like G3) which does not support AltiVec. They received a crash (with SIGILL) before this patch, and a working game after this patch.

Note that SuperTux uses SDL 2, and the patch was applied to our Espresso SDL 2 package. I wasn't able to find any SDL 3 clients to easily test on a Wii U, so I can't say for sure SDL 3 has the same bug nor can I say this is 100% guaranteed to work, but it seemed the right thing to submit here anyway. I'm not sure if there is still a way to submit fixes for SDL 2, but it would be great to have this there too, if there is another patch release planned.

The SIGILL handler is not very reliable and can cause crashes.

Linux provides the CPU's AltiVec support status in getauxval.
@slouken slouken merged commit 7490471 into libsdl-org:main May 4, 2025
39 checks passed
@slouken
Copy link
Collaborator

slouken commented May 4, 2025

This is merged to main and cherry-picked for the next SDL2 and SDL3 releases.

However, I think this will fail to build if altivec blitters is defined and __powerpc__ is not defined. Can you check that?

@awilfox
Copy link
Contributor Author

awilfox commented May 4, 2025

Hmm, yes, logically it would, but is it possible to have a environment where SDL_PLATFORM_LINUX and HAVE_GETAUXVAL are defined, SDL_ALTIVEC_BLITTERS is defined, and __powerpc__ isn't defined?

I suppose that the __powerpc__ check could be replaced with SDL_ALTIVEC_BLITTERS.

@@ -115,7 +115,7 @@
#define CPU_CFG2_LSX (1 << 6)
#define CPU_CFG2_LASX (1 << 7)

#if defined(SDL_ALTIVEC_BLITTERS) && defined(HAVE_SETJMP) && !defined(SDL_PLATFORM_MACOS) && !defined(SDL_PLATFORM_OPENBSD) && !defined(SDL_PLATFORM_FREEBSD)
#if defined(SDL_ALTIVEC_BLITTERS) && defined(HAVE_SETJMP) && !defined(SDL_PLATFORM_MACOS) && !defined(SDL_PLATFORM_OPENBSD) && !defined(SDL_PLATFORM_FREEBSD) && (defined(SDL_PLATFORM_LINUX) && !defined(HAVE_GETAUXVAL))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#if defined(SDL_ALTIVEC_BLITTERS) && defined(HAVE_SETJMP) && !defined(SDL_PLATFORM_MACOS) && !defined(SDL_PLATFORM_OPENBSD) && !defined(SDL_PLATFORM_FREEBSD) && (defined(SDL_PLATFORM_LINUX) && !defined(HAVE_GETAUXVAL))
#if defined(SDL_ALTIVEC_BLITTERS) && defined(HAVE_SETJMP) && !defined(SDL_PLATFORM_MACOS) && !defined(SDL_PLATFORM_OPENBSD) && !defined(SDL_PLATFORM_FREEBSD) && !(defined(SDL_PLATFORM_LINUX) && defined(HAVE_GETAUXVAL))

Otherwise this will be disabled on all non-Linux platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you're right - thanks! I'll fix this up in a few hours.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, you're right - thanks! I'll fix this up in a few hours.

Done in 29d2116

Copy link
Contributor

Choose a reason for hiding this comment

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

Done in 29d2116

Needs applying to release-3.2.x and possibly to SDL2 and release-2.32.x as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, in progress.

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.

4 participants