[PATCH] D63214: [InstCombine] canonicalize fmin/fmax to LLVM intrinsics minnum/maxnum

[Cameron McInally via Phabricator via llvm-commits]

cameron.mcinally added a subscriber: eli.friedman.
cameron.mcinally added a comment.

This LGTM at a high level, but I don't fully understand the AVR concerns from D63294 <https://reviews.llvm.org/D63294>. Maybe @eli.friedman would be a better reviewer?



================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1583
+  FastMathFlags FMF = CI->getFastMathFlags();
+  FMF.setNoSignedZeros();
   B.setFastMathFlags(FMF);
----------------
It's pretty clear that the next IEEE-754 will respect zero sign bits for fmin/fmax. Would there be a big difference now if we didn't set nsz here? My thinking is that this line will become a bug when the new draft lands (and it's fairly hidden).

That said, if there is a behavior change by not setting the nsz flag, then we should probably just wait.


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

https://reviews.llvm.org/D63214