[PATCH] D78827: Add support for #pragma clang fp reassoc(on|off) -- floating point control of associative math transformations

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/docs/LanguageExtensions.rst:3159
+controlled with this pragma.
+``#pragma clang fp allow_reassociation`` allows control over the reassociation
+of floating point expressions. When enabled, this pragma allows the expression
----------------
sepavloff wrote:
> I would say the previous name, `reassoc`, was more consistent. We do not use `allow_contraction`.
"contract" isn't a shortening, though, it's a verb.  The idea is that a pragma is a directive to the compiler.  So arguably the more consistent spelling would be something like `#pragma clang fp reassociate`.


================
Comment at: clang/docs/LanguageExtensions.rst:3177
+Both floating point reassociation and floating point contraction can be
+controlled with this pragma.
+``#pragma clang fp reassoc`` allows control over the reassociation
----------------
Let's go ahead and word this as if arbitrary things will be controllable in the future.  So:

> Currently, the following things can be controlled by this pragma:


================
Comment at: clang/docs/LanguageExtensions.rst:3191
 option is identical to using ``#pragma STDC FP_CONTRACT(ON)`` and it allows
 fusion as specified the language standard.  The ``fast`` option allows fusiong
 in cases when the language standard does not make this possible (e.g. across
----------------
Please fix this typo while you're here.


================
Comment at: clang/include/clang/Basic/LangOptions.h:186
+    FPM_Fast
   };
 
----------------
I'm not sure I think this fusion was an improvement; the net effect was to remove a few lines from this header and make a bunch of switches unnecessarily non-exhaustive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78827