Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEA] Enable building static libs #4673

Merged
merged 14 commits into from
May 10, 2022

Conversation

trxcllnt
Copy link
Collaborator

@trxcllnt trxcllnt commented Mar 30, 2022

This PR allows building libcuml++ static libs via CMake option -DBUILD_SHARED_LIBS=ON|OFF.

I was seeing a linker error due to not having -fPIC enabled, but I wouldn't have expected this to affect static libs. I'll investigate some more and turn it off if possible, but for now -fPIC is enabled.

@trxcllnt trxcllnt added 3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 30, 2022
@trxcllnt trxcllnt requested review from a team as code owners March 30, 2022 20:10
@robertmaynard
Copy link
Contributor

I was seeing a linker error due to not having -fPIC enabled, but I wouldn't have expected this to affect static libs. I'll investigate some more and turn it off if possible, but for now -fPIC is enabled.

All static code that will be embedded in to a shared library needs to be compiled with fPIC. Code that will be embedded into a binary that isn't compiled with fPIE has no such requirement ( statement presumes x86_64 arch ).

cpp/cmake/thirdparty/get_gputreeshap.cmake Outdated Show resolved Hide resolved
cpp/cmake/thirdparty/get_gputreeshap.cmake Outdated Show resolved Hide resolved
@dantegd dantegd added this to PR-WIP in v22.06 Release via automation Mar 30, 2022
@dantegd
Copy link
Member

dantegd commented Mar 31, 2022

rerun tests

v22.06 Release automation moved this from PR-WIP to PR-Reviewer approved Mar 31, 2022
@dantegd
Copy link
Member

dantegd commented Mar 31, 2022

@gpucibot merge

@dantegd dantegd added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Mar 31, 2022
@dantegd dantegd removed the 3 - Ready for Review Ready for review by team label Mar 31, 2022
@trxcllnt
Copy link
Collaborator Author

trxcllnt commented Apr 1, 2022

rerun tests

@trxcllnt
Copy link
Collaborator Author

rerun tests

@trxcllnt
Copy link
Collaborator Author

Not sure what this is about, but this seems unrelated to this PR:

FAILED: CMakeFiles/cuml++.dir/src/randomforest/randomforest.cu.o 
sccache /usr/local/cuda/bin/nvcc -forward-unknown-to-host-compiler -ccbin=/usr/bin/g++ -DCUML_CPP_API -DDISABLE_CUSPARSE_DEPRECATED -DRAFT_DISTANCE_COMPILED -DRAFT_NN_COMPILED -DTHRUST_DEVICE_SYSTEM=THRUST_DEVICE_SYSTEM_CUDA -DTHRUST_HOST_SYSTEM=THRUST_HOST_SYSTEM_CPP -Dcuml___EXPORTS -I$SRC_DIR/cpp/include -I$SRC_DIR/cpp/src -I$SRC_DIR/cpp/src/metrics -I$SRC_DIR/cpp/src_prims -I$PREFIX/include/rapids/thrust -I$PREFIX/include/rapids/thrust/dependencies -I$SRC_DIR/cpp/build/_deps/gputreeshap-src -isystem=$PREFIX/include -isystem=/usr/local/cuda/include -O3 -DNDEBUG --generate-code=arch=compute_60,code=[sm_60] --generate-code=arch=compute_70,code=[sm_70] --generate-code=arch=compute_75,code=[sm_75] --generate-code=arch=compute_80,code=[sm_80] --generate-code=arch=compute_86,code=[compute_86,sm_86] -Xcompiler=-fPIC --expt-extended-lambda --expt-relaxed-constexpr -Werror=all-warnings -Xcompiler=-Wall,-Werror,-Wno-error=deprecated-declarations,-Wno-error=sign-compare -Xcompiler=-Wno-deprecated-declarations -Xfatbin=-compress-all -Xcompiler=-fopenmp -Xcompiler -pthread -std=c++17 -MD -MT CMakeFiles/cuml++.dir/src/randomforest/randomforest.cu.o -MF CMakeFiles/cuml++.dir/src/randomforest/randomforest.cu.o.d -x cu -c $SRC_DIR/cpp/src/randomforest/randomforest.cu -o CMakeFiles/cuml++.dir/src/randomforest/randomforest.cu.o
$PREFIX/include/raft/random/detail/rng_impl.cuh(79): error: expected an identifier

@trxcllnt
Copy link
Collaborator Author

rerun tests

@trxcllnt
Copy link
Collaborator Author

trxcllnt commented May 9, 2022

rerun tests

1 similar comment
@trxcllnt
Copy link
Collaborator Author

rerun tests

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.06@3060322). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-22.06    #4673   +/-   ##
===============================================
  Coverage                ?   79.89%           
===============================================
  Files                   ?      181           
  Lines                   ?    11370           
  Branches                ?        0           
===============================================
  Hits                    ?     9084           
  Misses                  ?     2286           
  Partials                ?        0           
Flag Coverage Δ
dask 45.44% <0.00%> (?)
non-dask 69.51% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3060322...f999b78. Read the comment docs.

@rapids-bot rapids-bot bot merged commit 61524d7 into rapidsai:branch-22.06 May 10, 2022
v22.06 Release automation moved this from PR-Reviewer approved to Done May 10, 2022
wphicks added a commit to wphicks/cuml that referenced this pull request Jun 24, 2022
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
This PR allows building libcuml++ static libs via CMake option `-DBUILD_SHARED_LIBS=ON|OFF`.

~I was seeing a linker error due to not having `-fPIC` enabled, but I wouldn't have expected this to affect static libs. I'll investigate some more and turn it off if possible, but for now `-fPIC` is enabled.~

Authors:
  - Paul Taylor (https://github.com/trxcllnt)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: rapidsai#4673
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge CMake CUDA/C++ improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants