[SLP] introduce control over arithmetic horizontal reduction

[fedor.sergeev]

Summary

Horizontal reduction on AND (relatively recent transformation
introduced by rL345037) causes PR40310 miscompile.

Introducing -slp-vectorize-hor-arith boolean control to enable/disable
just horizontal reduction for arithmetic operations other than ADD/FADD.

Open question: true or false by default?

[fedor.sergeev]

clang-formatting

[RKSimon]

I'm really not sure about this - PR40310 is most likely a bug in the reduction code so isn't it likely that Add/FAdd can be affected by it as well?

[fedor.sergeev]

adding control to a couple tests that expect hor to happen

[fedor.sergeev]

In D56714#1357679, @RKSimon wrote:

I'm really not sure about this - PR40310 is most likely a bug in the reduction code so isn't it likely that Add/FAdd can be affected by it as well?

I'm not pushing this particular solution, its here only as a possible short-term.
However, I need to note that we have fuzzer running continuously and it started regularly finding bugs in reduction only after the aforementioned change went in.
Since late-October time we found ~10 different variations of it.

Given that we had no fuzzer bugs found in the vicinity of this code before late-October, it is safe to conclude that Add/FAdd does not trigger
the bug anywhere that often, even if the bug is in a common code.

[fedor.sergeev]

There is a proper fix suggested by @ABataev in PR40310, dropping this.

lib/Transforms/Vectorize/SLPVectorizer.cpp

static cl::opt<int>
     SLPCostThreshold("slp-threshold", cl::init(0), cl::Hidden,
                      cl::desc("Only vectorize if you gain more than this "
                               "number "));
 
 static cl::opt<bool>
 ShouldVectorizeHor("slp-vectorize-hor", cl::init(true), cl::Hidden,
                    cl::desc("Attempt to vectorize horizontal reductions"));
 
+static cl::opt<bool> ShouldVectorizeHorArith(
+    "slp-vectorize-hor-arith", cl::init(false), cl::Hidden,
+    cl::desc("Attempt to vectorize horizontal reductions for arithmetic "
+             "operations other than add/fadd."));
+
 static cl::opt<bool> ShouldStartVectorizeHorAtStore(
     "slp-vectorize-hor-store", cl::init(false), cl::Hidden,
     cl::desc(
         "Attempt to vectorize horizontal reductions feeding into a store"));
 
 static cl::opt<int>
 MaxVectorRegSizeOption("slp-max-reg-size", cl::init(128), cl::Hidden,
     cl::desc("Attempt to vectorize for this register size in bits"));
class OperationData {
     bool NoNaN = false;
 
     /// Checks if the reduction operation can be vectorized.
     bool isVectorizable() const {
       return LHS && RHS &&
              // We currently only support add/mul/logical && min/max reductions.
              ((Kind == RK_Arithmetic &&
                (Opcode == Instruction::Add || Opcode == Instruction::FAdd ||
-                Opcode == Instruction::Mul || Opcode == Instruction::FMul ||
+                (ShouldVectorizeHorArith &&
+                 (Opcode == Instruction::Mul || Opcode == Instruction::FMul ||
                   Opcode == Instruction::And || Opcode == Instruction::Or ||
-                Opcode == Instruction::Xor)) ||
+                  Opcode == Instruction::Xor)))) ||
               ((Opcode == Instruction::ICmp || Opcode == Instruction::FCmp) &&
                (Kind == RK_Min || Kind == RK_Max)) ||
               (Opcode == Instruction::ICmp &&
                (Kind == RK_UMin || Kind == RK_UMax)));
     }
 
     /// Creates reduction operation with the current opcode.
     Value *createOp(IRBuilder<> &Builder, const Twine &Name) const {

test/Transforms/SLPVectorizer/X86/PR39774.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -slp-vectorizer -S < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=skylake -slp-threshold=-7 | FileCheck %s --check-prefixes=ALL,CHECK
-; RUN: opt -slp-vectorizer -S < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=skylake -slp-threshold=-8 -slp-min-tree-size=6 | FileCheck %s --check-prefixes=ALL,FORCE_REDUCTION
+; RUN: opt -slp-vectorizer -slp-vectorize-hor -slp-vectorize-hor-arith=true -S < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=skylake -slp-threshold=-7 | FileCheck %s --check-prefixes=ALL,CHECK
+; RUN: opt -slp-vectorizer -slp-vectorize-hor -slp-vectorize-hor-arith=true -S < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=skylake -slp-threshold=-8 -slp-min-tree-size=6 | FileCheck %s --check-prefixes=ALL,FORCE_REDUCTION
 
 define void @Test(i32) {
 ; ALL-LABEL: @Test(
 ; ALL-NEXT:  entry:
 ; ALL-NEXT:    br label [[LOOP:%.*]]
 ; ALL:       loop:
 ; ALL-NEXT:    [[TMP1:%.*]] = phi <2 x i32> [ [[TMP11:%.*]], [[LOOP]] ], [ zeroinitializer, [[ENTRY:%.*]] ]
 ; ALL-NEXT:    [[SHUFFLE:%.*]] = shufflevector <2 x i32> [[TMP1]], <2 x i32> undef, <8 x i32> <i32 0, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1>

test/Transforms/SLPVectorizer/X86/reduction_unrolled.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -slp-vectorizer -slp-vectorize-hor -S -mtriple=x86_64-unknown-linux-gnu -mcpu=bdver2 -debug < %s 2>&1 | FileCheck %s --check-prefixes=CHECK,AVX
-; RUN: opt -slp-vectorizer -slp-vectorize-hor -S -mtriple=x86_64-unknown-linux-gnu -mcpu=core2 -debug < %s 2>&1 | FileCheck %s --check-prefixes=CHECK,SSE
+; RUN: opt -slp-vectorizer -slp-vectorize-hor -slp-vectorize-hor-arith=true -S -mtriple=x86_64-unknown-linux-gnu -mcpu=bdver2 -debug < %s 2>&1 | FileCheck %s --check-prefixes=CHECK,AVX
+; RUN: opt -slp-vectorizer -slp-vectorize-hor -slp-vectorize-hor-arith=true -S -mtriple=x86_64-unknown-linux-gnu -mcpu=core2 -debug < %s 2>&1 | FileCheck %s --check-prefixes=CHECK,SSE
 ; REQUIRES: asserts
 
 ; int test_add(unsigned int *p) {
 ;   int result = 0;
 ;   for (int i = 0; i < 8; i++)
 ;     result += p[i];
 ;   return result;
 ; }