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

[Mel Chen via Phabricator via llvm-commits]

Mel-Chen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4003
+    ReducedPartRdx =
+        createSentinelValueHandling(Builder, TTI, RdxDesc, ReducedPartRdx);
+
----------------
shiva0217 wrote:
> shiva0217 wrote:
> > Mel-Chen wrote:
> > > shiva0217 wrote:
> > > > Could the function rename to createSelectIVCmpTargetReduction and be called from createSelectCmpTargetReduction?
> > > > Perhaps CreateIntMaxReduce can be moved to the function?
> > > I'm afraid I won't be able to meet this requirement. Placing `createSentinelValueHandling` in this position is for handling the case when the vector width is 1. You could refer to CHECK-VF1IC4 in the test cases and focus on the `middle.block`. In implementation, VF1IC4 doesn't call `createTargetReduction`, but `ReducedPartRdx` still need to be did the sentinel value fixing.
> > > 
> > > However, perhaps we can create a new bool function for `RK == RecurKind::SelectIVICmp || RK == RecurKind::SelectIVFCmp`. This will most likely expand further and cause the if-condition to become too long.
> > Thanks for the explanation!
> > 
> > Could we use "} else if ((!VF.isVector() && !PhiR->isInLoop()))" to guard the generation?
> > 
> > It could be easier to understand the codegen is needed when VF is not a vector and createTargetReduction won't be invoked.
> > 
> > Should we rename createSentinelValueHandling as createSelectInitValOrReduction?
> > 
> > I feel it could reflect the codegen but in a less strong opinion.
> > 
> Oops, I think I mix the patch with some local changes. Please ignore the comment.
No problem. And I've just rebased this patch. Please continue with the review. Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150851