[PATCH] D139902: IR: Add nofpclass parameter attribute

Tue Dec 20 06:13:13 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:
> > > 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.

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

https://reviews.llvm.org/D139902