[PATCH] D88913: [FPEnv] Use strictfp metadata in casting nodes

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:137-139
+  FPOptions FPFeatures = E->getFPFeaturesInEffect(CGF.getLangOpts());
+
+  ConstructorHelper(FPFeatures);
----------------
The local variable `FPFeatures` can be eliminated here.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:154-155
+  OldRounding = CGF.Builder.getDefaultConstrainedRounding();
   if (OldFPFeatures == FPFeatures)
     return;
 
----------------
This check can be lifted to just after the definition of `OldFPFeatures`.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:618-619
     FPOptions OldFPFeatures;
+    llvm::fp::ExceptionBehavior OldExcept;
+    llvm::RoundingMode OldRounding;
     Optional<CGBuilderTy::FastMathFlagGuard> FMFGuard;
----------------
Is it possible to merge this variables into `OldFPFeatures`? In what cases state of `Builder` may differ from the state of `CGF.CurFPFeatures`?


================
Comment at: clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics-constrained.c:6
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon -target-feature +fullfp16 -target-feature +v8.2a\
-// RUN: -ffp-exception-behavior=strict \
+// RUN: -ffp-exception-behavior=maytrap -DEXCEPT=1 \
 // RUN: -fexperimental-strict-floating-point \
----------------
Why did you change exception behavior to `maytrap`?


================
Comment at: clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics-constrained.c:26
+// metadata from the AST instead of the global default from the command line.
+// FIXME: All cases of "fpexcept.maytrap" in this test are wrong.
+
----------------
Why they are wrong?


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

https://reviews.llvm.org/D88913