All kernels in the JNI should have hidden visibility #2168
Conversation
Signed-off-by: Jihoon Son <[email protected]>
Signed-off-by: Jihoon Son <[email protected]>
|
build |
| size_type num_rows, | ||
| bool ansi_mode, | ||
| bool strip) | ||
| void CUDF_KERNEL string_to_integer_kernel(T* out, |
There was a problem hiding this comment.
Should this be
void CUDF_KERNEL or CUDF_KERNEL void ?
There was a problem hiding this comment.
Validity wise it doesn't matter. Stylistically it can be argued we should be consistent. I think it's outside the scope of this change to enforce it though. This example started with void __global__ and ended with void __global__ static. This looks odd expanded out, but is valid and does the expected thing. I would prefer to see it as CUDF_KERNEL void as well.
There was a problem hiding this comment.
Oh, I was just replacing all __global__ with CUDF_KERNEL and never noticed this. I agree CUDF_KERNEL void is better and consistent. Will fix it.
There was a problem hiding this comment.
Ah just noticed it has been fixed already in #2178.
Fixes #1734.
This PR addresses a similar issue to rapidsai/cudf#14726. Instead of adding a new macro as suggested in #1734, it uses the one in libcudf added in rapidsai/cudf#14726 to avoid duplicate macros.