`-fPIC` for static libraries?

[lgarrison]

Hi finufft team,

Would it be possible to have a CMake option to compile the static libraries with -fPIC? This is for applications like jax-finufft, where we're building a shared library that links against the static finufft/cufinufft libs (https://github.com/flatironinstitute/jax-finufft/pull/107).

Right now, I'm achieving this with the following:

set_property(TARGET finufft PROPERTY POSITION_INDEPENDENT_CODE ON)
set_property(TARGET finufft_f32 PROPERTY POSITION_INDEPENDENT_CODE ON)
set_property(TARGET finufft_f64 PROPERTY POSITION_INDEPENDENT_CODE ON)

set_property(TARGET cufinufft PROPERTY POSITION_INDEPENDENT_CODE ON)
set_property(TARGET cufinufft_objects PROPERTY POSITION_INDEPENDENT_CODE ON)
set_property(TARGET cufinufft_common_objects PROPERTY POSITION_INDEPENDENT_CODE ON)

It would be great to be able to do something instead like:

set(FINUFFT_POSITION_INDEPENDENT_CODE ON)

or

set(FINUFFT_STATIC_LINKING PIC)

Thanks!

[lgarrison]

I should add that this is for v2.3.0-rc1.

[ahbarnett]

This seems like a couple of lines in the CMakeLists , but I don't know cmake well enough to add them. I agree it would go in 2.3.0 since it doesn't affect defaults nor CI. I hope one of our cmake experts can add them, or discuss. Thanks!

[DiamonDinoia]

I foresaw that this could be a problem. I will have a look at what the best solution is.

[lu1and10]

set_property(TARGET ${target} PROPERTY POSITION_INDEPENDENT_CODE ON) was in v2.2.0, somehow recent cmake changes removed it, we could add it back for static lib build.

https://github.com/flatironinstitute/finufft/blob/51892059a4b457a99a2569ac11e9e91cd2e289e7/CMakeLists.txt#L114

[DiamonDinoia]

AFAIK in cmake the default should be off so I removed it. There is a point on enabling it, for example when installing the library as it might be linked inside shared libraries but I would rather give the user the option.

[lu1and10]

AFAIK in cmake the default should be off so I removed it. There is a point on enabling it, for example when installing the library as it might be linked inside shared libraries but I would rather give the user the option.

with makefile we always enable fPIC(https://github.com/flatironinstitute/finufft/blob/fe5ff0baad37de62e81bff8537e72e584dce1cd9/makefile#L105), did not see much difference. Are you suggesting remove that in makefile too?

[DiamonDinoia]

-fpic causes a performance penalty as some functions might not be able to be inlined. How much it changes varies: https://stackoverflow.com/questions/15861759/how-much-overhead-can-the-fpic-flag-add In general one can compile a library with -fpic and the executable without and the performance should be the same (?). Otherwise using LTO on the final application should allow to recover any lost performance. Another good practice is to declare functions static where possible or in nested namespaces, symbols that are not exported will not be negatively impacted by performance.

[lu1and10]

I don't see much difference in our perf test with -fPIC, we can check

[ahbarnett]

I also have never seen a performance difference in FINUFFT (just checked again on my ryzen2). As per discussion threads, -fPIC is needed for shared lib (fails for me without it) with gcc. So I propose not to change the makefile for 2.3.0 release. For static lib, fPIC is not needed, as I just cheked. People can hack their own makefile if they want to change.

For cmake could someone add Lehman's request (with the default OFF), something like option: set(FINUFFT_STATIC_LINKING_PIC ON) Lehman needs this for his app.

Thanks! Alex

[DiamonDinoia]

Hi @lgarrison,

Could you check that https://github.com/flatironinstitute/finufft/pull/551 works? I added a new option FINUFFT_POSITION_INDEPENDENT_CODE which seems the least invasive way of doing this having FINUFFT_STATIC_LINKING=PIC might be the way that enforces the best user behavior but I am not sure how I feel about a variable being both a boolean and a string at the same time.

Once you confirm this works, we can bring it in and tag 2.3.0.

Thanks, Marco

[lgarrison]

Yes, it works great. Thanks!

[ahbarnett]

we even made the default ON for you :)