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

[libc++] Fix one case in saturate_cast.pass.cpp for 64-bit on z/OS #86724

Merged
merged 2 commits into from
Mar 27, 2024

Conversation

zibi2
Copy link
Contributor

@zibi2 zibi2 commented Mar 26, 2024

On z/OS int128 is disabled causing one of the cases in saturate_cast.pass.cpp to fail. The failure is only in 64-bit mode.
In this case the std::numeric_limits<long long int>::max() is within std::numeric_limits<unsigned long int>::min()
and std::numeric_limits<unsigned long int>::max() therefore, saturate_cast( sBigMax) == LONG_MAX and not ULONG_MAX as original test.

In 32-bit, saturate_cast<unsigned long int>( sBigMax) == ULONG_MAX like on other platforms where int128 is enabled.

This PR is required to pass this test case on z/OS and possibly on other platforms where int128 is not supported/enabled.

Co-authored-by: Sean Perry <perry@ca.ibm.com>
@zibi2 zibi2 added the z/OS label Mar 26, 2024
@zibi2 zibi2 self-assigned this Mar 26, 2024
@zibi2 zibi2 requested a review from a team as a code owner March 26, 2024 19:55
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 26, 2024

@llvm/pr-subscribers-libcxx

Author: None (zibi2)

Changes

On z/OS int128 is disabled causing one of the cases in saturate_cast.pass.cpp to fail. The failure is only in 64-bit mode.
In this case the std::numeric_limits&lt;long long int&gt;::max() is within std::numeric_limits&lt;unsigned long int&gt;::min()
and std::numeric_limits&lt;unsigned long int&gt;::max() therefore, saturate_cast<unsigned long int>( sBigMax) == LONG_MAX and not ULONG_MAX as original test.

In 32-bit, saturate_cast&lt;unsigned long int&gt;( sBigMax) == ULONG_MAX like on other platforms where int128 is enabled.

This PR is required to pass this test case on z/OS and possibly on other platforms where int128 is not supported/enabled.


Full diff: https://github.com/llvm/llvm-project/pull/86724.diff

1 Files Affected:

  • (modified) libcxx/test/std/numerics/numeric.ops/numeric.ops.sat/saturate_cast.pass.cpp (+1-1)
diff --git a/libcxx/test/std/numerics/numeric.ops/numeric.ops.sat/saturate_cast.pass.cpp b/libcxx/test/std/numerics/numeric.ops/numeric.ops.sat/saturate_cast.pass.cpp
index c06a9ed2d5cb42..59f397d5c0ee76 100644
--- a/libcxx/test/std/numerics/numeric.ops/numeric.ops.sat/saturate_cast.pass.cpp
+++ b/libcxx/test/std/numerics/numeric.ops/numeric.ops.sat/saturate_cast.pass.cpp
@@ -329,7 +329,7 @@ constexpr bool test() {
   { [[maybe_unused]] std::same_as<unsigned long int> decltype(auto) _ = std::saturate_cast<unsigned long int>(sBigMax); }
   assert(std::saturate_cast<unsigned long int>(  sBigMin) == 0UL);       // saturated
   assert(std::saturate_cast<unsigned long int>(    sZero) == 0UL);
-  assert(std::saturate_cast<unsigned long int>(  sBigMax) == ULONG_MAX); // saturated
+  assert(std::saturate_cast<unsigned long int>(  sBigMax) == (sizeof(UIntT)>sizeof(unsigned long int)?ULONG_MAX:LONG_MAX)); // saturated depending on underlying types
 
   { [[maybe_unused]] std::same_as<unsigned long int> decltype(auto) _ = std::saturate_cast<unsigned long int>(uBigMax); }
   assert(std::saturate_cast<unsigned long int>(    uZero) == 0UL);

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a formatting nitpick.

@zibi2 zibi2 changed the title Fix one case in saturate_cast.pass.cpp for 64-bit on z/OS [libc++] Fix one case in saturate_cast.pass.cpp for 64-bit on z/OS Mar 27, 2024
@zibi2 zibi2 merged commit 6e6d266 into llvm:main Mar 27, 2024
51 checks passed
@zibi2 zibi2 deleted the z_saturated_cast branch May 13, 2024 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. z/OS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants