[PATCH] D92284: [VPlan] Manage induction value creation using VPValues.

Gil Rapaport via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 19 14:39:19 PST 2021


gilr added a comment.
Herald added a subscriber: tschuett.

Should the (final) goal be to move the whole phi-trunc-cast logic from code-gen to vplanning phase?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1888
   for (unsigned Part = 0; Part < UF; ++Part) {
-    VectorLoopValueMap.setVectorValue(EntryVal, Part, LastInduction);
+    // VectorLoopValueMap.setVectorValue(EntryVal, Part, LastInduction);
+    State.set(Def, EntryVal, LastInduction, Part);
----------------
Delete


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1950
   // induction update chain itself.
   Instruction *CastInst = *Casts.begin();
+  assert(CastDef);
----------------
Isn't extracting CastInst here from ID redundant now, given CastDef which wraps it?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1951
   Instruction *CastInst = *Casts.begin();
+  assert(CastDef);
   if (Lane < UINT_MAX)
----------------
&& "error message"


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8076
+      IV,
+      IsTrunc ? cast<TruncInst>(getVPValue(0)->getUnderlyingValue()) : nullptr,
+      getVPValue(0), getNumDefinedValues() == 2 ? getVPValue(1) : nullptr,
----------------
Can IsTrunc be replaced by dyn_cast<>? (and isa<> in print()?)


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8077
+      IsTrunc ? cast<TruncInst>(getVPValue(0)->getUnderlyingValue()) : nullptr,
+      getVPValue(0), getNumDefinedValues() == 2 ? getVPValue(1) : nullptr,
+      State);
----------------
Might be good to have named getters for the trunc and the cast defs.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8319
+
+  if (hasScalarValue(Def, {Part, 0})) {
+    Value *ScalarValue = get(Def, {Part, 0});
----------------
- Make this an early exit?
- Worth leaving a TODO for replacing the IF with an assertion once all scalar recipes are VPDef'ed and throwing away that callback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92284



More information about the llvm-commits mailing list