[SLP] Make computeExtractCost() more conservative by checking for non-extract values

[vporpo]

Summary

This function will currently checks for consecutive extracts but it won't check:
a. If these extracts have the same operand, and
b. If any of the values is not an extract.
In either of these cases the cost should be higher because it would require
additional instructions to move the data into a common register.

This patch adds the checks for the non-extract value and for common operand.

[ABataev]

This is not correct. There are 2 reasons for the vectorization in this: 1. InsertElement cost to position 0 is 0, though it should be non-zero. 2. The cost of the node with the same values as another gather node must be 0, currently it is -1. I have a patch, will send it for review in couple hours.

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

computeExtractCost(ArrayRef<Value *> VL, FixedVectorType *VecTy,
                    ArrayRef<int> Mask, TargetTransformInfo &TTI) {
   unsigned NumOfParts = TTI.getNumberOfParts(VecTy);
 
   if (ShuffleKind != TargetTransformInfo::SK_PermuteSingleSrc || !NumOfParts ||
       VecTy->getNumElements() < NumOfParts)
     return TTI.getShuffleCost(ShuffleKind, VecTy, Mask);
 
   bool AllConsecutive = true;
+  auto IsExtract = [](Value *V) {
+    return isa<ExtractElementInst>(V) || isa<ExtractValueInst>(V);
+  };
+  bool ExtractFromCommonOperand = true;
   unsigned EltsPerVector = VecTy->getNumElements() / NumOfParts;
   unsigned Idx = -1;
   InstructionCost Cost = 0;
 
-  // Process extracts in blocks of EltsPerVector to check if the source vector
-  // operand can be re-used directly. If not, add the cost of creating a shuffle
-  // to extract the values into a vector register.
+  auto GetExtractOperand = [](Value *V) -> Value * {
+    if (auto *EEI = dyn_cast<ExtractElementInst>(V))
+      return EEI->getVectorOperand();
+    if (auto *EVI = dyn_cast<ExtractValueInst>(V))
+      return EVI->getAggregateOperand();
+    llvm_unreachable("Expected ExtractElement or ExtractValue");
+  };
+  Value *ExtractOperand0 = IsExtract(VL[0]) ? GetExtractOperand(VL[0]) : VL[0];
+
+  // Process extracts in blocks of EltsPerVector to check if the source
+  // vector operand can be re-used directly. If not, add the cost of
+  // creating a shuffle to extract the values into a vector register.
   SmallVector<int> RegMask(EltsPerVector, UndefMaskElem);
   for (auto *V : VL) {
     ++Idx;
+    // Check if we extract from a single common operand.
+    Value *Operand = IsExtract(V) ? GetExtractOperand(V) : V;
+    if (Operand != ExtractOperand0)
+      ExtractFromCommonOperand = false;
 
     // Reached the start of a new vector registers.
     if (Idx % EltsPerVector == 0) {
       RegMask.assign(EltsPerVector, UndefMaskElem);
       AllConsecutive = true;
       continue;
     }
 
     // Need to exclude undefs from analysis.
     if (isa<UndefValue>(V) || Mask[Idx] == UndefMaskElem)
       continue;
 
     // Check all extracts for a vector register on the target directly
     // extract values in order.
     unsigned CurrentIdx = *getExtractIndex(cast<Instruction>(V));
     if (!isa<UndefValue>(VL[Idx - 1]) && Mask[Idx - 1] != UndefMaskElem) {
       unsigned PrevIdx = *getExtractIndex(cast<Instruction>(VL[Idx - 1]));
       AllConsecutive &= PrevIdx + 1 == CurrentIdx &&
                         CurrentIdx % EltsPerVector == Idx % EltsPerVector;
       RegMask[Idx % EltsPerVector] = CurrentIdx % EltsPerVector;
     }
 
-    if (AllConsecutive)
+    // If all are consecutive and are all extracts from the same operand then we
+    // can reuse that operand.
+    if (AllConsecutive && ExtractFromCommonOperand)
       continue;
 
     // Skip all indices, except for the last index per vector block.
     if ((Idx + 1) % EltsPerVector != 0 && Idx + 1 != VL.size())
       continue;
 
     // If we have a series of extracts which are not consecutive and hence
     // cannot re-use the source vector register directly, compute the shuffle

llvm/test/Transforms/SLPVectorizer/X86/buildvector-float-and-extract-lane1.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
 ; RUN: opt -passes=slp-vectorizer -mtriple=-x86_64-grtev4-linux-gnu -S -mcpu=corei7-avx -pass-remarks-output=%t < %s | FileCheck %s
 ; RUN: FileCheck --input-file=%t --check-prefix=YAML %s
 
 ; Building a vector of a float (lane 0) and an extract from lane 1 (lane 1)
 ; should have a cost of +1 because we need an extra instruction to combine
 ; both into a single vector.
 ; So this code should not be vectorized.
 
 ; YAML: --- !Passed
 ; YAML: Pass:            slp-vectorizer
 ; YAML: Name:            VectorizedList
 ; YAML: Function:        test
 ; YAML: Args:
 ; YAML:   - String:          'SLP vectorized with cost '
-; YAML:   - Cost:            '-2'
+; YAML:   - Cost:            '-1'
 ; YAML:   - String:          ' and with tree size '
 ; YAML:   - TreeSize:        '3'
 ; YAML: ...
 define void @test(<4 x float> %vec, float %a, float %b, ptr %ptr) {
 ; CHECK-LABEL: define void @test
 ; CHECK-SAME: (<4 x float> [[VEC:%.*]], float [[A:%.*]], float [[B:%.*]], ptr [[PTR:%.*]]) #[[ATTR0:[0-9]+]] {
 ; CHECK-NEXT:    [[FADD:%.*]] = fadd float [[A]], [[B]]
 ; CHECK-NEXT:    [[TMP1:%.*]] = shufflevector <4 x float> [[VEC]], <4 x float> poison, <2 x i32> <i32 undef, i32 1>