[PATCH] D120215: [LV] Invalidate widening decisions after maximizing vector bandwidth

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

> This still now the EmulatedMaskMemRefHack costs (a bit unfortunately), ...

Looks like a word may be missing.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5553
   ElementCount MaxVF = MaxVectorElementCount;
-  if (TTI.shouldMaximizeVectorBandwidth() ||
-      (MaximizeBandwidth && isScalarEpilogueAllowed())) {
+  if ((MaximizeBandwidth.getNumOccurrences() > 0 && MaximizeBandwidth) ||
+      TTI.shouldMaximizeVectorBandwidth()) {
----------------
> This slightly changes the way that the MaximizeVectorBandwidth option works to make it easier to test, always honouring the option if it is set.

IIUC this effectively ignores the default value of the option now, right? How does that make it easier to test?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5554
+  if ((MaximizeBandwidth.getNumOccurrences() > 0 && MaximizeBandwidth) ||
+      TTI.shouldMaximizeVectorBandwidth()) {
     auto MaxVectorElementCountMaxBW = ElementCount::get(
----------------
nit: check ` TTI.shouldMaximizeVectorBandwidth()` first ,like in the original code? 


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/maximize-bandwidth-invalidate.ll:13
+
+; COST: LV: Found an estimated cost of 3000000 for VF 2 For instruction:   %0 = load
+; COST: LV: Found an estimated cost of 3000000 for VF 4 For instruction:   %0 = load
----------------
Could you pre-commit the test and and update the diff to only show the difference?


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/maximize-bandwidth-invalidate.ll:94
+  %incdec.ptr = getelementptr inbounds i8, i8* %pInVec.addr.042, i64 1
+  %0 = load i8, i8* %pInVec.addr.042, align 1
+  %conv = sext i8 %0 to i32
----------------
The test is only checking the cost of `%0`, could the loop body be reduced to fewer loads/reductions?


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

https://reviews.llvm.org/D120215