Skip to content

[MathExtras] Favor using the hexadecimal FP constants #123180

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 1 commit into from
Jan 26, 2025

Conversation

jayfoad
Copy link
Contributor

@jayfoad jayfoad commented Jan 16, 2025

This just fixes a TODO now that we are using C++17.

This just fixes a TODO now that we are using C++17.
@llvmbot
Copy link
Member

llvmbot commented Jan 16, 2025

@llvm/pr-subscribers-llvm-support

Author: Jay Foad (jayfoad)

Changes

This just fixes a TODO now that we are using C++17.


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

1 Files Affected:

  • (modified) llvm/include/llvm/Support/MathExtras.h (+30-31)
diff --git a/llvm/include/llvm/Support/MathExtras.h b/llvm/include/llvm/Support/MathExtras.h
index 574e9a61166037..5a6f51adc07f39 100644
--- a/llvm/include/llvm/Support/MathExtras.h
+++ b/llvm/include/llvm/Support/MathExtras.h
@@ -43,38 +43,37 @@ using common_sint =
 /// Mathematical constants.
 namespace numbers {
 // TODO: Track C++20 std::numbers.
-// TODO: Favor using the hexadecimal FP constants (requires C++17).
 // clang-format off
-constexpr double e          = 2.7182818284590452354, // (0x1.5bf0a8b145769P+1) https://oeis.org/A001113
-                 egamma     = .57721566490153286061, // (0x1.2788cfc6fb619P-1) https://oeis.org/A001620
-                 ln2        = .69314718055994530942, // (0x1.62e42fefa39efP-1) https://oeis.org/A002162
-                 ln10       = 2.3025850929940456840, // (0x1.26bb1bbb55516P+1) https://oeis.org/A002392
-                 log2e      = 1.4426950408889634074, // (0x1.71547652b82feP+0)
-                 log10e     = .43429448190325182765, // (0x1.bcb7b1526e50eP-2)
-                 pi         = 3.1415926535897932385, // (0x1.921fb54442d18P+1) https://oeis.org/A000796
-                 inv_pi     = .31830988618379067154, // (0x1.45f306dc9c883P-2) https://oeis.org/A049541
-                 sqrtpi     = 1.7724538509055160273, // (0x1.c5bf891b4ef6bP+0) https://oeis.org/A002161
-                 inv_sqrtpi = .56418958354775628695, // (0x1.20dd750429b6dP-1) https://oeis.org/A087197
-                 sqrt2      = 1.4142135623730950488, // (0x1.6a09e667f3bcdP+0) https://oeis.org/A00219
-                 inv_sqrt2  = .70710678118654752440, // (0x1.6a09e667f3bcdP-1)
-                 sqrt3      = 1.7320508075688772935, // (0x1.bb67ae8584caaP+0) https://oeis.org/A002194
-                 inv_sqrt3  = .57735026918962576451, // (0x1.279a74590331cP-1)
-                 phi        = 1.6180339887498948482; // (0x1.9e3779b97f4a8P+0) https://oeis.org/A001622
-constexpr float ef          = 2.71828183F, // (0x1.5bf0a8P+1) https://oeis.org/A001113
-                egammaf     = .577215665F, // (0x1.2788d0P-1) https://oeis.org/A001620
-                ln2f        = .693147181F, // (0x1.62e430P-1) https://oeis.org/A002162
-                ln10f       = 2.30258509F, // (0x1.26bb1cP+1) https://oeis.org/A002392
-                log2ef      = 1.44269504F, // (0x1.715476P+0)
-                log10ef     = .434294482F, // (0x1.bcb7b2P-2)
-                pif         = 3.14159265F, // (0x1.921fb6P+1) https://oeis.org/A000796
-                inv_pif     = .318309886F, // (0x1.45f306P-2) https://oeis.org/A049541
-                sqrtpif     = 1.77245385F, // (0x1.c5bf8aP+0) https://oeis.org/A002161
-                inv_sqrtpif = .564189584F, // (0x1.20dd76P-1) https://oeis.org/A087197
-                sqrt2f      = 1.41421356F, // (0x1.6a09e6P+0) https://oeis.org/A002193
-                inv_sqrt2f  = .707106781F, // (0x1.6a09e6P-1)
-                sqrt3f      = 1.73205081F, // (0x1.bb67aeP+0) https://oeis.org/A002194
-                inv_sqrt3f  = .577350269F, // (0x1.279a74P-1)
-                phif        = 1.61803399F; // (0x1.9e377aP+0) https://oeis.org/A001622
+constexpr double e          = 0x1.5bf0a8b145769P+1, // (2.7182818284590452354) https://oeis.org/A001113
+                 egamma     = 0x1.2788cfc6fb619P-1, // (.57721566490153286061) https://oeis.org/A001620
+                 ln2        = 0x1.62e42fefa39efP-1, // (.69314718055994530942) https://oeis.org/A002162
+                 ln10       = 0x1.26bb1bbb55516P+1, // (2.3025850929940456840) https://oeis.org/A002392
+                 log2e      = 0x1.71547652b82feP+0, // (1.4426950408889634074)
+                 log10e     = 0x1.bcb7b1526e50eP-2, // (.43429448190325182765)
+                 pi         = 0x1.921fb54442d18P+1, // (3.1415926535897932385) https://oeis.org/A000796
+                 inv_pi     = 0x1.45f306dc9c883P-2, // (.31830988618379067154) https://oeis.org/A049541
+                 sqrtpi     = 0x1.c5bf891b4ef6bP+0, // (1.7724538509055160273) https://oeis.org/A002161
+                 inv_sqrtpi = 0x1.20dd750429b6dP-1, // (.56418958354775628695) https://oeis.org/A087197
+                 sqrt2      = 0x1.6a09e667f3bcdP+0, // (1.4142135623730950488) https://oeis.org/A00219
+                 inv_sqrt2  = 0x1.6a09e667f3bcdP-1, // (.70710678118654752440)
+                 sqrt3      = 0x1.bb67ae8584caaP+0, // (1.7320508075688772935) https://oeis.org/A002194
+                 inv_sqrt3  = 0x1.279a74590331cP-1, // (.57735026918962576451)
+                 phi        = 0x1.9e3779b97f4a8P+0; // (1.6180339887498948482) https://oeis.org/A001622
+constexpr float ef          = 0x1.5bf0a8P+1F, // (2.71828183) https://oeis.org/A001113
+                egammaf     = 0x1.2788d0P-1F, // (.577215665) https://oeis.org/A001620
+                ln2f        = 0x1.62e430P-1F, // (.693147181) https://oeis.org/A002162
+                ln10f       = 0x1.26bb1cP+1F, // (2.30258509) https://oeis.org/A002392
+                log2ef      = 0x1.715476P+0F, // (1.44269504)
+                log10ef     = 0x1.bcb7b2P-2F, // (.434294482)
+                pif         = 0x1.921fb6P+1F, // (3.14159265) https://oeis.org/A000796
+                inv_pif     = 0x1.45f306P-2F, // (.318309886) https://oeis.org/A049541
+                sqrtpif     = 0x1.c5bf8aP+0F, // (1.77245385) https://oeis.org/A002161
+                inv_sqrtpif = 0x1.20dd76P-1F, // (.564189584) https://oeis.org/A087197
+                sqrt2f      = 0x1.6a09e6P+0F, // (1.41421356) https://oeis.org/A002193
+                inv_sqrt2f  = 0x1.6a09e6P-1F, // (.707106781)
+                sqrt3f      = 0x1.bb67aeP+0F, // (1.73205081) https://oeis.org/A002194
+                inv_sqrt3f  = 0x1.279a74P-1F, // (.577350269)
+                phif        = 0x1.9e377aP+0F; // (1.61803399) https://oeis.org/A001622
 // clang-format on
 } // namespace numbers
 

@ebahapo
Copy link
Contributor

ebahapo commented Jan 25, 2025

LGTM. Thank you.

@jayfoad jayfoad merged commit 0c78485 into llvm:main Jan 26, 2025
10 checks passed
@jayfoad jayfoad deleted the mathextras-hex branch January 26, 2025 15:32
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 26, 2025

LLVM Buildbot has detected a new failure on builder flang-aarch64-dylib running on linaro-flang-aarch64-dylib while building llvm at step 5 "build-unified-tree".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/50/builds/9425

Here is the relevant piece of the build log for the reference
Step 5 (build-unified-tree) failure: build (failure)
...
2725.093 [1937/3/4882] Building CXX object tools/mlir/lib/Analysis/CMakeFiles/obj.MLIRAnalysis.dir/DataFlow/IntegerRangeAnalysis.cpp.o
2725.138 [1934/5/4883] Building CXX object tools/mlir/lib/Analysis/CMakeFiles/obj.MLIRAnalysis.dir/DataFlow/DenseAnalysis.cpp.o
2725.822 [1934/4/4884] Building CXX object tools/mlir/lib/Conversion/TosaToArith/CMakeFiles/obj.MLIRTosaToArith.dir/TosaToArithPass.cpp.o
2725.835 [1934/3/4885] Building CXX object tools/mlir/lib/CAPI/Dialect/CMakeFiles/obj.MLIRCAPIControlFlow.dir/ControlFlow.cpp.o
2726.112 [1930/6/4886] Linking CXX static library lib/libMLIRAnalysis.a
2726.149 [1930/5/4887] Building CXX object tools/mlir/lib/Conversion/TosaToLinalg/CMakeFiles/obj.MLIRTosaToLinalg.dir/TosaToLinalgPass.cpp.o
2726.797 [1930/4/4888] Building CXX object tools/mlir/lib/Conversion/AMDGPUToROCDL/CMakeFiles/obj.MLIRAMDGPUToROCDL.dir/AMDGPUToROCDL.cpp.o
2727.325 [1930/3/4889] Building CXX object tools/mlir/lib/Conversion/ArithCommon/CMakeFiles/obj.MLIRArithAttrToLLVMConversion.dir/AttrToLLVMConverter.cpp.o
2727.381 [1930/2/4890] Building CXX object tools/mlir/lib/Conversion/AffineToStandard/CMakeFiles/obj.MLIRAffineToStandard.dir/AffineToStandard.cpp.o
2728.058 [1922/9/4891] Building CXX object tools/mlir/test/lib/IR/CMakeFiles/MLIRTestIR.dir/TestVisitorsGeneric.cpp.o
FAILED: tools/mlir/test/lib/IR/CMakeFiles/MLIRTestIR.dir/TestVisitorsGeneric.cpp.o 
/usr/local/bin/c++ -DGTEST_HAS_RTTI=0 -DMLIR_INCLUDE_TESTS -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/build/tools/mlir/test/lib/IR -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/IR -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/build/tools/mlir/include -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/include -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/build/include -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/llvm/include -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/IR/../Dialect/Test -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/build/tools/mlir/test/lib/IR/../Dialect/Test -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Wundef -Werror=mismatched-tags -O3 -DNDEBUG -std=c++17  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT tools/mlir/test/lib/IR/CMakeFiles/MLIRTestIR.dir/TestVisitorsGeneric.cpp.o -MF tools/mlir/test/lib/IR/CMakeFiles/MLIRTestIR.dir/TestVisitorsGeneric.cpp.o.d -o tools/mlir/test/lib/IR/CMakeFiles/MLIRTestIR.dir/TestVisitorsGeneric.cpp.o -c /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/IR/TestVisitorsGeneric.cpp
In file included from /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/IR/TestVisitorsGeneric.cpp:9:
/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/IR/../Dialect/Test/TestOps.h:148:10: fatal error: 'TestOps.h.inc' file not found
  148 | #include "TestOps.h.inc"
      |          ^~~~~~~~~~~~~~~
1 error generated.
2728.330 [1922/8/4892] Building CXX object tools/mlir/lib/Conversion/MathToSPIRV/CMakeFiles/obj.MLIRMathToSPIRV.dir/MathToSPIRVPass.cpp.o
2728.338 [1922/7/4893] Building CXX object tools/mlir/lib/Conversion/ArithToEmitC/CMakeFiles/obj.MLIRArithToEmitC.dir/ArithToEmitCPass.cpp.o
2728.709 [1922/6/4894] Building CXX object tools/mlir/lib/Conversion/ArmNeon2dToIntr/CMakeFiles/obj.MLIRArmNeon2dToIntr.dir/ArmNeon2dToIntr.cpp.o
2728.766 [1922/5/4895] Building CXX object tools/mlir/lib/Conversion/ComplexToLLVM/CMakeFiles/obj.MLIRComplexToLLVM.dir/ComplexToLLVM.cpp.o
2728.821 [1922/4/4896] Building CXX object tools/mlir/lib/Conversion/ArithToLLVM/CMakeFiles/obj.MLIRArithToLLVM.dir/ArithToLLVM.cpp.o
2728.875 [1922/3/4897] Building CXX object tools/mlir/lib/Conversion/TosaToTensor/CMakeFiles/obj.MLIRTosaToTensor.dir/TosaToTensor.cpp.o
2728.909 [1922/2/4898] Building CXX object tools/mlir/lib/Conversion/ArithToArmSME/CMakeFiles/obj.MLIRArithToArmSME.dir/ArithToArmSME.cpp.o
2728.938 [1922/1/4899] Building CXX object tools/mlir/lib/Conversion/MathToROCDL/CMakeFiles/obj.MLIRMathToROCDL.dir/MathToROCDL.cpp.o
ninja: build stopped: subcommand failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants