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