[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff accepted this revision.
sepavloff added a comment.

LGTM

There is concern with restoring FP options but it could be solved separately.

Looking forward to results of SPEC runs!



================
Comment at: clang/docs/UsersManual.rst:1389
    * ``precise`` Disables optimizations that are not value-safe on floating-point data, although FP contraction (FMA) is enabled (``-ffp-contract=fast``).  This is the default behavior.
-   * ``strict`` Enables ``-frounding-math`` and ``-ffp-exception-behavior=strict``, and disables contractions (FMA).  All of the ``-ffast-math`` enablements are disabled.
+   * ``strict`` Enables ``-frounding-math`` and ``-ffp-exception-behavior=strict``, and disables contractions (FMA).  All of the ``-ffast-math`` enablements are disabled. Enables ``STDC FENV_ACCESS``.
    * ``fast`` Behaves identically to specifying both ``-ffast-math`` and ``ffp-contract=fast``
----------------
For a user manual `Enables STDC FENV_ACCESS` looks too vague. Probably we need to reword it to make more clear for compiler users. Something like `Assumes implicit #pragma STDC FENV_ACCESS ON at the beginning of source files`. Or, maybe this statement could be dropped.


================
Comment at: clang/lib/Sema/SemaAttr.cpp:1020-1023
+    // Resume the default rounding and exception modes.
+    NewFPFeatures.setRoundingModeOverride(
+        llvm::RoundingMode::NearestTiesToEven);
+    NewFPFeatures.setFPExceptionModeOverride(LangOptions::FPE_Ignore);
----------------
The previous values of rounding mode and exception behavior may differ from the default values. For example, `#pragma STDC FENV_ROUND` may set constant rounding direction different from FE_TONEAREST`. Similarly, command line options may set exception behavior different from `ignore`.
Can pragma stack be used for this?


================
Comment at: clang/lib/Sema/SemaDecl.cpp:14281-14282
 
+  if (FSI->UsesFPIntrin)
+    assert(FD);
+  if (FSI->UsesFPIntrin && !FD->hasAttr<StrictFPAttr>())
----------------
It is better to put `assert` into compound statement to avoid compiler confusion in release mode. Or to use something like: `assert(!FSI->UsesFPIntrin || FD)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528