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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

I neglected to hit the "submit" button last week, these are the inline questions I have.

Also in addition to the comments above, check-clang is showing 1 LIT test failing AST/const-fpfeatures.cpp
I believe the clang constant folder is returning False for the expression, and the constant folding is being done by the LLVM constant folder
llvm-project/clang/test/AST/const-fpfeatures.cpp:21:11: error: CHECK: expected string not found in input
// CHECK: @V1 = {{.*}} float 1.000000e+00

  ^

<stdin>:1:1: note: scanning from here
; ModuleID = '/export/iusers/mblower/sandbox/llorg/llvm-project/clang/test/AST/const-fpfeatures.cpp'
^
<stdin>:24:1: note: possible intended match here
@V1 = global float 0.000000e+00, align 4

The reason i think clang constant folder/my patch/ isn't to blame for faulty logic is because if I change the test to make V1 constexpr, there is an error message "not constant". So i'm not sure if the bug is in clang or the test case needs to be fixed?



================
Comment at: clang/lib/AST/ExprConstant.cpp:2734
   if (LHS.isNaN()) {
-    Info.CCEDiag(E, diag::note_constexpr_float_arithmetic) << LHS.isNaN();
+    Info.FFDiag(E, diag::note_constexpr_float_arithmetic) << LHS.isNaN();
     return Info.noteUndefinedBehavior();
----------------
This should be FFDiag?


================
Comment at: clang/lib/AST/ExprConstant.cpp:12302
+        E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+      // Note: Compares may raise invalid in some cases involving NaN or sNaN.
+      Info.FFDiag(E, diag::note_constexpr_float_arithmetic_strict);
----------------
My scribbled notes say "ISO 10967 LIA-1 
Equality returns invalid if either operand is signaling NaN
Other comparisons < <= >= > return invalid if either operand is NaN".  I'm not putting my hands on the exact reference. 


================
Comment at: clang/test/CodeGen/pragma-fenv_access.c:9
+// CHECK-LABEL: @func_01
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+
----------------
sepavloff wrote:
> Shall the rounding mode be `dynamic` rather than `tonearest`? If `#pragma STDC FENV_ACCESS ON` is specified, it means FP environment may be changed in arbitrary way and we cannot expect any particular rounding mode. Probably the environment was changed in the caller of `func_01`.
Yes thanks @sepavloff !  i made this change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528