Skip to content

[libc][math] Fix range and comments in exhaustive hypotf_test #131769

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

Merged
merged 3 commits into from
Mar 24, 2025

Conversation

meltq
Copy link
Contributor

@meltq meltq commented Mar 18, 2025

((23U + 127U) << 23) + 1 evaluates to (2^23)+1 as opposed to 2^24, so should instead be (24U + 127U) << 23. Additionally, range for both inputs is inclusive of STOP. The comments have been updated reflecting these changes.

@llvmbot llvmbot added the libc label Mar 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 18, 2025

@llvm/pr-subscribers-libc

Author: Tejas Vipin (meltq)

Changes

((23U + 127U) &lt;&lt; 23) + 1 evaluates to (2^23)+1 as opposed to 2^24. This is also noticeable when running the test itself, which prints

[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcHypotfExhaustiveTest.PositiveRange
-- Testing for FE_TONEAREST in range [0x4b000000, 0x4b000001) --

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

1 Files Affected:

  • (modified) libc/test/src/math/exhaustive/hypotf_test.cpp (+1-1)
diff --git a/libc/test/src/math/exhaustive/hypotf_test.cpp b/libc/test/src/math/exhaustive/hypotf_test.cpp
index 04da55d4d3a9f..601ec9f749462 100644
--- a/libc/test/src/math/exhaustive/hypotf_test.cpp
+++ b/libc/test/src/math/exhaustive/hypotf_test.cpp
@@ -53,7 +53,7 @@ using LlvmLibcHypotfExhaustiveTest = LlvmLibcExhaustiveMathTest<HypotfChecker>;
 
 // Range of the first input: [2^23, 2^24);
 static constexpr uint32_t START = (23U + 127U) << 23;
-static constexpr uint32_t STOP = ((23U + 127U) << 23) + 1;
+static constexpr uint32_t STOP = (24U + 127U) << 23;
 
 TEST_F(LlvmLibcHypotfExhaustiveTest, PositiveRange) {
   test_full_range_all_roundings(START, STOP);

@meltq
Copy link
Contributor Author

meltq commented Mar 18, 2025

@overmighty @lntue Requesting review.

@meltq
Copy link
Contributor Author

meltq commented Mar 24, 2025

@overmighty @lntue ping.

@lntue
Copy link
Contributor

lntue commented Mar 24, 2025

I think I intentionally test it including next exponent. Feel free to update the comment instead.

@meltq meltq changed the title [libc][math] Fix STOP limit for x in exhaustive hypotf_test [libc][math] Fix range comments in exhaustive hypotf_test Mar 24, 2025
@meltq
Copy link
Contributor Author

meltq commented Mar 24, 2025

@lntue updated. Also updated PR title and desc to reflect this.

@meltq meltq changed the title [libc][math] Fix range comments in exhaustive hypotf_test [libc][math] Fix range and comments in exhaustive hypotf_test Mar 24, 2025
@meltq
Copy link
Contributor Author

meltq commented Mar 24, 2025

@lntue updated.

@lntue lntue merged commit 1f96788 into llvm:main Mar 24, 2025
14 of 15 checks passed
@meltq meltq deleted the hypotf_correct branch March 24, 2025 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants