[llvm] [SLP] Report the correct operand to getArithmeticInstrCost() when duplicated scalars (PR #174442)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Ryan Buchner (bababuck)

<details>
<summary>Changes</summary>

Before, we were selecting the wrong operand in cases when Scalars contained duplicate values. Stems from #<!-- -->135797.

Using:
`opt -passes=slp-vectorizer -mtriple=riscv64 -mattr=+v t.ll`
```
target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
target triple = "riscv64"

define void @<!-- -->foo(ptr noalias %A, ptr noalias %B) {
entry:
  %0 = load i32, ptr %B
  %add = add nsw i32 %0, 1
  store i32 %add, ptr %A
  %arrayidx.1 = getelementptr inbounds nuw i8, ptr %B, i64 4
  %1 = load i32, ptr %arrayidx.1
  %add.1 = add nsw i32 %1, 1
  %arrayidx2.1 = getelementptr inbounds nuw i8, ptr %A, i64 4
  store i32 %add.1, ptr %arrayidx2.1
  %arrayidx.2 = getelementptr inbounds nuw i8, ptr %B, i64 8
  %2 = load i32, ptr %arrayidx.2
  %add.2 = add nsw i32 %2, 1
  %arrayidx2.2 = getelementptr inbounds nuw i8, ptr %A, i64 8
  store i32 %add.2, ptr %arrayidx2.2
  %arrayidx.3 = getelementptr inbounds nuw i8, ptr %B, i64 12

  %arrayidx2.3 = getelementptr inbounds nuw i8, ptr %A, i64 12

  store i32 %add, ptr %arrayidx2.3
  %arrayidx.4 = getelementptr inbounds nuw i8, ptr %B, i64 16
  %4 = load i32, ptr %arrayidx.4
  %add.4 = add nsw i32 %4, 1
  %arrayidx2.4 = getelementptr inbounds nuw i8, ptr %A, i64 16
  store i32 %add.4, ptr %arrayidx2.4
  %arrayidx.5 = getelementptr inbounds nuw i8, ptr %B, i64 20
  %5 = load i32, ptr %arrayidx.5
  %add.5 = add nsw i32 %5, 1
  %arrayidx2.5 = getelementptr inbounds nuw i8, ptr %A, i64 20
  store i32 %add.5, ptr %arrayidx2.5
  %arrayidx.6 = getelementptr inbounds nuw i8, ptr %B, i64 24
  %6 = load i32, ptr %arrayidx.6
  %add.6 = add nsw i32 %6, 1
  %arrayidx2.6 = getelementptr inbounds nuw i8, ptr %A, i64 24
  store i32 %add.6, ptr %arrayidx2.6
  %arrayidx.7 = getelementptr inbounds nuw i8, ptr %B, i64 28
  %7 = load i32, ptr %arrayidx.7
  %add.7 = add nsw i32 %7, 1
  %arrayidx2.7 = getelementptr inbounds nuw i8, ptr %A, i64 28
  store i32 %add.7, ptr %arrayidx2.7
  ret void
}
```

The following trace is produced, note the wrong operand is used for `Idx > 2`

Before:
```
GetScalarCost(), Idx=0
UniqueValues[Idx]:   %add = add nsw i32 %0, 1
Op1:   %0 = load i32, ptr %B, align 4
GetScalarCost(), Idx=1
UniqueValues[Idx]:   %add.1 = add nsw i32 %1, 1
Op1:   %1 = load i32, ptr %arrayidx.1, align 4
GetScalarCost(), Idx=2
UniqueValues[Idx]:   %add.2 = add nsw i32 %2, 1
Op1:   %2 = load i32, ptr %arrayidx.2, align 4
GetScalarCost(), Idx=3
UniqueValues[Idx]:   %add.4 = add nsw i32 %3, 1
Op1:   %0 = load i32, ptr %B, align 4
GetScalarCost(), Idx=4
UniqueValues[Idx]:   %add.5 = add nsw i32 %4, 1
Op1:   %3 = load i32, ptr %arrayidx.4, align 4
GetScalarCost(), Idx=5
UniqueValues[Idx]:   %add.6 = add nsw i32 %5, 1
Op1:   %4 = load i32, ptr %arrayidx.5, align 4
GetScalarCost(), Idx=6
UniqueValues[Idx]:   %add.7 = add nsw i32 %6, 1
Op1:   %5 = load i32, ptr %arrayidx.6, align 4
```

After:
```
GetScalarCost(), Idx=0
UniqueValues[Idx]:   %add = add nsw i32 %0, 1
Op1:   %0 = load i32, ptr %B, align 4
GetScalarCost(), Idx=1
UniqueValues[Idx]:   %add.1 = add nsw i32 %1, 1
Op1:   %1 = load i32, ptr %arrayidx.1, align 4
GetScalarCost(), Idx=2
UniqueValues[Idx]:   %add.2 = add nsw i32 %2, 1
Op1:   %2 = load i32, ptr %arrayidx.2, align 4
GetScalarCost(), Idx=3
UniqueValues[Idx]:   %add.4 = add nsw i32 %3, 1
Op1:   %3 = load i32, ptr %arrayidx.4, align 4
GetScalarCost(), Idx=4
UniqueValues[Idx]:   %add.5 = add nsw i32 %4, 1
Op1:   %4 = load i32, ptr %arrayidx.5, align 4
GetScalarCost(), Idx=5
UniqueValues[Idx]:   %add.6 = add nsw i32 %5, 1
Op1:   %5 = load i32, ptr %arrayidx.6, align 4
GetScalarCost(), Idx=6
UniqueValues[Idx]:   %add.7 = add nsw i32 %6, 1
Op1:   %6 = load i32, ptr %arrayidx.7, align 4
```

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


1 Files Affected:

- (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+8-3) 


``````````diff
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 0b3d6220ef556..5bd274a275492 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -14835,7 +14835,11 @@ BoUpSLP::getEntryCost(const TreeEntry *E, ArrayRef<Value *> VectorizedVals,
       E->isAltShuffle() ? (unsigned)Instruction::ShuffleVector : E->getOpcode();
   if (E->CombinedOp != TreeEntry::NotCombinedOp)
     ShuffleOrOp = E->CombinedOp;
-  SmallSetVector<Value *, 16> UniqueValues(VL.begin(), VL.end());
+  SmallSetVector<Value *, 16> UniqueValues;
+  SmallVector<unsigned, 16> UniqueIndexes;
+  for (auto [Idx, V] : enumerate(VL))
+    if (UniqueValues.insert(V))
+      UniqueIndexes.push_back(Idx);
   const unsigned Sz = UniqueValues.size();
   SmallBitVector UsedScalars(Sz, false);
   for (unsigned I = 0; I < Sz; ++I) {
@@ -15351,13 +15355,14 @@ BoUpSLP::getEntryCost(const TreeEntry *E, ArrayRef<Value *> VectorizedVals,
       // We cannot retrieve the operand from UniqueValues[Idx] because an
       // interchangeable instruction may be used. The order and the actual
       // operand might differ from what is retrieved from UniqueValues[Idx].
-      Value *Op1 = E->getOperand(0)[Idx];
+      unsigned Lane = UniqueIndexes[Idx];
+      Value *Op1 = E->getOperand(0)[Lane];
       Value *Op2;
       SmallVector<const Value *, 2> Operands(1, Op1);
       if (isa<UnaryOperator>(UniqueValues[Idx])) {
         Op2 = Op1;
       } else {
-        Op2 = E->getOperand(1)[Idx];
+        Op2 = E->getOperand(1)[Lane];
         Operands.push_back(Op2);
       }
       TTI::OperandValueInfo Op1Info = TTI::getOperandInfo(Op1);

``````````

</details>


https://github.com/llvm/llvm-project/pull/174442