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

[Vasileios Porpodas via Phabricator via llvm-commits]

vporpo created this revision.
vporpo added reviewers: ABataev, fhahn, dmgreen.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
vporpo requested review of this revision.
Herald added subscribers: llvm-commits, pcwang-thead.
Herald added a project: LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148773

Files:
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
  llvm/test/Transforms/SLPVectorizer/X86/buildvector-float-and-extract-lane1.ll


Index: llvm/test/Transforms/SLPVectorizer/X86/buildvector-float-and-extract-lane1.ll
===================================================================
--- llvm/test/Transforms/SLPVectorizer/X86/buildvector-float-and-extract-lane1.ll
+++ llvm/test/Transforms/SLPVectorizer/X86/buildvector-float-and-extract-lane1.ll
@@ -13,7 +13,7 @@
 ; 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: ...
Index: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
===================================================================
--- llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -6300,16 +6300,33 @@
     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) {
@@ -6332,7 +6349,9 @@
       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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148773.515178.patch
Type: text/x-patch
Size: 2808 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230420/ba5404c5/attachment.bin>