Skip to content

Re-land: [Analysis] Ensure use of strict fp exceptions in ConstantFolding #137652

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
Apr 29, 2025

Conversation

pratlucas
Copy link
Contributor

To perform constant folding in math operations, the implementation of
the ConstantFolding Analysis relies on the use of the math functions
from the host's libm. In particular, it relies on checking the value of
errno and IEEE exceptions to determine when an operation is safe to be
constant-folded.

On some platforms, such as BSD or Darwin, math library functions don't
set errno, so the ConstantFolding check depends only on the value of
IEEE exceptions. As the FP exception behaviour is set to ignore by
default, the compiler can perform optimisations that would get in the
way of such checks being performed correctly.

This patch sets the FP exception behaviour to strict when compiling
the ConstantFolding.cpp source file, ensuring the value of IEEE
exceptions can be reliably used by its implementation.

This re-lands the changes from #136139, but using the -ftrapping-math
compile option instead of -ffp-exception-behavior for GCC support.

…ding

To perform constant folding in math operations, the implementation of
the ConstantFolding Analysis relies on the use of the math functions
from the host's libm. In particular, it relies on checking the value of
errno and IEEE exceptions to determine when an operation is safe to be
constant-folded.

On some platforms, such as BSD or Darwin, math library functions don't
set errno, so the ConstantFolding check depends only on the value of
IEEE exceptions. As the FP exception behaviour is set to `ignore` by
default, the compiler can perform optimisations that would get in the
way of such checks being performed correctly.

This patch sets the FP exception behaviour to `strict` when compiling
the `ConstantFolding.cpp` source file, ensuring the value of IEEE
exceptions can be reliably used by its implementation.

This re-lands the changes from llvm#136139, but using the `-ftrapping-math`
compile option instead of `-ffp-exception-behavior` for GCC support.
@llvmbot
Copy link
Member

llvmbot commented Apr 28, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Lucas Duarte Prates (pratlucas)

Changes

To perform constant folding in math operations, the implementation of
the ConstantFolding Analysis relies on the use of the math functions
from the host's libm. In particular, it relies on checking the value of
errno and IEEE exceptions to determine when an operation is safe to be
constant-folded.

On some platforms, such as BSD or Darwin, math library functions don't
set errno, so the ConstantFolding check depends only on the value of
IEEE exceptions. As the FP exception behaviour is set to ignore by
default, the compiler can perform optimisations that would get in the
way of such checks being performed correctly.

This patch sets the FP exception behaviour to strict when compiling
the ConstantFolding.cpp source file, ensuring the value of IEEE
exceptions can be reliably used by its implementation.

This re-lands the changes from #136139, but using the -ftrapping-math
compile option instead of -ffp-exception-behavior for GCC support.


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

1 Files Affected:

  • (modified) llvm/lib/Analysis/CMakeLists.txt (+11)
diff --git a/llvm/lib/Analysis/CMakeLists.txt b/llvm/lib/Analysis/CMakeLists.txt
index fbf3b587d6bd2..a17a75e6fbcac 100644
--- a/llvm/lib/Analysis/CMakeLists.txt
+++ b/llvm/lib/Analysis/CMakeLists.txt
@@ -23,6 +23,17 @@ if (DEFINED LLVM_HAVE_TF_AOT OR LLVM_HAVE_TFLITE)
   endif()
 endif()
 
+# The implementation of ConstantFolding.cpp relies on the use of math functions
+# from the host. In particular, it relies on the detection of floating point
+# exceptions originating from such math functions to prevent invalid cases
+# from being constant folded. Therefore, we must ensure that fp exceptions are
+# handled correctly.
+if (MSVC)
+  set_source_files_properties(ConstantFolding.cpp PROPERTIES COMPILE_OPTIONS "/fp:except")
+elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
+  set_source_files_properties(ConstantFolding.cpp PROPERTIES COMPILE_OPTIONS "-ftrapping-math")
+endif()
+
 add_llvm_component_library(LLVMAnalysis
   AliasAnalysis.cpp
   AliasAnalysisEvaluator.cpp

Copy link
Contributor

@antoniofrighetto antoniofrighetto left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@pratlucas pratlucas merged commit 67783eb into llvm:main Apr 29, 2025
10 of 13 checks passed
@pratlucas pratlucas deleted the reland-constant-folding-fp branch April 29, 2025 14:24
@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 29, 2025

LLVM Buildbot has detected a new failure on builder sanitizer-x86_64-linux-android running on sanitizer-buildbot-android while building llvm at step 2 "annotate".

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

Here is the relevant piece of the build log for the reference
Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure)
...
[       OK ] AddressSanitizer.AtoiAndFriendsOOBTest (2346 ms)
[ RUN      ] AddressSanitizer.HasFeatureAddressSanitizerTest
[       OK ] AddressSanitizer.HasFeatureAddressSanitizerTest (0 ms)
[ RUN      ] AddressSanitizer.CallocReturnsZeroMem
[       OK ] AddressSanitizer.CallocReturnsZeroMem (18 ms)
[ DISABLED ] AddressSanitizer.DISABLED_TSDTest
[ RUN      ] AddressSanitizer.IgnoreTest
[       OK ] AddressSanitizer.IgnoreTest (0 ms)
[ RUN      ] AddressSanitizer.SignalTest
[       OK ] AddressSanitizer.SignalTest (191 ms)
[ RUN      ] AddressSanitizer.ReallocTest
[       OK ] AddressSanitizer.ReallocTest (32 ms)
[ RUN      ] AddressSanitizer.WrongFreeTest
[       OK ] AddressSanitizer.WrongFreeTest (134 ms)
[ RUN      ] AddressSanitizer.LongJmpTest
[       OK ] AddressSanitizer.LongJmpTest (0 ms)
[ RUN      ] AddressSanitizer.ThreadStackReuseTest
[       OK ] AddressSanitizer.ThreadStackReuseTest (1 ms)
[ DISABLED ] AddressSanitizer.DISABLED_MemIntrinsicUnalignedAccessTest
[ DISABLED ] AddressSanitizer.DISABLED_LargeFunctionSymbolizeTest
[ DISABLED ] AddressSanitizer.DISABLED_MallocFreeUnwindAndSymbolizeTest
[ RUN      ] AddressSanitizer.UseThenFreeThenUseTest
[       OK ] AddressSanitizer.UseThenFreeThenUseTest (128 ms)
[ RUN      ] AddressSanitizer.FileNameInGlobalReportTest
[       OK ] AddressSanitizer.FileNameInGlobalReportTest (128 ms)
[ DISABLED ] AddressSanitizer.DISABLED_StressStackReuseAndExceptionsTest
[ RUN      ] AddressSanitizer.MlockTest
[       OK ] AddressSanitizer.MlockTest (0 ms)
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadedTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowIn
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowLeft
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowRight
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFHigh
[ DISABLED ] AddressSanitizer.DISABLED_DemoOOM
[ DISABLED ] AddressSanitizer.DISABLED_DemoDoubleFreeTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoNullDerefTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoFunctionStaticTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoTooMuchMemoryTest
[ RUN      ] AddressSanitizer.LongDoubleNegativeTest
[       OK ] AddressSanitizer.LongDoubleNegativeTest (0 ms)
[----------] 19 tests from AddressSanitizer (28458 ms total)

[----------] Global test environment tear-down
[==========] 22 tests from 2 test suites ran. (28461 ms total)
[  PASSED  ] 22 tests.

  YOU HAVE 1 DISABLED TEST

Step 24 (run instrumented asan tests [aarch64/aosp_coral-userdebug/AOSP.MASTER]) failure: run instrumented asan tests [aarch64/aosp_coral-userdebug/AOSP.MASTER] (failure)
...
[ RUN      ] AddressSanitizer.HasFeatureAddressSanitizerTest
[       OK ] AddressSanitizer.HasFeatureAddressSanitizerTest (0 ms)
[ RUN      ] AddressSanitizer.CallocReturnsZeroMem
[       OK ] AddressSanitizer.CallocReturnsZeroMem (6 ms)
[ DISABLED ] AddressSanitizer.DISABLED_TSDTest
[ RUN      ] AddressSanitizer.IgnoreTest
[       OK ] AddressSanitizer.IgnoreTest (0 ms)
[ RUN      ] AddressSanitizer.SignalTest
[       OK ] AddressSanitizer.SignalTest (312 ms)
[ RUN      ] AddressSanitizer.ReallocTest
[       OK ] AddressSanitizer.ReallocTest (22 ms)
[ RUN      ] AddressSanitizer.WrongFreeTest
[       OK ] AddressSanitizer.WrongFreeTest (244 ms)
[ RUN      ] AddressSanitizer.LongJmpTest
[       OK ] AddressSanitizer.LongJmpTest (0 ms)
[ RUN      ] AddressSanitizer.ThreadStackReuseTest
[       OK ] AddressSanitizer.ThreadStackReuseTest (2 ms)
[ DISABLED ] AddressSanitizer.DISABLED_MemIntrinsicUnalignedAccessTest
[ DISABLED ] AddressSanitizer.DISABLED_LargeFunctionSymbolizeTest
[ DISABLED ] AddressSanitizer.DISABLED_MallocFreeUnwindAndSymbolizeTest
[ RUN      ] AddressSanitizer.UseThenFreeThenUseTest
[       OK ] AddressSanitizer.UseThenFreeThenUseTest (306 ms)
[ RUN      ] AddressSanitizer.FileNameInGlobalReportTest
[       OK ] AddressSanitizer.FileNameInGlobalReportTest (283 ms)
[ DISABLED ] AddressSanitizer.DISABLED_StressStackReuseAndExceptionsTest
[ RUN      ] AddressSanitizer.MlockTest
[       OK ] AddressSanitizer.MlockTest (0 ms)
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadedTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowIn
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowLeft
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowRight
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFHigh
[ DISABLED ] AddressSanitizer.DISABLED_DemoOOM
[ DISABLED ] AddressSanitizer.DISABLED_DemoDoubleFreeTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoNullDerefTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoFunctionStaticTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoTooMuchMemoryTest
[ RUN      ] AddressSanitizer.LongDoubleNegativeTest
[       OK ] AddressSanitizer.LongDoubleNegativeTest (0 ms)
[----------] 19 tests from AddressSanitizer (71075 ms total)

[----------] Global test environment tear-down
[==========] 22 tests from 2 test suites ran. (71088 ms total)
[  PASSED  ] 22 tests.

  YOU HAVE 1 DISABLED TEST

Serial 17031FQCB00176

@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 30, 2025

LLVM Buildbot has detected a new failure on builder lld-x86_64-win running on as-worker-93 while building llvm at step 7 "test-build-unified-tree-check-all".

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

Here is the relevant piece of the build log for the reference
Step 7 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/90/95' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-19856-90-95.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=95 GTEST_SHARD_INDEX=90 C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe --gtest_filter=ProgramEnvTest.CreateProcessLongPath
--
C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(160): error: Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(163): error: fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied



C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:160
Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:163
fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied




********************


chapuni added a commit that referenced this pull request Apr 30, 2025
I've introduced the split lib `AnalysisFpExc`. Let me know if better
solutions would be applicable.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…ding (llvm#137652)

To perform constant folding in math operations, the implementation of
the ConstantFolding Analysis relies on the use of the math functions
from the host's libm. In particular, it relies on checking the value of
errno and IEEE exceptions to determine when an operation is safe to be
constant-folded.

On some platforms, such as BSD or Darwin, math library functions don't
set errno, so the ConstantFolding check depends only on the value of
IEEE exceptions. As the FP exception behaviour is set to `ignore` by
default, the compiler can perform optimisations that would get in the
way of such checks being performed correctly.

This patch sets the FP exception behaviour to `strict` when compiling
the `ConstantFolding.cpp` source file, ensuring the value of IEEE
exceptions can be reliably used by its implementation.

This re-lands the changes from llvm#136139, but using the `-ftrapping-math`
compile option instead of `-ffp-exception-behavior` for GCC support.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
I've introduced the split lib `AnalysisFpExc`. Let me know if better
solutions would be applicable.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…ding (llvm#137652)

To perform constant folding in math operations, the implementation of
the ConstantFolding Analysis relies on the use of the math functions
from the host's libm. In particular, it relies on checking the value of
errno and IEEE exceptions to determine when an operation is safe to be
constant-folded.

On some platforms, such as BSD or Darwin, math library functions don't
set errno, so the ConstantFolding check depends only on the value of
IEEE exceptions. As the FP exception behaviour is set to `ignore` by
default, the compiler can perform optimisations that would get in the
way of such checks being performed correctly.

This patch sets the FP exception behaviour to `strict` when compiling
the `ConstantFolding.cpp` source file, ensuring the value of IEEE
exceptions can be reliably used by its implementation.

This re-lands the changes from llvm#136139, but using the `-ftrapping-math`
compile option instead of `-ffp-exception-behavior` for GCC support.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
I've introduced the split lib `AnalysisFpExc`. Let me know if better
solutions would be applicable.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…ding (llvm#137652)

To perform constant folding in math operations, the implementation of
the ConstantFolding Analysis relies on the use of the math functions
from the host's libm. In particular, it relies on checking the value of
errno and IEEE exceptions to determine when an operation is safe to be
constant-folded.

On some platforms, such as BSD or Darwin, math library functions don't
set errno, so the ConstantFolding check depends only on the value of
IEEE exceptions. As the FP exception behaviour is set to `ignore` by
default, the compiler can perform optimisations that would get in the
way of such checks being performed correctly.

This patch sets the FP exception behaviour to `strict` when compiling
the `ConstantFolding.cpp` source file, ensuring the value of IEEE
exceptions can be reliably used by its implementation.

This re-lands the changes from llvm#136139, but using the `-ftrapping-math`
compile option instead of `-ffp-exception-behavior` for GCC support.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
I've introduced the split lib `AnalysisFpExc`. Let me know if better
solutions would be applicable.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…ding (llvm#137652)

To perform constant folding in math operations, the implementation of
the ConstantFolding Analysis relies on the use of the math functions
from the host's libm. In particular, it relies on checking the value of
errno and IEEE exceptions to determine when an operation is safe to be
constant-folded.

On some platforms, such as BSD or Darwin, math library functions don't
set errno, so the ConstantFolding check depends only on the value of
IEEE exceptions. As the FP exception behaviour is set to `ignore` by
default, the compiler can perform optimisations that would get in the
way of such checks being performed correctly.

This patch sets the FP exception behaviour to `strict` when compiling
the `ConstantFolding.cpp` source file, ensuring the value of IEEE
exceptions can be reliably used by its implementation.

This re-lands the changes from llvm#136139, but using the `-ftrapping-math`
compile option instead of `-ffp-exception-behavior` for GCC support.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
I've introduced the split lib `AnalysisFpExc`. Let me know if better
solutions would be applicable.
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
…ding (llvm#137652)

To perform constant folding in math operations, the implementation of
the ConstantFolding Analysis relies on the use of the math functions
from the host's libm. In particular, it relies on checking the value of
errno and IEEE exceptions to determine when an operation is safe to be
constant-folded.

On some platforms, such as BSD or Darwin, math library functions don't
set errno, so the ConstantFolding check depends only on the value of
IEEE exceptions. As the FP exception behaviour is set to `ignore` by
default, the compiler can perform optimisations that would get in the
way of such checks being performed correctly.

This patch sets the FP exception behaviour to `strict` when compiling
the `ConstantFolding.cpp` source file, ensuring the value of IEEE
exceptions can be reliably used by its implementation.

This re-lands the changes from llvm#136139, but using the `-ftrapping-math`
compile option instead of `-ffp-exception-behavior` for GCC support.
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
I've introduced the split lib `AnalysisFpExc`. Let me know if better
solutions would be applicable.
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