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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
