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

[David Sherwood via Phabricator via llvm-commits]

david-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;
----------------
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!


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

https://reviews.llvm.org/D129560