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

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:95-99
 
-  void add(uint8_t Flag) { Bits |= Flag; }
-  void remove(uint8_t Flag) { Bits &= ~Flag; }
+  void remove(uint8_t Flags) {
+    RemoveBits |= Flags;
+    AddBits &= ~Flags;
+  }
----------------
sdesmalen wrote:
> Perhaps I'm missing something, but this logic with `AddBits` and `RemoveBits` seems unnecessarily complicated. Can't you have a single bitfield and do something like this:
> 
>   void set(uint8_t Flags, bool Enable) {
>     if (Enable)
>       Bits |= Flags;
>     else
>       Bits &= ~Flags;
>   }
> 
> ?
The reason for this is because at the time of parsing the string we have no idea what the default options are going to be. I basically wanted to avoid creating a dependency on the CPU such that the user has to put the -sve-tail-folding option after the -mcpu flag. In the tests I added two RUN lines for both "-mcpu=neoverse-v1 -sve-tail-folding=default" and "-sve-tail-folding=default -mcpu=neoverse-v1". In the latter case we can't build on top of the default bits because we don't yet have them at the time of parsing. An example I'm thinking of is this:

  -sve-tail-folding=default+nosimple -mcpu=neoverse-v1

which is a bit daft (and we don't even have a nosimple option yet!). We only know the default (simple only for neoverse-v1) once we've parsed the -mcpu flag and therefore we can't remove the simple flag until later. So I tried doing this by keeping a record of the bits we want to add and remove, and apply them later. That's why a single 'Bits' field like you mentioned above doesn't work. I can have a look at your suggestion above and see if that solves the problem.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130618