[REVIEW] Enforce default visibility for get_map.
#833
Conversation
|
Please help with the label, I don't have permission to edit the label on GitHub. Other CI errors should be unrelated. |
|
@trivialfis we are one day from code freeze for 21.08 so please retarget to 21.10. Thanks! |
|
Got it, will reopen the PR. |
| @@ -95,7 +96,8 @@ inline std::mutex& map_lock() | |||
| return map_lock; | |||
| } | |||
|
|
|||
| inline auto& get_map() | |||
| // Must have default visibility, see: https://github.com/rapidsai/rmm/issues/826 | |||
| RMM_EXPORT inline auto& get_map() | |||
There was a problem hiding this comment.
Don't you need this on all functions in this file?
There was a problem hiding this comment.
I tested it with XGBoost and it works fine. I'm not entirely sure what's happening exactly during runtime linking. I guess as long as this function is not inlined and the linker can find the right symbol it should be fine?
There was a problem hiding this comment.
This is the only one that matters as the function local static is the only symbol that need be shared across DLLs.
|
I moved the PR to #834 as requested. |
|
You don't need a new PR. You could have just clicked "edit" next to the title and selected 21.10. Then merge 21.10 into your branch. |
|
Let me try that. |
Good to learn something new every day. ;-) |
jrhemstad
added
improvement
|
rerun tests |
|
rerun tests |
|
@gpucibot merge |
Define macros used for specifying symbol visibility and use it on
get_map. The macros are only useful for gcc/clang with glibc on Linux. Sorry I'm not familiar with other platforms.Close #826 .
I'm not sure how to test this on CI.