[PATCH] D118979: [AArch64] Set maximum VF with shouldMaximizeVectorBandwidth

Mon Apr 4 02:38:17 PDT 2022

dmgreen added a comment.

I think this option makes a lot of sense - and we have cleaned up a lot of places where it was causing issues. This can change a lot of performance, but it seems to be pretty good in my experiments. More is likely to come up, but I think we are close to ready to go so long as we keep an eye on the performance.

================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:39

+static cl::opt<bool> AArch64MaximizeBandwidth(
+    "aarch64-vectorizer-maximize-bandwidth", cl::init(true), cl::Hidden,
----------------
There is a vectorizer-maximize-bandwidth option is the vectorizer that can override the target option for shouldMaximizeVectorBandwidth. I don't think adding an aarch64 option is necessary, can you remove it?

================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5589

+  TargetTransformInfo::RegisterKind K =
+      ComputeScalableMaxVF ? TargetTransformInfo::RGK_ScalableVector
----------------
K is a bit of a short name. Perhaps use RegKind or something like it?

================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/sve-illegal-type.ll:90
 ; CHECK: store i1 %[[EXTRACT1]], i1* %dst
-; CHECK: %[[EXTRACT2:.*]] = extractelement <2 x i1> %[[ICMP]], i32 1
+; CHECK: %[[EXTRACT2:.*]] = extractelement <64 x i1> %[[ICMP]], i32 1
 ; CHECK: store i1 %[[EXTRACT2]], i1* %dst
----------------
This is worrying - should it be vectorizing 64x for in i1 type! (and are there a lot of other extracts now)?

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

https://reviews.llvm.org/D118979