Skip to content

feature module: Fix handling of multiple conflicts per attribute #12

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

Open
wants to merge 1 commit into
base: main-numpymeson
Choose a base branch
from

Conversation

Vogtinator
Copy link

  • Attributes without match were never actually added to the list
  • Only the last conflict actually had an effect, earlier results were discarded

- Attributes without match were never actually added to the list
- Only the last conflict actually had an effect, earlier results were
  discarded
@mattip
Copy link
Member

mattip commented Apr 3, 2024

Also see mesonbuild#11307 which will make this module available in upstream meson

@mattip
Copy link
Member

mattip commented Apr 3, 2024

@seiko2plus, @rgommers could you take a look?

@rgommers
Copy link
Member

rgommers commented Apr 5, 2024

This seems reasonable to me, but it's @seiko2plus's code and it needs to be integrated into his feature branch too in order not to lose this fix when we rebase or discard this fork. So I'll leave it to @seiko2plus to review/incorporate.

@Vogtinator
Copy link
Author

Vogtinator commented Jun 11, 2024

Ping @seiko2plus

@Vogtinator
Copy link
Author

Ping.

@rgommers
Copy link
Member

Sorry this stalled @Vogtinator. Since @seiko2plus isn't available, I'll attempt to get this merged. I tested an integration branch with this change included in numpy/numpy#27550, and got one relevant failure on armhf under QEMU:

 $ /usr/bin/python -c 'import sys; del sys.path[0]; import numpy'
Traceback (most recent call last):
  File "/numpy/build-install/usr/lib/python3/dist-packages/numpy/_core/__init__.py", line 23, in <module>
    from . import multiarray
  File "/numpy/build-install/usr/lib/python3/dist-packages/numpy/_core/multiarray.py", line 10, in <module>
    from . import overrides
  File "/numpy/build-install/usr/lib/python3/dist-packages/numpy/_core/overrides.py", line 7, in <module>
    from numpy._core._multiarray_umath import (
ImportError: /numpy/build-install/usr/lib/python3/dist-packages/numpy/_core/_multiarray_umath.cpython-310-arm-linux-gnueabihf.so: undefined symbol: _ZN2np7highway10qsort_simd11QSort_ASIMDIjEEvPT_i

From the description and numpy/numpy#26151 it seems that you were actually trying to fix something for armhf? I don't have local access to armhf, but either way it would be helpful to have a clearer description of how exactly things are broken for you without this PR.

@Vogtinator
Copy link
Author

From the description and numpy/numpy#26151 it seems that you were actually trying to fix something for armhf? I don't have local access to armhf, but either way it would be helpful to have a clearer description of how exactly things are broken for you without this PR.

Yes, probably caused by:

  ASIMD.update(args: [
    {'val': '-mfpu=neon-fp-armv8', 'match': '-mfpu=.*'},
    '-march=armv8-a+simd'
  ])

In the build log the issue is visible as:

Test features "ASIMD" : Unsupported due to Compiler fails against the test code of "ASIMD"
Test features "ASIMDHP" : Unsupported due to Implied feature "ASIMD" is not supported
Test features "ASIMDFHM" : Unsupported due to Implied feature "ASIMD" is not supported
Test features "SVE" : Unsupported due to Implied feature "ASIMD" is not supported
Configuring npy_cpu_dispatch_config.h using configuration
Message: 
CPU Optimization Options
  baseline:
    Requested : min
    Enabled   : 
  dispatch:
    Requested : max -xop -fma4
    Enabled   : NEON NEON_FP16 NEON_VFPV4

With this PR, ASIMD should show up as supported by the compiler and get used for runtime dispatch.

@Vogtinator
Copy link
Author

Ping

@seiko2plus
Copy link
Member

@Vogtinator, sorry for being away for a while. I confirm the bug you reported on armhf and your patch fixes it. Regarding the error @rgommers caught from CI, it's highway/QSort related, likely marked as incompatible platform via VQSORT_COMPILER_COMPATIBLE. As a quick fix, I will apply your patch to bring asimd support for arm7 for the rest of the kernels.

seiko2plus added a commit to seiko2plus/numpy that referenced this pull request Apr 8, 2025
  The fix in numpy/meson#12 for ASIMD*(32-bit) compile-time feature detection revealed a new
  build error on aarch32 platforms:

    ImportError: /numpy/build-install/usr/lib/python3/dist-packages/numpy/_core/
    _multiarray_umath.cpython-310-arm-linux-gnueabihf.so: undefined symbol:
    _ZN2np7highway10qsort_simd11QSort_ASIMDIjEEvPT_i

  This patch prevents platform detection constants of Highway from being exposed across
  translation units with different compiler flags (baseline). This approach
  eliminates detection mismatches that were causing symbol resolution failures
  in the Highway QSort implementation.
seiko2plus added a commit to seiko2plus/numpy that referenced this pull request Apr 8, 2025
  The fix in numpy/meson#12 for ASIMD*(32-bit) compile-time feature detection revealed a new
  build error on aarch32 platforms:

    ImportError: /numpy/build-install/usr/lib/python3/dist-packages/numpy/_core/
    _multiarray_umath.cpython-310-arm-linux-gnueabihf.so: undefined symbol:
    _ZN2np7highway10qsort_simd11QSort_ASIMDIjEEvPT_i

  This patch prevents platform detection constants of Highway from being exposed across
  translation units with different compiler flags (baseline). This approach
  eliminates detection mismatches that were causing symbol resolution failures
  in the Highway QSort implementation.
Copy link
Member

@seiko2plus seiko2plus left a comment

Choose a reason for hiding this comment

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

@Vogtinator, LGTM, thank you!

Regarding your note about test failures on armhf under QEMU (mentioned in numpy/numpy#27550): @rgommers, I can confirm that the armhf test now passes in PR numpy/numpy#28635 after fixing the link error."

seiko2plus added a commit to seiko2plus/numpy that referenced this pull request Apr 8, 2025
  The fix in numpy/meson#12 for ASIMD*(32-bit) compile-time feature detection revealed a new
  build error on aarch32 platforms:

    ImportError: /numpy/build-install/usr/lib/python3/dist-packages/numpy/_core/
    _multiarray_umath.cpython-310-arm-linux-gnueabihf.so: undefined symbol:
    _ZN2np7highway10qsort_simd11QSort_ASIMDIjEEvPT_i

  This patch prevents platform detection constants of Highway from being exposed across
  translation units with different compiler flags (baseline). This approach
  eliminates detection mismatches that were causing symbol resolution failures
  in the Highway QSort implementation.
seiko2plus added a commit to seiko2plus/numpy that referenced this pull request Apr 8, 2025
  The fix in numpy/meson#12 for ASIMD*(32-bit) compile-time feature detection revealed a new
  build error on aarch32 platforms:

    ImportError: /numpy/build-install/usr/lib/python3/dist-packages/numpy/_core/
    _multiarray_umath.cpython-310-arm-linux-gnueabihf.so: undefined symbol:
    _ZN2np7highway10qsort_simd11QSort_ASIMDIjEEvPT_i

  This patch prevents platform detection constants of Highway from being exposed across
  translation units with different compiler flags (baseline). This approach
  eliminates detection mismatches that were causing symbol resolution failures
  in the Highway QSort implementation.
charris pushed a commit to charris/numpy that referenced this pull request Apr 10, 2025
  The fix in numpy/meson#12 for ASIMD*(32-bit) compile-time feature detection revealed a new
  build error on aarch32 platforms:

    ImportError: /numpy/build-install/usr/lib/python3/dist-packages/numpy/_core/
    _multiarray_umath.cpython-310-arm-linux-gnueabihf.so: undefined symbol:
    _ZN2np7highway10qsort_simd11QSort_ASIMDIjEEvPT_i

  This patch prevents platform detection constants of Highway from being exposed across
  translation units with different compiler flags (baseline). This approach
  eliminates detection mismatches that were causing symbol resolution failures
  in the Highway QSort implementation.
MaanasArora pushed a commit to MaanasArora/numpy that referenced this pull request Apr 11, 2025
  The fix in numpy/meson#12 for ASIMD*(32-bit) compile-time feature detection revealed a new
  build error on aarch32 platforms:

    ImportError: /numpy/build-install/usr/lib/python3/dist-packages/numpy/_core/
    _multiarray_umath.cpython-310-arm-linux-gnueabihf.so: undefined symbol:
    _ZN2np7highway10qsort_simd11QSort_ASIMDIjEEvPT_i

  This patch prevents platform detection constants of Highway from being exposed across
  translation units with different compiler flags (baseline). This approach
  eliminates detection mismatches that were causing symbol resolution failures
  in the Highway QSort implementation.
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