[PATCH] D127041: [LLVM] Add the support for fmax and fmin in atomicrmw instruction

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp:67-69
+    case AtomicRMWInst::FMax:
+    case AtomicRMWInst::FMin:
       return CF->isNaN();
----------------
tianshilei1992 wrote:
> arsenm wrote:
> > This is just wrong.
> > 
> > I think this is questionable for the fadd/fsub case since I guess it depends if the payload bits are preserved?
> I fixed the cases for `fmax` and `fmin`. That's the only case off the top of my head.
> 
> As for `fadd`/`fsub`, I think it will be fine because the operation will be changed to `Xchg`, and the result will be `CF`, which conforms with the standard that:
> > An operation that propagates a NaN operand to its result and has a single NaN as an input should produce a NaN with the payload of the input NaN if representable in the destination format.
This is missing test coverage


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127041