[PATCH] D76992: [VPlan] Add & use VPValue operands for VPWidenRecipe (NFC).

Ayal Zaks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 30 15:51:53 PDT 2020


Ayal added a comment.

Thanks for following-up quickly. The part that extends VPWidenRecipe to have a User for holding its VPValue operands looks ok, but could the patch make sure that these operands are indeed used correctly by ILV::widenInstruction(), instead of moving the latter over to its execute()? The former may involve refactoring the case that deals with Calls, to record cost-based UseVectorInstrinsic/NeedToScalarize decisions when constructing the VPlan/recipe rather than during codegen; to record InvariantCond of Selects instead of accessing getSCEV(I.getOperand(0)) during codegen, possibly cleaning up to generate ScalarCond OR Cond but not both. In any case, accesses to I’s operands should all change to access User's operands instead, possibly with their underlying ingredient if needed.

This follows the changes D70865 <https://reviews.llvm.org/D70865> made to ILV::vectorizeInterleaveGroup() and ILV::vectorizeMemoryInstruction(), focusing only on transforming the def/use relations between recipes, as a first (or in this case second ;-) separate step.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7197
+  Instruction &I = *Ingredient;
+  auto GetVectorOps = [&State](ArrayRef<VPValue *> Ops, unsigned Part) {
+    SmallVector<Value *, 4> VecOps;
----------------
Simpler to pass the User to GetVectorOps() and have it iterate over its Operands?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7360
+      for (unsigned i = 0, ie = CI->getNumArgOperands(); i != ie; ++i) {
+        VPValue *Arg = User.getOperand(i);
+        // Some intrinsics have a scalar argument - don't replace it with a
----------------
Instead of accessing CI->getNumArgOperands(), get the number of User’s operands; or simply iterate over them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76992





More information about the llvm-commits mailing list