[PATCH] D137220: [LV][IVDescriptors] Fix recurrence identity element for FMin and FMax reductions.

[Karthik Senthil via Phabricator via llvm-commits]

karthiksenthil added a comment.

In D137220#3903584 <https://reviews.llvm.org/D137220#3903584>, @spatel wrote:

>> In D137220#3902612 <https://reviews.llvm.org/D137220#3902612>, @spatel wrote:
>>
>>> Should these be guarded with an FMF check?
>>>
>>> The enum definition says:
>>>
>>>   FMin,       ///< FP min implemented in terms of select(cmp()).
>>>   FMax,       ///< FP max implemented in terms of select(cmp()).
>>>
>>> So if we have a NaN input:
>>> fmin(NaN, Inf) --> select (fcmp olt NaN, Inf), NaN, Inf --> Inf (so the NaN input did not survive)
>>
>> Do you mean that identity value computation for FMin/FMax should be guarded with a `FMF.noNaNs()` check? What would be the identity value when the flag is absent?
>
> Yes, I'm not sure what it means if we don't have FMF.noNaNs(). Is it possible to create this recurrence without that FMF? If not, can we assert that FMF.noNaNs() is set?

Looks like the following code in `RecurrenceDescriptor::isRecurrenceInstr` guarantees that `nnan` flag is set for FMin/FMax reductions -

  case Instruction::FCmp:
  case Instruction::ICmp:
  case Instruction::Call:
    if (isSelectCmpRecurrenceKind(Kind))
      return isSelectCmpPattern(L, OrigPhi, I, Prev);
    if (isIntMinMaxRecurrenceKind(Kind) ||
        (((FuncFMF.noNaNs() && FuncFMF.noSignedZeros()) ||
          (isa<FPMathOperator>(I) && I->hasNoNaNs() &&
           I->hasNoSignedZeros())) &&
         isFPMinMaxRecurrenceKind(Kind)))
      return isMinMaxPattern(I, Kind, Prev);

I've added asserts in the latest diff to check that `FMF.noNaNs()` is set.


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

https://reviews.llvm.org/D137220