[PATCH] D77539: [LV] Add VPValue operands to VPBlendRecipe (NFCI)

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

minor comments, looks good to me, thanks.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6882
+    Operands.push_back(Plan->getOrAddVPValue(Phi->getIncomingValue(In)));
     if (EdgeMask)
+      Operands.push_back(EdgeMask);
----------------
This matches current behavior, as an NFCI patch should.

Suggest as a follow-up to remove the assert and replace this with something like

```
  VPValue *IncomingValue = Plan->getOrAddVPValue(Phi->getIncomingValue(In));
  if (!EdgeMask)
    return new VPBlendRecipe(Phi, {IncomingValue}):
  Operands.push_back(IncomingValue);
  Operands.push_back(EdgeMask);
```
which assures that the number of operands will be either one or even (still need to assert it isn't 2), and fixes PR44800 as @fhahn commented.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7405
+  // incoming value has no mask.
+  unsigned NumIncoming = (User.getNumOperands() + 1) / 2;
 
----------------
Have the recipe provide interfaces for getNumIncomingValues(), getIncomingValue(predecessor#), getMask(predecessor#), hiding their interleaved storage layout?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:882
+           "Expected either a single incoming value or an even number of "
+           "operands");
   }
----------------
"... an even number of operands"[, excluding 2]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77539