flatironinstitute / finufft

Non-uniform fast Fourier transform library of types 1,2,3 in dimensions 1,2,3
Other
303 stars 79 forks source link

add fpic option for static lib #550

Closed lu1and10 closed 2 months ago

lu1and10 commented 2 months ago

per request issue #544, adding fPIC option for static lib build.

ahbarnett commented 2 months ago

@lgarrison does this work for your #544 ?

ahbarnett commented 2 months ago

SOunds good to me.

Remember, all Lehman was asking for was a one-line cmake option he can use to make all static libs use -fPIC.

Please go ahead with the adjustment.

On Thu, Sep 5, 2024 at 12:38 PM Marco Barbone @.***> wrote:

@.**** requested changes on this pull request.

In CMakeLists.txt https://github.com/flatironinstitute/finufft/pull/550#discussion_r1745859170 :

@@ -242,6 +243,9 @@ function(set_finufft_options target) endif() target_link_libraries(${target} PRIVATE xsimd) target_link_libraries(${target} PRIVATE ${FINUFFT_FFTLIBS})

  • if(FINUFFT_STATIC_LINKING AND FINUFFT_STATIC_LINKING_PIC)
  • set_property(TARGET ${target} PROPERTY POSITION_INDEPENDENT_CODE ON)

Close but this might break with fftw, ducc, dependencies should also be -fPIC :)

So we should use set_property(TARGET ${target} PROPERTY POSITION_INDEPENDENT_CODE FINUFFT_STATIC_LINKING_PIC) for all targets and this variable should be set to on when we do shared linking

— Reply to this email directly, view it on GitHub https://github.com/flatironinstitute/finufft/pull/550#pullrequestreview-2283585935, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNZRSR7XJMPEFRBK63XAZTZVCCFXAVCNFSM6AAAAABNW2GQXCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEOBTGU4DKOJTGU . You are receiving this because you commented.Message ID: @.***>

-- *-------------------------------------------------------------------~^`^~._.~' |\ Alex Barnett Center for Computational Mathematics, Flatiron Institute | \ http://users.flatironinstitute.org/~ahb 646-876-5942

lgarrison commented 2 months ago

Yes, any syntax like this would work for me! Does this setting propagate to the cufinufft libs, too? (And any dependent libs, as @DiamonDinoia mentioned.) It looks like it might just apply to the finufft libs.

DiamonDinoia commented 2 months ago

Since #551 supersedes it I am closing this. In case we need it, we can reopen.