Skip to content

Conversation

Sanjaykumar030
Copy link
Contributor

This PR continues the work from #29678 to improve CPU feature detection in the Meson build system, specifically for the s390x architecture.

Problem:
The current detection for VXE and VXE2 is ambiguous:

  • VXE2: The regex 'VX.*' is overly greedy and could match any string containing “VX”.
  • VXE: The simple match for 'VX' could incorrectly enable VXE on systems that only support VX.

These ambiguities may cause NumPy to compile with unsupported CPU instructions, risking Illegal Instruction crashes or performance issues.

Solution:
This PR replaces the fragile patterns with precise regular expressions using word boundaries (\b):

  • VXE → '\\bvxe\\b'
  • VXE2 → '\\bvxe2\\b'

No changes are made to the feature hierarchy, compiler flags, or test code.

@Sanjaykumar030 Sanjaykumar030 changed the title FIX: Correct ambiguous logic for s390x CPU feature detection FIX: Correct ambiguous logic for s390x CPU feature detection Sep 6, 2025
@Sanjaykumar030
Copy link
Contributor Author

Hi @jorenham,

This PR refines the s390x CPU feature detection by replacing ambiguous VXE and VXE2 regex patterns with precise word-boundary matches. All tests pass, and there are no changes to compiler flags or feature hierarchy. Could you please review when you have a moment?

Thanks!

@charris charris merged commit 9bc8468 into numpy:main Sep 6, 2025
77 checks passed
@charris
Copy link
Member

charris commented Sep 6, 2025

Thanks @Sanjaykumar030 .

@charris
Copy link
Member

charris commented Sep 6, 2025

I'm am curious about the change to lower case. Is the match independent of case?

@charris charris added 00 - Bug 09 - Backport-Candidate PRs tagged should be backported labels Sep 6, 2025
@charris charris changed the title FIX: Correct ambiguous logic for s390x CPU feature detection BUG: Correct ambiguous logic for s390x CPU feature detection Sep 6, 2025
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Sep 6, 2025
@Sanjaykumar030
Copy link
Contributor Author

Thanks for the question, @charris. The new regex patterns are case-sensitive, chosen to align with the lowercase feature strings reported on s390x. This keeps the behavior consistent with the existing build logic and avoids any breaking changes.

@jorenham
Copy link
Member

jorenham commented Sep 7, 2025

@charris You might want to have a look at this PR by the same author.

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.

3 participants