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

Mon Jul 18 05:48:01 PDT 2022

paulwalker-arm added a comment.

Assuming you agree with my suggestion I think it's worth breaking the LoopAccessInfo->LoopVectorizationLegality change into a separate NFC patch because it will make things easier if you miss the branch point and want to back port this change since you'll only be changing AArch64TargetTransformInfo.cpp.

================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:48
+                          "Don't tail-predicate loops using SVE"),
+               clEnumValN(TailPredication::EnabledNoReductions, "enabled-no-reductions",
+                          "Enable tail-predication with SVE, except for loops "
----------------
This doesn't really scale. What happens when there's another reason to allow the user to control tail predication? You'll need to add `EnabledNoBlob2`, then `EnabledNoReductionOrBlob2`.....

Is it worth adding a custom parsing class (assuming something doesn't already exist) so that we can do `-sve-tail-predication` for the default and then allow users to add a comma separated list of <option>s to enable or no-<option> to disable, along with your existing `disabled`/`enabled` options.

I'm not asking you to add new reasons to disable, only to make it easier to add then if necessary. That said, with this approach you could split the reduction and first order recurrences.

By extension I'm suggesting TailPredication wants to be a bit field like enum with `Disabled=0` and `Enabled=AllOnes`.

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

https://reviews.llvm.org/D129560