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] Test more edge cases in bitmask operations #13490

Open
vyasr opened this issue Jun 1, 2023 · 1 comment
Open

[FEA] Test more edge cases in bitmask operations #13490

vyasr opened this issue Jun 1, 2023 · 1 comment
Labels
0 - Backlog In queue waiting for assignment feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. tests Unit testing for project

Comments

@vyasr
Copy link
Contributor

vyasr commented Jun 1, 2023

Is your feature request related to a problem? Please describe.
libcudf has multiple functions (both internal and external) for manipulating bitmasks. These functions often make use of bitwise operations and CUDA intrinsics in kernels, which may have sharp edges around when bitmasks contain uninitialized bits or in other ways. We recently uncovered one in #13479, and it was only uncovered via intermittent failures from dask_cudf (Python) tests.

Describe the solution you'd like
We should consider adding more tests that directly play with bitmasks, perhaps bitmasks containing invalid data in certain ways, to better understand the potential failure modes. In some cases these may be test cases that we do not need to support since libcudf generally follows a "garbage in, garbage out" approach, but in some cases the "garbage in" may be generated by libcudf itself and therefore something that we need to support.

I will update this issue with specific tests cases that may prove relevant:

  • A bitmask with enough bits that we have > 1 CUDA block of data, and potentially with junk bits either at the beginning or the end.
@vyasr vyasr added feature request New feature or request Needs Triage Need team to review and classify labels Jun 1, 2023
@wence-
Copy link
Contributor

wence- commented Jun 5, 2023

Some (slightly unstructured) thoughts here.

  • The fix for [BUG] Intermittent failures in groupby cumulative scans when keys contain nulls. #13479 (Fix valid count computation in offset_bitmask_binop kernel #13489) was actually a race condition (so only indirectly uninitialised bits). Unfortunately, it wasn't spotted by compute-sanitizer in synccheck mode, since, AFAICT synccheck only detects hazards on shared memory conflicts, whereas the relevant kernel in this case had a RAW-conflict in global memory.

  • Bitmasks allocations are always a multiple of 64 bytes (512 bits), and any words not contributing to the actual bitmask are left uninitialised. This has a twofold consequence, memcheck will not catch off-by-one errors (for most kernels) since the memory is allocated; initcheck complains on most memcpy-like operations because the source array for a bitmask is almost-always partially uninitialised. There is good reason not to fully initialise the extra bits by making an additional kernel call to memzero the relevant parts every time a bitmask is manipulated. However, it might be worthwhile adapting all of the bitmask-manipulation kernels to explicitly zero out the trailing allocation (that doesn't correspond to an actual bitmask). That way we might be able to spot actual bugs due to initcheck errors. But see discussion in [BUG] cudf::strings::from_integers does not appear to be compute-sanitizer --tool initcheck clean #12667.

@GregoryKimball GregoryKimball added 0 - Backlog In queue waiting for assignment libcudf Affects libcudf (C++/CUDA) code. tests Unit testing for project and removed Needs Triage Need team to review and classify labels Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 - Backlog In queue waiting for assignment feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. tests Unit testing for project
Projects
None yet
Development

No branches or pull requests

3 participants