[PATCH] D77869: [VPlan] Introduce VPWidenSelectRecipe (NFC).

Gil Rapaport via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 12 06:23:40 PDT 2020


gilr added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6940
+  auto *SI = dyn_cast<SelectInst>(I);
+  if (!SI)
+    return nullptr;
----------------
SI is not used beyond this point until recipe construction. Better to either use it consistently instead of I or only check isa<> here and cast<> at recipe construction.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6944
+  bool IsPredicated = LoopVectorizationPlanner::getDecisionAndClampRange(
+      [&](unsigned VF) { return CM.isScalarWithPredication(I, VF); }, Range);
+
----------------
This can fold into willWiden.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:724
+  O << " +\n"
+    << Indent << "\"WIDEN-SELECT" << VPlanIngredient(&Ingredient) << "\\l\"";
+}
----------------
fhahn wrote:
> gilr wrote:
> > Should also print InvariantCond as e.g. in VPWidenGEPRecipe::print()
> Done, but I am not sure how to best print it specifically for the condition. I've added a note to the debug output.
Agree, standardizing how to mark invariant operands would be easier for recipes using VPValues (e.g. just adding "[Inv]" before/after invariant operands).


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:725
+    << Indent << "\"WIDEN-SELECT" << VPlanIngredient(&Ingredient)
+    << " (condition is " << (InvariantCond ? "" : " not ")
+    << "loop invariant)\\l\"";
----------------
Since it's an infrequent optimization it might be cleaner to only print if relevant, i.e. " << (InvariantCond ? " (condition is loop invariant)" : "").


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77869





More information about the llvm-commits mailing list