Correct GTest print check for 128-bit ints on Windows#1029
Merged
stanleytsang-amd merged 1 commit intoAug 12, 2025
Conversation
Currently, on Windows, GTest cannot print 128-bit ints. We have a check in test_utils::protected_assert_eq that avoids calling ASSERT_EQ on 128-bit int values directly, since this will cause the values to be printed in the event of an error. This check was relying on the is_int128 alias, which was being set to false_type when ROCPRIM_HAS_INT128_SUPPORT was false. As a result, when 128-bit types were passed in, our check could not detect them and would fail to stop the printing. In rocprim/types.hpp, the types rocprim::int128_t and rocprim::uint128_t are now defined regardless of how ROCPRIM_HAS_INT128_SUPPORT is set. This means we no longer need to guard against usage of these types in our test code (we only need to use ROCPRIM_HAS_INT128_SUPPORT in cases where we're doing some operation that explicitly won't work on 128-bit ints). This change removes the code that sets the is_int128 alias to false_type when ROCPRIM_HAS_INT128_SUPPORT is not set. Doing this is enough to fix the check in test_utils::protected_assert_eq.
marbre
reviewed
Aug 4, 2025
Member
marbre
left a comment
marbre
left a comment
There was a problem hiding this comment.
I can confirm that this unbreaks the Windows build in TheRock: https://github.com/ROCm/TheRock/actions/runs/16724375578/job/47336097097?pr=1185
Thanks for the fix!
Member
|
@umfranzw @stanleytsang-amd can this be merged? |
Contributor
Author
|
@marbre, apologies for the delay - some unrelated CI failures are blocking my ability to merge. I think they may have been fixed now. I will restart them again and see how it goes. |
Contributor
Author
|
/azp run rocPRIM |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
Member
|
Seems Azure CI was not able to run. I can however confirm, that with this fix, TheRock is able to build and run rocPRIM tests successfully. |
stanleytsang-amd
deleted the
users/umfranzw/fix_rocprim_int128_print_check
branch
lamikr
pushed a commit
that referenced
this pull request
Aug 19, 2025
Currently, on Windows, GTest cannot print 128-bit ints. We have a check in test_utils::protected_assert_eq that avoids calling ASSERT_EQ on 128-bit int values directly, since this will cause the values to be printed in the event of an error. This check was relying on the is_int128 alias, which was being set to false_type when ROCPRIM_HAS_INT128_SUPPORT was false. As a result, when 128-bit types were passed in, our check could not detect them and would fail to stop the printing. In rocprim/types.hpp, the types rocprim::int128_t and rocprim::uint128_t are now defined regardless of how ROCPRIM_HAS_INT128_SUPPORT is set. This means we no longer need to guard against usage of these types in our test code (we only need to use ROCPRIM_HAS_INT128_SUPPORT in cases where we're doing some operation that explicitly won't work on 128-bit ints). This change removes the code that sets the is_int128 alias to false_type when ROCPRIM_HAS_INT128_SUPPORT is not set. Doing this is enough to fix the check in test_utils::protected_assert_eq. Cherry-picked from: #1029
HPC-Ken
pushed a commit
to HPC-Ken/rocm-libraries
that referenced
this pull request
Aug 22, 2025
Currently, on Windows, GTest cannot print 128-bit ints. We have a check in test_utils::protected_assert_eq that avoids calling ASSERT_EQ on 128-bit int values directly, since this will cause the values to be printed in the event of an error. This check was relying on the is_int128 alias, which was being set to false_type when ROCPRIM_HAS_INT128_SUPPORT was false. As a result, when 128-bit types were passed in, our check could not detect them and would fail to stop the printing. In rocprim/types.hpp, the types rocprim::int128_t and rocprim::uint128_t are now defined regardless of how ROCPRIM_HAS_INT128_SUPPORT is set. This means we no longer need to guard against usage of these types in our test code (we only need to use ROCPRIM_HAS_INT128_SUPPORT in cases where we're doing some operation that explicitly won't work on 128-bit ints). This change removes the code that sets the is_int128 alias to false_type when ROCPRIM_HAS_INT128_SUPPORT is not set. Doing this is enough to fix the check in test_utils::protected_assert_eq. Cherry-picked from: ROCm#1029
marbre
added a commit
to ROCm/TheRock
that referenced
this pull request
Sep 3, 2025
This patch was needed prior to ROCm/rocm-libraries#1029. With the fixes to upstream, the local patch is no longer needed.
stanleytsang-amd
pushed a commit
that referenced
this pull request
Oct 23, 2025
Currently, on Windows, GTest cannot print 128-bit ints. We have a check in `test_utils::protected_assert_eq` that avoids calling `ASSERT_EQ` on 128-bit int values directly, since this will cause the values to be printed in the event of an error. This check was relying on the `is_int128` alias, which was being set to `false_type` when `ROCPRIM_HAS_INT128_SUPPORT` was `false`. As a result, when 128-bit types were passed in, our check could not detect them and would fail to stop the printing. In [rocprim/types.hpp](https://github.com/ROCm/rocm-libraries/blob/develop/projects/rocprim/rocprim/include/rocprim/types.hpp#L69), the types `rocprim::int128_t` and `rocprim::uint128_t` are now defined regardless of how `ROCPRIM_HAS_INT128_SUPPORT` is set. This means we no longer need to guard against usage of these types in our test code (we only need to use `ROCPRIM_HAS_INT128_SUPPORT` in cases where we're doing some operation that explicitly won't work on 128-bit ints). This change removes the code that sets the `is_int128` alias to `false_type` when `ROCPRIM_HAS_INT128_SUPPORT` is not set. Doing this is enough to fix the check in `test_utils::protected_assert_eq`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, on Windows, GTest cannot print 128-bit ints. We have a check in
test_utils::protected_assert_eqthat avoids callingASSERT_EQon 128-bit int values directly, since this will cause the values to be printed in the event of an error.This check was relying on the
is_int128alias, which was being set tofalse_typewhenROCPRIM_HAS_INT128_SUPPORTwasfalse. As a result, when 128-bit types were passed in, our check could not detect them and would fail to stop the printing.In rocprim/types.hpp, the types
rocprim::int128_tandrocprim::uint128_tare now defined regardless of howROCPRIM_HAS_INT128_SUPPORTis set. This means we no longer need to guard against usage of these types in our test code (we only need to useROCPRIM_HAS_INT128_SUPPORTin cases where we're doing some operation that explicitly won't work on 128-bit ints).This change removes the code that sets the
is_int128alias tofalse_typewhenROCPRIM_HAS_INT128_SUPPORTis not set. Doing this is enough to fix the check intest_utils::protected_assert_eq.