[PATCH] D155786: [LV] Rename the Select[I|F]Cmp reduction pattern to [I|F]AnyOf. (NFC)

[Mel Chen via Phabricator via llvm-commits]

Mel-Chen marked 3 inline comments as done.
Mel-Chen added inline comments.


================
Comment at: llvm/include/llvm/Analysis/IVDescriptors.h:237
   ///   select(cmp(),x,y) where one of (x,y) is loop invariant.
   static bool isSelectCmpRecurrenceKind(RecurKind Kind) {
+    return Kind == RecurKind::IAnyOf || Kind == RecurKind::FAnyOf;
----------------
Ayal wrote:
> Mel-Chen wrote:
> > Ayal wrote:
> > > Would it make (more) sense to rename this `isAnyOfRecurrenceKind()`?
> > I suggest this:
> > ```
> > static bool isAnyOfRecurrenceKind(RecurKind Kind) {
> >   return Kind == RecurKind::IAnyOf || Kind == RecurKind::FAnyOf;
> > }
> > 
> > static bool isSelectCmpRecurrenceKind(RecurKind Kind) {
> >   return isAnyOfRecurrenceKind(Kind);
> > }
> > ```
> Will `isSelectCmpRecurrenceKind()` still be needed?
> Min/max may also uses a compare-select pattern, leading to potential confusion of "isSelectCmpRecurrenceKind()".
Because eventually, D150851 will need `isSelectCmpRecurrenceKind()`. Both `AnyOf` and `FindLastIV` share common characteristics, such as similar identification methods. Using `isSelectCmpRecurrenceKind()` to control similar behavior should be better than `isAnyOfRecurrenceKind() || isFindLastIVRecurrenceKind()`. However, I also agree that `isSelectCmpRecurrenceKind()` might lead to confusion since the format of min max reduction is really similar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155786