[LV] Enable Histogram autovectorization by default

[huntergr-arm]

Enabling the feature by default in order to get further testing data.

There are a few loops in popular benchmarks where this kicks in,
but none of them are particularly hot, so I don't know if this is
beneficial on current hardware. But I haven't seen any regressions yet
either.

I'll take another look at the cost functions to see if there's improvements
I can make (as separate PRs).

[llvmbot]

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Graham Hunter (huntergr-arm)

Changes

Enabling the feature by default in order to get further testing data.

There are a few loops in popular benchmarks where this kicks in,
but none of them are particularly hot, so I don't know if this is
beneficial on current hardware. But I haven't seen any regressions yet
either.

I'll take another look at the cost functions to see if there's improvements
I can make (as separate PRs).

---

Full diff: https://github.com/llvm/llvm-project/pull/158292.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp (+1-1)
• (modified) llvm/test/Transforms/LoopVectorize/increment.ll (+1-3)

diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
index ff35db14f7094..3c771492a1e92 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
@@ -80,7 +80,7 @@ static cl::opt<LoopVectorizeHints::ScalableForceKind>
                 "cost is inconclusive.")));
 
 static cl::opt<bool> EnableHistogramVectorization(
-    "enable-histogram-loop-vectorization", cl::init(false), cl::Hidden,
+    "enable-histogram-loop-vectorization", cl::init(true), cl::Hidden,
     cl::desc("Enables autovectorization of some loops containing histograms"));
 
 /// Maximum vectorization interleave count.
diff --git a/llvm/test/Transforms/LoopVectorize/increment.ll b/llvm/test/Transforms/LoopVectorize/increment.ll
index b5af65a8c99c8..e71b50544ff67 100644
--- a/llvm/test/Transforms/LoopVectorize/increment.ll
+++ b/llvm/test/Transforms/LoopVectorize/increment.ll
@@ -33,13 +33,11 @@ define void @inc(i32 %n) nounwind uwtable noinline ssp {
   ret void
 }
 
-; Can't vectorize this loop because the access to A[X] is non-linear.
-;
 ;  for (i = 0; i < n; ++i) {
 ;    A[B[i]]++;
 ;
 ;CHECK-LABEL: @histogram(
-;CHECK-NOT: <4 x i32>
+;CHECK:  call void @llvm.experimental.vector.histogram.add.v4p0.i32(<4 x ptr> %{{.*}}, i32 1, <4 x i1> splat (i1 true))
 ;CHECK: ret i32
 define i32 @histogram(ptr nocapture noalias %A, ptr nocapture noalias %B, i32 %n) nounwind uwtable ssp {
 entry:

[paschalis-mpeis]

Looks good. Let's see how it performs in the wild and what opportunities it uncovers.

[fhahn]

It would probably be good to add some micro benchmarks for loops with histograms to https://github.com/llvm/llvm-test-suite/tree/main/MicroBenchmarks/LoopVectorization, to make it easier to evaluate performance

[fhahn]

reverse ping.

Are there any remaining blockers for changing the defaults?

[Andarwinux]

reverse ping.

Are there any remaining blockers for changing the defaults?

https://godbolt.org/z/3YxEeWzd3

The codegen on x86 is terrible,

[huntergr-arm]

reverse ping.

Are there any remaining blockers for changing the defaults?

I think I need to fix the cost model for AArch64 with SVE 2 (it was still trying to work with the legacy cost model and doesn't account for the gather/scatter cost, which were separate at that point). I'll post a fix for that.

The benchmarks you requested haven't been reviewed yet though: llvm/llvm-test-suite#367

[huntergr-arm]

reverse ping.
Are there any remaining blockers for changing the defaults?

https://godbolt.org/z/3YxEeWzd3

The codegen on x86 is terrible,

Hmm. Perhaps the default costs need to be increased to avoid it? (It's possible to use vpconflict for this, but the X86 backend isn't my area of expertise)

[huntergr-arm]

Raised #216337 to address the cost underestimates. (We'll overestimate in some cases instead, but that's a safer starting position)