WIP: patch OpenBLAS build for quadmath symbols #85
Conversation
|
@matthew-brett I am going to push forward with this to solve numpy/numpy#27080. How can I make sure no quadmath symbols are used in the static compilation of libscipy-openblas.dll? |
|
@carlkl - this was your patch - any thoughts on - for example - debugging or upstreaming to OpenBLAS? |
|
This patch should be reworked or better replaced by a different PR. In a static compilation of libscipy-openblas.dll all references to libquadmath symbols should disappear. This behaviour can be proved by linking with a linking map. I've done this a while ago. I'm using now a different patchset for Openblas for binary size reduction. I can take a look at this the next days I there is interest. |
|
The changes in the patch still are valid, although the line numbers have changed a bit. I tried it and the library builds, so it would be nice to put this in for windows, especially in light of the different licensing of quadmath (LGPL) which makes the static linking on windows problematic, see numpy/numpy#27080 |
Apply Carl's patch to OpenBLAS export build of library, to remove libquatmath symbol. See: MacPython#82 (comment)
mattip
force-pushed
the
drop-quadmath-symbol
branch
from
10a6e3c
to
e7d27fb
Compare
|
I rebased off main and updated the line numbers in the patch. |
|
The dynamic link command is failing since |
|
This patch was supplied myself several years ago, either as part of an issue or privately per EMail. (I can't remember). Regarding libquadmath symbols: some time ago I investigated whether libquadmath symbols are actually used in a statically linked OpenBLAS binary. This is not the case, as can be seen by examining the linker map, at least if However, I won't have time before the weekend to go into further details. |
|
You may tryout something like this: # On Windows, we only generate a DLL without a version suffix. This is because
# applications which link against the dynamic library reference a fixed DLL name
# in their import table. By instead using a stable name it is possible to
# upgrade between library versions, without needing to re-link an application.
# For more details see: https://github.com/xianyi/OpenBLAS/issues/127.
ifeq ($(DEBUG), 1)
../$(LIBDLLNAME) : ../$(LIBNAME) $(LIBPREFIX).def dllinit.$(SUFFIX)
$(RANLIB) ../$(LIBNAME)
$(CC) $(CFLAGS) $(LDFLAGS) $(LIBPREFIX).def dllinit.$(SUFFIX) \
-shared -static -static-libgfortran -o ../$(LIBDLLNAME) -Wl,--out-implib,../$(IMPLIBNAME) \
-Wl,--whole-archive ../$(LIBNAME) -Wl,--no-whole-archive $(FEXTRALIB) $(EXTRALIB)
else
../$(LIBDLLNAME) : ../$(LIBNAME) $(LIBPREFIX).def dllinit.$(SUFFIX)
$(RANLIB) ../$(LIBNAME)
$(CC) $(CFLAGS) $(LDFLAGS) $(LIBPREFIX).def dllinit.$(SUFFIX) \
-shared -static -static-libgfortran -o ../$(LIBDLLNAME) -Wl,--out-implib,../$(IMPLIBNAME) \
-Wl,-gc-sections -Wl,-s \
-Wl,--whole-archive ../$(LIBNAME) -Wl,--no-whole-archive $(FEXTRALIB) $(EXTRALIB)
endifSomething was not working with the DEBUG case as far as I remember, so you can also simply try: # On Windows, we only generate a DLL without a version suffix. This is because
# applications which link against the dynamic library reference a fixed DLL name
# in their import table. By instead using a stable name it is possible to
# upgrade between library versions, without needing to re-link an application.
# For more details see: https://github.com/xianyi/OpenBLAS/issues/127.
../$(LIBDLLNAME) : ../$(LIBNAME) $(LIBPREFIX).def dllinit.$(SUFFIX)
$(RANLIB) ../$(LIBNAME)
$(CC) $(CFLAGS) $(LDFLAGS) $(LIBPREFIX).def dllinit.$(SUFFIX) \
-shared -static -static-libgfortran -o ../$(LIBDLLNAME) -Wl,--out-implib,../$(IMPLIBNAME) \
-Wl,-gc-sections -Wl,-s \
-Wl,--whole-archive ../$(LIBNAME) -Wl,--no-whole-archive $(FEXTRALIB) $(EXTRALIB)Another hint: some years ago creation the creation of the |
mattip
force-pushed
the
drop-quadmath-symbol
branch
from
38a3b31
to
2df815f
Compare
|
@mattip, @matthew-brett |
|
Sorry, I am not sure I understand. What is the difference between the latest suggestion and the patch? |
|
The replacement of the |
|
Are you sure? When I changed Currently the patch includes |
|
at least it doesn't harm. In some years one may ask why this was necessary... During the linking stage libquadmath is necessary to resolve |
|
Thanks, that helps clear up what is going on.
Cool. I will see if I can make this part of the build script. |
mattip
force-pushed
the
drop-quadmath-symbol
branch
2 times, most recently
from
6b8258c
to
849965d
Compare
mattip
force-pushed
the
drop-quadmath-symbol
branch
from
849965d
to
3aefe48
Compare
|
I would like to merge this and get scipy-openblas wheels built for consideration in the upcoming NumPy 2.1 release, to mitigate issue numpy/numpy#27080. Any objections? |
|
Do we currently trigger uses of the relevant |
No idea. Any thoughts how to check it? |
|
Maybe via a LD_PRELOAD stub shared object on linux that exports quadmath_snprintf (since on linux we dynamically link)? |
|
Some years ago I replaced The symbol |
|
Would we be better placed to make a stub |
|
I am not so concerned that the slight differences between |
Is that code still around? |
|
This is the full list of all symbols libquadmath.dll (gcc-ucrt 64bit) depends on: list of libquadmath.dll dependencies |
|
Sorry to be confusing - my worry was rather that we somehow generate a segfault or similar with this relinking, triggered when some BLAS / LAPACK routine ends up calling |
|
Ahh. There is some test code for quadmath. We could
But I think your analysis when you replaced |
|
Thanks for the analysis. My suggestion before was to write some stub that would catch that very rare case and give some informative error, just in case. |
|
A stub with warning is a good idea IMHO. |
|
OK, let's see how hard it is to add a stub that prints a warning to stderr and calls |
|
Hey all. Unless the stub is like <= 1hr of work and can be finished today, I would strongly suggest that it's way more important to get new wheels up than mitigate some very low-probability event with a better error message. NumPy 2.1.x just branched 2.5 hours ago and if we miss the boat there we both have the potential license concern being unresolved and may cause an extra numpy RC to be needed, delaying the rollout for Python 3.13 support. |
|
Another analysis: if you link OpenBLAS non-static, you will get te following additional dependencies: the reason you need libquadmath.dll relies on the fact that libgfortran-5.dll depends on libquadmath. |
|
AFAIK mingw64 has no (void)0or with the help of asm something like that: .text
.global quadmath_snprintf
quadmath_snprintf:
ret |
|
Sorry, I don't follow. Are you suggesting we replace |
|
I'm sure, that I found another way for a detailed analysis: Using i.e. if you use i.e. but with My conclusion is: replace `quadmath_snprintf`` with a no-op operation. |
If I understand correctly, then it really doesn't matter what replaces |
Yes, correctly. The result is the same. |
|
Closing, this was merged in the v0.3.28 builds in #178 and backported to the v0.3.27 builds, thanks to @carlkl and @matthew-brett for the original patches and for mentoring the progress. |
|
It may be interesting to see how numpy/numpy-user-dtypes#98 progresses, and if it makes us rethink using |
Apply Carl's patch to OpenBLAS export build of library, to remove
libquatmath symbol.
See:
#82 (comment)
Disabled for 32-bit (symbol missing).
We should try and apply this for Unix builds.