[PATCH] D96604: [LoopVectorizer] Require no-signed-zeros-fp-math=true for fmin/fmax

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Analysis/IVDescriptors.cpp:597-599
     if (!isIntMinMaxRecurrenceKind(Kind) &&
-        (!HasFunNoNaNAttr || !isFPMinMaxRecurrenceKind(Kind)))
+        (!(FMF.noNaNs() && FMF.noSignedZeros()) ||
+         !isFPMinMaxRecurrenceKind(Kind)))
----------------
That logic is hard to follow. How about inverting the condition, so we are positively identifying a min/max candidate?

I'm thinking:

```
    if (isIntMinMaxRecurrenceKind(Kind) ||
        (FMF.noNans() && FMF.noSignedZeros() && 
         isFPMinMaxRecurrenceKind(Kind)))
      return isMinMaxSelectCmpPattern(I, Prev);
```


================
Comment at: llvm/test/Transforms/LoopVectorize/float-minmax-instruction-flag.ll:151
 
+define float @minloopmissingnsz(float* nocapture readonly %arg) #1 {
+; CHECK-LABEL: @minloopmissingnsz(
----------------
I know we have an FMF mess currently, but can you add a test comment here to explain that this should not vectorize even after we switch to IR-level FMF? Or if there are other tests already checking for that scenario, point to those. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96604