Repository navigation
BLD: Fix PPC64LE build failure with -mcpu=power9 flag - #29627
SumitkCodes wants to merge 2 commits into
Conversation
Fixes numpy#29622: VSX2 targets were missing -mvsx flag and incorrectly using VSX3 intrinsics. - Add -mvsx to VSX2 configuration - Guard VSX3 intrinsics with NPY__CPU_TARGET_VSX3 - Resolves build error when CFLAGS='-mcpu=power9' on PPC64LE
charris
changed the title
|
There are some failing tests around compiler options. |
|
@mattip Working on it |
The test was expecting '-mcpu=power8' but the fix correctly generates '-mcpu=power8 -mvsx' for VSX2 targets. Update test expectations to match the actual (correct) compiler flags being generated. This resolves the failing tests that were expecting the old format while the code correctly generates the new format with -mvsx flag. Fixes the failing tests mentioned in PR numpy#29627.
|
@mattip can you check the changes now |
|
LGTM. It would be nice if @seiko2plus could chime in. |
rgommers
left a comment
rgommers
left a comment
There was a problem hiding this comment.
A few questions. The change does solve a reported issue, but I'm not sure that everything is correct, so some more explanations would be helpful.
| __m128 mf = _mm_load_ss(&f); | ||
| bits_ = _mm_extract_epi16(_mm_cvtps_ph(mf, _MM_FROUND_TO_NEAREST_INT), 0); | ||
| #elif defined(NPY_HAVE_VSX3) && defined(NPY_HAVE_VSX_ASM) | ||
| #elif defined(NPY_HAVE_VSX3) && defined(NPY_HAVE_VSX_ASM) && defined(NPY__CPU_TARGET_VSX3) |
There was a problem hiding this comment.
As explained in https://numpy.org/devdocs/reference/simd/how-it-works.html#generating-the-main-configuration-header, these NPY__CPU_TARGET_ defines shouldn't be used. Why is this change necessary?
There was a problem hiding this comment.
The issue is that VSX2 targets (compiled with -mcpu=power8 -mvsx) were trying to use VSX3 intrinsics, causing compilation errors.
NPY_HAVE_VSX3 tells us the project supports VSX3, but NPY__CPU_TARGET_VSX3 tells us we're currently compiling a VSX3-specific target. We need both to prevent VSX2 targets from using VSX3 intrinsics.
This isn't runtime dispatch it's preventing compilation errors during the build. The macro is only defined when compiling VSX3 targets so it's the right tool for this job.
There was a problem hiding this comment.
NPY__CPU_TARGET_VSX3 is a private #definition and should not be used here.
Filtering out VSX from the regex of VSX3 and VSX4 should fix the build issue.
NPY__CPU_TARGET_VSX3 was originally used internally with distutils for
dispatchable sources, serving as a helper to define implied feature macros.
Since moving to Meson, we explicitly pass implied feature #definitions instead.
There was a problem hiding this comment.
So on other words, adding && defined(NPY__CPU_TARGET_VSX3) will always disables this branch.
| endif | ||
| VSX2 = mod_features.new( | ||
| 'VSX2', 2, implies: VSX, args: {'val': '-mcpu=power8', 'match': '.*vsx'}, | ||
| 'VSX2', 2, implies: VSX, args: ['-mcpu=power8', '-mvsx'], |
There was a problem hiding this comment.
-mvsx is already defined in the VSX feature, and right in this line we have implies: VSX. So this looks like it'll start inserting duplicate flags.
Also, can you explain why 'match': '.*vsx' needs to be dropped?
There was a problem hiding this comment.
I removed the 'match': '.*vsx' thinking the pattern matching wasn't working properly for VSX2 targets
The build error suggests VSX2 targets aren't getting the right flags
but maybe the real issue is elsewhere in the inheritance chain. Should I revert this change?
There was a problem hiding this comment.
Dropping 'match': '.*vsx'. It's only needed here alongside the other targets.
The match key is a regex used to filter implied flags. Normally, the compiler
should imply this by default, which is why it’s explicitly defined here.
There was a problem hiding this comment.
I cannot say what effect implies: VSX has since the cpu features module is still "pending upstream integration, but ultimately it does not in fact seem to have the effect of adding the implied flags. :/
There was a problem hiding this comment.
In fact, nowhere in the entire logs do I see -mvsx anywhere, not even in translation units that get built as "VSX". Instead, they get built with -mcpu=power8. I don't know what this is supposed to imply.
|
@seiko2plus Ping. |
|
hi! any chance we could have this sorted out soon? I'm doing P9 and soon some P10 stuff in Linux + FreeBSD bare metal and pseries guest VMs and i'd like to use numpy and some other tooling on it. is there anything i/we can do to get this sorted out? Thanks! |
|
Oh, we've missed an anniversary. |
eli-schwartz
left a comment
eli-schwartz
left a comment
There was a problem hiding this comment.
I just hit this in gentoo packaging CI. Given:
CPU Optimization Options
baseline:
Requested :
Enabled :
dispatch:
Requested : MAX
Enabled : VSX VSX2 VSX3 VSX4
It fails to build (i.e. USE="cpudetection" CPU_FLAGS_PPC="" emerge numpy disables all baseline, but enables cpudispatch).
The generated command line looks like e.g. this (full log at https://paste.gentoo.zip/8foqtRIX):
powerpc64le-unknown-linux-gnu-gcc -Inumpy/_core/libargfunc.dispatch.h_VSX2.a.p -Inumpy/_core -I../numpy-2.5.1/numpy/_core -Inumpy/_core/include -I../numpy-2.5.1/numpy/_core/include -I../numpy-2.5.1/numpy/_core/src/common -I../numpy-2.5.1/numpy/_core/src/multiarray -I../numpy-2.5.1/numpy/_core/src/multiarray/stringdtype -I../numpy-2.5.1/numpy/_core/src/npymath -I../numpy-2.5.1/numpy/_core/src/umath -I/usr/include/python3.14 -I/var/tmp/portage/dev-python/numpy-2.5.1/work/numpy-2.5.1-python3_14/meson_cpu -fdiagnostics-color=always -Wall -Winvalid-pch -std=c11 -fno-strict-aliasing -O2 -pipe -fno-strict-aliasing -DNDEBUG -fPIC -DNPY_INTERNAL_BUILD -DHAVE_NPY_CONFIG_H -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE=1 -D_LARGEFILE64_SOURCE=1 -O3 -DNPY_HAVE_VSX2 -DHWY_COMPILE_ONLY_STATIC -DHWY_DISABLE_ATTR -mcpu=power8 -DNPY_MTARGETS_CURRENT=VSX2 -MD -MQ numpy/_core/libargfunc.dispatch.h_VSX2.a.p/meson-generated_argfunc.dispatch.c.o -MF numpy/_core/libargfunc.dispatch.h_VSX2.a.p/meson-generated_argfunc.dispatch.c.o.d -o numpy/_core/libargfunc.dispatch.h_VSX2.a.p/meson-generated_argfunc.dispatch.c.o -c numpy/_core/libargfunc.dispatch.h_VSX2.a.p/argfunc.dispatch.c
No -mvsx in sight.
Previously, I see:
Generating multi-targets for "_umath_tests.dispatch.h"
Enabled targets: VSX3, VSX2, VSX, baseline
Generating multi-targets for "argfunc.dispatch.h"
Enabled targets: VSX2, baseline
| endif | ||
| VSX2 = mod_features.new( | ||
| 'VSX2', 2, implies: VSX, args: {'val': '-mcpu=power8', 'match': '.*vsx'}, | ||
| 'VSX2', 2, implies: VSX, args: ['-mcpu=power8', '-mvsx'], |
There was a problem hiding this comment.
I cannot say what effect implies: VSX has since the cpu features module is still "pending upstream integration, but ultimately it does not in fact seem to have the effect of adding the implied flags. :/
| endif | ||
| VSX2 = mod_features.new( | ||
| 'VSX2', 2, implies: VSX, args: {'val': '-mcpu=power8', 'match': '.*vsx'}, | ||
| 'VSX2', 2, implies: VSX, args: ['-mcpu=power8', '-mvsx'], |
There was a problem hiding this comment.
In fact, nowhere in the entire logs do I see -mvsx anywhere, not even in translation units that get built as "VSX". Instead, they get built with -mcpu=power8. I don't know what this is supposed to imply.
Fixes #29622
Problem:
When
CFLAGS='-mcpu=power9'is set on PPC64LE systems, NumPy build fails with:Root Cause:
VSX2 targets were missing
-mvsxflagVSX2 targets incorrectly used VSX3 intrinsics even when compiled for VSX2
Solution:
-mvsxflag to VSX2 configuration inmeson_cpu/ppc64/meson.build-mvsxflag to VSX2 flags innumpy/distutils/ccompiler_opt.pyNPY__CPU_TARGET_VSX3innumpy/_core/src/common/half.hpp