[PATCH] D130618: [AArch64][LoopVectorize] Enable tail-folding by default for SVE

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:62-63
+        add(TFAll);
       else if (TailFoldType == "default")
-        Bits = 0; // Currently defaults to never tail-folding.
+        NeedsDefault = true;
       else if (TailFoldType == "simple")
----------------
sdesmalen wrote:
> This class seems to be mixing two concepts:
> * Features that the loop requires for tail folding (i.e. recurrences/reductions/anything-else).
> * What the target wants as the default and the parsing-logic for user-visible toggles.
> 
> This makes it quite tricky to follow the logic, especially the need for both `NeedsDefault` + `DefaultBits` and Add/RemoveBits`.
> 
> Perhaps it would make more sense to have two classes:
> 
>   class TailFoldingKind;    // This encodes which features the loop requires
>   class TailFoldingOption;  // This encodes the default (which itself will be a TailFoldingKind object), and has logic to parse strings.
> 
> Then you can add a method such as `TailFoldingKind TailFoldingOption::getSupportedMode() const { .. }` that you can use to query if the target's mode satisfies the required TailFoldingKind from the Loop.
This doesn't quite work because you've lost the position of the default value compared to the explicitly enabled/disabled options.  It's almost as if you want to defer parsing until when the data is required and then have something like `TailFoldingKindLoc.parseWithDefault(getTailFoldingDefaultForCPU()) `.  I think if you can do this, many of the other changes in the patch become necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130618