JuliaLinearAlgebra / libblastrampoline

Using PLT trampolines to provide a BLAS and LAPACK demuxing library.
MIT License
66 stars 17 forks source link

Force setting of some `CFLAGS` and `LDFLAGS` #45

Closed giordano closed 3 years ago

giordano commented 3 years ago

Fix #44. @nalimilan could you please test this out? I think this is now doing what you want:

% LANG=C make -C src CFLAGS="" LDFLAGS="" VERBOSE=1
make: Entering directory '/home/mose/repo/libblastrampoline/src'
cc -o build/libblastrampoline.o -fPIC -DF2C_AUTODETECTION -c libblastrampoline.c
cc -o build/dl_utils.o -fPIC -DF2C_AUTODETECTION -c dl_utils.c
cc -o build/config.o -fPIC -DF2C_AUTODETECTION -c config.c
cc -o build/autodetection.o -fPIC -DF2C_AUTODETECTION -c autodetection.c
cc -o build/threading.o -fPIC -DF2C_AUTODETECTION -c threading.c
cc -o build/deepbindless_surrogates.o -fPIC -DF2C_AUTODETECTION -c deepbindless_surrogates.c
cc -o build/trampolines/trampolines_x86_64.o -fPIC -DF2C_AUTODETECTION -c trampolines/trampolines_x86_64.S
cc -o build/f2c_adapters.o -fPIC -DF2C_AUTODETECTION -c f2c_adapters.c
cc -o build/libblastrampoline.so -fPIC -DF2C_AUTODETECTION build/libblastrampoline.o build/dl_utils.o build/config.o build/autodetection.o build/threading.o build/deepbindless_surrogates.o build/trampolines/trampolines_x86_64.o build/f2c_adapters.o -shared -ldl
make: Leaving directory '/home/mose/repo/libblastrampoline/src'
giordano commented 3 years ago

Well, tests didn't like it :smile:

nalimilan commented 3 years ago

Thanks! This works here. No idea about the test failures, but I'm surprised that the C code doesn't seem to be built in the logs?

staticfloat commented 3 years ago

This breaks the tests because test/dgemm_test/Makefile sources Make.inc, and this forces LDFLAGS to include -shared, which is not what we want.

I originally created CFLAGS_add for this exact purpose; clients should provide CFLAGS_add and leave CFLAGS alone. If that's not standard, we can instead swap the names CFLAGS -> LBT_CFLAGS and CFLAGS_add -> CFLAGS.

nalimilan commented 3 years ago

AFAICT overriding CFLAGS is considered as standard (notably by distributions), so swapping variable names sounds better (and it's what Julia does for example).

staticfloat commented 3 years ago

Yeah, I think you're right. I think the CFLAGS_add thing was a habit picked up from an old version of Julia. ;)

staticfloat commented 3 years ago

Feels like we're using the wrong compiler?

nalimilan commented 3 years ago

Any news on this?

giordano commented 3 years ago

The x86 Linux job didn't compile with -m32, which explains the error, but if I log into the machine i get:

julia> include("test/utils.jl")
lbt_get_default_func (generic function with 1 method)

julia> needs_m32()
true

Great.

Edit: ok, the problem was that in the tests we were still using CFLAGS_add

giordano commented 3 years ago

Should be good to go now, tests passing on all platforms.

nalimilan commented 3 years ago

Thanks! It would be nice to backport this to Julia 1.7 to help other distro packagers.