[PATCH] D87391: [Intrinsics] define semantics for experimental fmax/fmin vector reductions

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:4805
     NeutralElem = DAG.getConstantFP(
-        std::numeric_limits<double>::infinity(), dl, ElemVT);
+        std::numeric_limits<double>::quiet_NaN(), dl, ElemVT);
     break;
----------------
nikic wrote:
> spatel wrote:
> > craig.topper wrote:
> > > Do we need to drop nonan FMF then? Probably should have been dropping noinf before.
> > > 
> > > Do we have non-power of 2 tests for X86? X86 needs nonan to optimally lower fmaxnum/fminnum. But if you put a nan here then we shouldn't be using optimal lowering.
> > Yes, we need to drop 'nnan' - otherwise this would create poison.
> > No, we don't have non-pow-2 vector sizes in x86 tests from what I see. I'll add some.
> Given how much X86 needs nnan for a decent lowering here, would it make sense to keep using +/- infinity if nnan is set, and only use qNan if it is not set?
Yes, but I think it's a little trickier than that. As Craig hinted, if we use inf, then we need to clear 'ninf' or we have the same poison problem.

Given that this is probably just crashing currently, the bar for quality is pretty low. :)
I'd defer enhancements to a follow-up if that's ok.


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

https://reviews.llvm.org/D87391