[PATCH] D129560: [AArch64] Add target hook for preferPredicateOverEpilogue

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:3034-3042
+  if (!(TailFoldingKindLoc & TailFoldingKind::TFReductions) &&
+      LVL->getReductionVars().size())
+    return false;
+
+  if (!(TailFoldingKindLoc & TailFoldingKind::TFRecurrences) &&
+      LVL->getFirstOrderRecurrences().size())
+    return false;
----------------
david-arm wrote:
> paulwalker-arm wrote:
> > I don't think this works now we have the expanded bitfield.  I think you need logic like:
> > ```
> > TailFoldingKind Required = 0;
> > if (LVL->getReductionVars().size())
> >   Required.add(TailFoldingKind::TFReductions)
> > if (LVL->getReductionVars().size())
> >   Required.add(TailFoldingKind::TFRecurrences)
> > if (!Required)
> >   Required.add(TailFoldingKind::TFSimple)
> > 
> > return TailFoldingKindLoc & Required
> > ```
> Hmm, the existing code I have does work since the tests I added all pass so I don't believe there is a bug. With the existing version if the user didn't request reductions and the loop contains at least one reduction then `!(TailFoldingKindLoc & TailFoldingKind::TFReductions) && LVL->getReductionVars().size()` is true and so we return false. This is the same as your suggested code because `Required != TailFoldingKindLoc` in that case. We then return true at the end if either: a) the loop is 'simple', or b) the user has explicitly permitted tail-folding with reductions and/or recurrences.
> 
> Having said that, I'm happy to give your suggestion a try if you think it reads better!
If `TailFoldingKind::TFReductions` is set and the loop contains no reduction then tail folding will be enabled regardless of whether `TailFoldingKind::TFSimple` is set.  Which I think is wrong?


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

https://reviews.llvm.org/D129560