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

Mon Oct 26 06:12:04 PDT 2020

sepavloff added a comment.

Generally the patch looks good. But the need to expect incorrect values in tests is a concern. Maybe this is a consequence of storing exception behavior in a separate field of CGFPOptionsRAII. This misbehavior should be fixed.

================
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.
+
----------------
kpn wrote:
> kpn wrote:
> > sepavloff wrote:
> > > kpn wrote:
> > > > sepavloff wrote:
> > > > > Why they are wrong?
> > > > Because the #pragma covers the entire file and sets exception handling to "strict". Thus all constrained intrinsic calls should be "strict", and if they are "maytrap" or "ignore" then we have a bug.
> > > What is the reason for that? Does `#pragma float_control` work incorrectly? Why  in `clang/test/CodeGen/complex-math-strictfp.c` exception handling is correct?
> > The #pragma works just fine. The problem is that we need to get the strictfp bits from the AST to the IRBuilder, and we haven't finished implementing that. So sometimes it works, like in complex-math-strictfp.c, and sometimes it doesn't. As you can see in the tests in this patch.
> I forgot to mention that complex-math-strictfp.c gets a correct BinOpInfo passed down to get the IRBuilder set correctly. But there are other places that don't correctly set BinOpInfo and so we get inconsistent behavior despite the call to the IRBuilder being wrapped in FPOptsRAII. And _that's_ why I structured this patch the way I did.
> The problem is that we need to get the strictfp bits from the AST to the IRBuilder

What is the status of this problem? Is it on track?

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

https://reviews.llvm.org/D88913