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

Add SSE2, AVX2 & NEON support to pygame.print_debug_info() #2897

Merged
merged 1 commit into from
Jun 8, 2024

Conversation

MyreMylar
Copy link
Member

These are the most common instruction sets we support paths for and could assist us in detecting bugs. There are more instruction sets listed in system.get_cpu_instuction_sets() but currently we don't make much use of them in pygame-ce.

Suggested by @Starbuck5

These are the most common instruction sets we support
paths for and could assist us in detecting bugs.
@MyreMylar MyreMylar requested a review from a team as a code owner June 1, 2024 13:18
Copy link
Member

@damusss damusss left a comment

Choose a reason for hiding this comment

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

Great idea, will help detect bugs like the recent segfaults more easily

Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@Starbuck5 Starbuck5 left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@Starbuck5 Starbuck5 left a comment

Choose a reason for hiding this comment

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

Ankith wanted to review this so slapping down a request changes to hold it until he can take a look.

Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

I don't have a problem with this PR per se, but my opinion is that we should probably have a centralized way to get and set the SIMD level at runtime.

Everytime someone has to test a lower SIMD level they have to mess with either the buildconfig or the source code itself, and this is not ideal. Especially if we want a user to test out something.

One approach is to have env variables PG_ENABLE_[AVX2|SSE2|NEON] that can be configured before pygame init. On init, these variables are read, and along with runtime detection, we set the corresponding runtime check values.

Another approach is to have some API, probably in pygame.system, to pythonically get/set this

@Starbuck5
Copy link
Member

I don't have a problem with this PR per se, but my opinion is that we should probably have a centralized way to get and set the SIMD level at runtime.

You should open an issue about that. It shouldn't block this PR.

We will get this for free when SDL3 comes out, btw. SDL_HINT_CPU_FEATURE_MASK.

Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

True, I'm opening a separate issue for it, as it is independent of this PR.

Thanks for the PR 🎉

@oddbookworm oddbookworm dismissed Starbuck5’s stale review June 8, 2024 01:46

Starbuck approved, but requested changes so Ankith could review, Ankith approved, so I'm dismissing this so I can merge it

@oddbookworm oddbookworm merged commit 2920a8d into main Jun 8, 2024
38 checks passed
@oddbookworm oddbookworm deleted the add-common-cpu-instuctions-to-debug-info branch June 8, 2024 01:47
@ankith26 ankith26 added this to the 2.5.1 milestone Jun 11, 2024
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.

5 participants