[PATCH] D150851: [LoopVectorize] Vectorize select-cmp reduction pattern for increasing integer induction variable

[Shiva Chen via Phabricator via llvm-commits]

shiva0217 added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/LoopUtils.h:364
 /// between two values: 1) an initial PHI start value, and 2) a loop invariant
-/// value. This function uses \p LoopExitInst to determine 2), which we then use
-/// to select between \p Left and \p Right. Any lane value in \p Left that
-/// matches 2) will be merged into \p Right.
+/// value and increasing loop induction variable. This function uses \p
+/// LoopExitInst to determine 2), which we then use to select between \p Left
----------------
or an increasing loop induction variable?


================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:1162
 
+Value *llvm::createSentinelValueHandling(IRBuilderBase &Builder,
+                                         const TargetTransformInfo *TTI,
----------------
It might be worth to have a comment to describe that the SelectIVICmp and SelectIVFCmp code generation will use Identity value to determine the if condition in the following case has ever been true.

  int r = 331;
  for (int i = 0; i < n; i++)
    if (src[i] > 111)
      r = i;

When the reduction value(Rdx) equal to the Identity value(Iden), it reveals the condition never been true. So it will select the InitVal.

It might be the reason that in IsIncreasingLoopInduction, the function will check the IV start value to avoid the IV overlapping the Identity value.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4003
+    ReducedPartRdx =
+        createSentinelValueHandling(Builder, TTI, RdxDesc, ReducedPartRdx);
+
----------------
Could the function rename to createSelectIVCmpTargetReduction and be called from createSelectCmpTargetReduction?
Perhaps CreateIntMaxReduce can be moved to the function?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:1307
     }
+  } else if (RK == RecurKind::SelectIVICmp || RK == RecurKind::SelectIVFCmp) {
+    StartV = Iden = RdxDesc.getRecurrenceIdentity(RK, VecTy->getScalarType(),
----------------
Perhaps a comment to describe that SelectIVICmp and SelectIVFCmp will initial the reduction PHI with Iden and createSentinelValueHandling will use Iden to determine the if condition in the loop has ever been true?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150851