[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

Joerg Sonnenberger via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 4 16:44:53 PDT 2021


joerg added inline comments.


================
Comment at: clang/docs/UsersManual.rst:1484
+   Where unsafe floating point optimizations are enabled, this option
+   determines whether the optimizer honors parentheses when floating-point
+   expressions are evaluated.  If unsafe floating point optimizations are
----------------
aaron.ballman wrote:
> mibintc wrote:
> > aaron.ballman wrote:
> > > We may need to expound on what "honor parentheses" means. The summary for the patch mentions `(a + b) + c`, but does this also mean `(x + y) * z`  could be interpreted as `x + (y * z)`? What about for non-arithmetic expressions involving parentheses, like `(float1 == float2 && float1 == float3) || blah()`?
> > Thanks for your review. Just a quick first response, What do you mean "(x + y) * z could be reinterpreted as x + (y * z)" -- not following you here? 
> "whether the optimizer honors the parentheses" reads to me like `(x + y) * z` could either honor or not honor the parentheses, so that could either evaluate as `(x + y) * z` or `x + (y * z)` depending on whether the parens are honored or not.
I would prefer to start with a description of the intended behavior from first principles. The logic here should not be tied to fast-mode as it also overlaps a lot with the formulation of fused multiply-add expressions and certainly should have specified behavior on the interaction. I'm also not sure about what you mean with value-safe FP modes, IIRC the C/C++ standard explicitly leave it open in which order floating point expressions of the same operand priority are evaluated.

I'm also a bit puzzled why __arithmethic_fence should not be considered a constant expression?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100118/new/

https://reviews.llvm.org/D100118



More information about the llvm-commits mailing list