Skip to content

update to work around numpy issue 24067 - #101

Merged
mattip merged 7 commits into
MacPython:mainfrom
mattip:issue-24067
Jul 16, 2023
Merged

mattip merged 7 commits into
MacPython:mainfrom
mattip:issue-24067

Conversation

@mattip

@mattip mattip commented Jun 30, 2023

Copy link
Copy Markdown
Collaborator

No description provided.

@mattip

mattip commented Jul 2, 2023

Copy link
Copy Markdown
Collaborator Author

something is off with the cross-compilation for arm64. Is this a known problem?

@mattip

mattip commented Jul 11, 2023

Copy link
Copy Markdown
Collaborator Author

The macos arm64 cross build is failing to compile.

Maybe we are using an older gfortran-cross that does not know about SVE, there are errors about intrinsics

../kernel/arm64/sgemm_kernel_sve_v2x8.S:873:2: error: instruction requires: sve

@isuruf, we are using this one, any thoughts?

curl -L -O https://github.com/isuruf/gcc/releases/download/gcc-11.3.0-2/gfortran-darwin-arm64-cross.tar.gz

@martin-frbg: is there an environment variable that would skip the SVE intrinsics?

@martin-frbg

martin-frbg commented Jul 11, 2023 •

Copy link
Copy Markdown

NO_SVE might work but this looks more like the (cross)compiler is expecting a "+sve" on the -march= option and not getting it. (does VORTEX even support SVE? not sure how it even gets there... will try to find time to read your log later)

@martin-frbg

Copy link
Copy Markdown

That build is using clang to cross-compile to arm64 under OSX ? Pretty sure that is a cursed configuration - never tried, never could have worked. The only OSX ARM64 target is VORTEX (M1/M2 family) which does not support SVE, and none of the usual DYNAMIC_ARCH targets for ARM64 make sense in this configuration to my knowledge. (DYNAMIC_ARCH even is anything but VORTEX support).
But the primary issue is simply that the -march= option setting in Makefile.arm64 is still geared towards gcc, and while there was some attempt made to handle "or CLANG" it seems I did not add this on all nested conditionals so some version checks may still be failing. This was probably aggravated by the unconditional addition of SVE kernels for the Graviton cpus, so any fallback to non-SVE -march settings for older "gcc" (that may be not gcc at all) would now result in build breakage.
In short, if M1/M2 OSX is your only goal, TARGET=VORTEX without DYNAMIC_ARCH should work. If you really want DYNAMIC_ARCH, use gcc for now (and it would probably make a lot more sense to set TARGET=ARMV8, as that is supposed to be the least common denominator of all cpus in DYNAMIC_ARCH, used to select the compiler options when compiling all the common code in interface,driver,lapack,etc)

@martin-frbg

Copy link
Copy Markdown

Probably fixed by OpenMathLib/OpenBLAS#4136 if you want to try cherry-picking that one.

@mattip

mattip commented Jul 11, 2023 •

Copy link
Copy Markdown
Collaborator Author

I tried the single commit 16624159 using the patch_source command. It does not seem to change the error.

@martin-frbg

Copy link
Copy Markdown

Strange, what is your version of clang ? (Compiles fine for me locally on Samsung Galaxy Tab, need to setup something similar to your CI job to reproduce the circumstances of your crossbuild but cannot think of any fundamental difference)
Pity that your build log only shows the warnings, not the invocation lines to check if any cpu-specific -march gets added at all.

@mattip

mattip commented Jul 12, 2023

Copy link
Copy Markdown
Collaborator Author

Pity that your build log only shows the warnings, not the invocation lines

I remember turning on set +e caused some problems on macos, but maybe that has been improved. Let's see...

@martin-frbg

Copy link
Copy Markdown

Looks like my test job (Xcode 12.4 on Azure CI) autoselected NO_SVE in c_check and consequently left out the Neoverse targets.

@mattip

mattip commented Jul 12, 2023

Copy link
Copy Markdown
Collaborator Author

ahh, the patch_source function was outdated and exited early

@mattip

mattip commented Jul 12, 2023

Copy link
Copy Markdown
Collaborator Author

Pity that your build log only shows the warnings, not the invocation lines

Is there some flag I could add to make to show the invocations? I am using

CFLAGS=' -arch arm64 -fvisibility=protected'
make BUFFERSIZE=20 DYNAMIC_ARCH=1 USE_OPENMP=0 NUM_THREADS=64 BINARY=64 INTERFACE64=1 SYMBOLSUFFIX=64_ OBJCONV=/Users/runner/work/openblas-libs/openblas-libs/objconv/objconv TARGET=VORTEX

and

CFLAGS=' -arch arm64 -fvisibility=protected'
make BUFFERSIZE=20 DYNAMIC_ARCH=1 USE_OPENMP=0 NUM_THREADS=64 BINARY=64 TARGET=VORTEX

@mattip

mattip commented Jul 12, 2023

Copy link
Copy Markdown
Collaborator Author

Clang version is Apple clang version 12.0.5 (clang-1205.0.22.11)

@mattip

mattip commented Jul 12, 2023

Copy link
Copy Markdown
Collaborator Author

There is also this

clang: error: the clang compiler does not support '-mtune=neoverse-v1'

@martin-frbg

Copy link
Copy Markdown

Thanks - have updated my CI job to the same compiler version and substituted "cortex-x1" for neoverse-v1 with clang after checking with --print-supported-cpus. This is probably not the final fix though - according to spack/spack#36481 (comment) , newer AppleClang appears to accept neoverse-v1 just fine but has done away with "vortex" in favor of "apple-m1"

@mattip

mattip commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator Author

This is probably not the final fix though - according to spack/spack#36481 (comment) , newer AppleClang appears to accept neoverse-v1 just fine but has done away with "vortex" in favor of "apple-m1"

Bleh.

@martin-frbg

Copy link
Copy Markdown

my test build succeeds now after adding a bunch of silly casts to various sve kernels in #4140 - just to help clang around a perceived ambiguity

@mattip

mattip commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator Author

Cool, it is green. Do I understand correctly that you are not downloading a cross compiler, rather using the one in XCode?

@martin-frbg

Copy link
Copy Markdown

Yes (or at least I think so :/ )

@mattip

mattip commented Jul 14, 2023

Copy link
Copy Markdown
Collaborator Author

It seems that your cross-compile build does not use a fortran compiler. The cross-compile build here does try very hard to get a fortran compiler. Is the work here to get a gfortran compiler unecessary?

@martin-frbg

Copy link
Copy Markdown

Not unnecessary if you want a full-featured and up to date LAPACK - at least not in general, only for the very specific issue of getting the sve kernels compiled. Anyone without a Fortran compiler gets an f2c-mangled dose of old LAPACK roughly corresponding to 3.9.0 with backported fixes - the newer additions mostly use too many modern Fortran features to be easily translatable, so any 3.10/3.11 functions in C are likely to be incomplete or dummies.

@mattip mattip changed the title update to commit 2183dbcf to work around numpy issue 24067 update to work around numpy issue 24067 Jul 14, 2023
@martin-frbg

Copy link
Copy Markdown

looks like something in your unix build machinery does not like the now-empty patch_source function

@mattip

mattip commented Jul 16, 2023

Copy link
Copy Markdown
Collaborator Author

Build is green, lets go with this. Then NumPy and Scipy can change to use "3d31191b" rather than "v0.3.23", to work around the numpy/numpy#24067

@mattip
mattip merged commit a64e515 into MacPython:main Jul 16, 2023
@mattip
mattip deleted the issue-24067 branch December 22, 2025 06:12
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.

2 participants