[PATCH] D139902: IR: Add nofpclass parameter attribute

Tue Dec 20 07:00:01 PST 2022

arsenm added inline comments.

================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14781
   case X86::BI__builtin_ia32_reduce_fmax_ph128: {
+    /// FIXME: This is broken
     Function *F =
----------------
pengfei wrote:
> arsenm wrote:
> > pengfei wrote:
> > > arsenm wrote:
> > > > pengfei wrote:
> > > > > What's broken here? Intrinsics are usually mapped directly to instructions which have different assumptions on the values.
> > > > Accidental diff. This is forcibly setting fast math flags on the builtins regardless of the fp mode, and it's not using a flag guard. This is going to infect later built instructions with wrong fast math flags
> > > Target specific intrinsics are flexible. Some of them imply specific fast math flags. Even compiled without fast math flags, passing e.g., NaNs in this case is UB.
> > Regardless of the behavior of the builtins, this is still not using a flag guard. It's making any instructions inserted with this Builder also have the fast math flags.
> Agreed. Thanks for the explanation!
https://godbolt.org/z/PTseYq3Y9 demonstrates the problem. The fadd ends up with nnan implying that c cannot be nan which is not implied by the source

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

https://reviews.llvm.org/D139902