[PATCH] D41139: [SLPVectorizer] Don't ignore scalar extraction instructions of aggregate value

[Guozhi Wei via Phabricator via llvm-commits]

Carrot created this revision.
Carrot added reviewers: hfinkel, mkuper, RKSimon.
Herald added subscribers: nemanjai, rengolin.

For code like

struct S {

  void* p;
  void* q;

};

static S kS0;

S getS() {

  return kS0;

}

LLVM generate following instructions for ppc

  lxvx 0, 0, 3
  mfvsrld 3, 0
  mfvsrd 4, 0

Ideal result should be just

  ld 3, 0(4)
  ld 4, 8(4)

The problem is in SLPVectorizer, the vector build instructions (insertvalue for aggregate type) is passed to BoUpSLP.buildTree, it is treated as UserIgnoreList, so later in cost estimation, the cost of these instructions are not counted. 
For aggregate value, later usage are more likely to be done in integer registers, either used as individual scalars or used as a whole for function call or return value. So for vectorization of aggregate value, the scalar extraction instructions are required in cost estimation.


https://reviews.llvm.org/D41139

Files:
  include/llvm/Transforms/Vectorize/SLPVectorizer.h
  lib/Transforms/Vectorize/SLPVectorizer.cpp
  test/Transforms/SLPVectorizer/PowerPC/aggregate.ll


Index: test/Transforms/SLPVectorizer/PowerPC/aggregate.ll
===================================================================
--- test/Transforms/SLPVectorizer/PowerPC/aggregate.ll
+++ test/Transforms/SLPVectorizer/PowerPC/aggregate.ll
@@ -0,0 +1,19 @@
+; RUN: opt -S -mtriple=powerpc64-linux-gnu -mcpu=pwr9 -mattr=+vsx -slp-vectorizer < %s | FileCheck %s
+
+%struct.S = type { i8*, i8* }
+
+ at kS0 = common global %struct.S zeroinitializer, align 8
+
+define { i64, i64 } @getS() {
+entry:
+  %0 = load i64, i64* bitcast (%struct.S* @kS0 to i64*), align 8
+  %1 = load i64, i64* bitcast (i8** getelementptr inbounds (%struct.S, %struct.S* @kS0, i64 0, i32 1) to i64*), align 8
+  %2 = insertvalue { i64, i64 } undef, i64 %0, 0
+  %3 = insertvalue { i64, i64 } %2, i64 %1, 1
+  ret { i64, i64 } %3
+}
+
+; CHECK: load i64
+; CHECK-NOT: load <2 x i64>
+; CHECK-NOT: extractelement
+
Index: lib/Transforms/Vectorize/SLPVectorizer.cpp
===================================================================
--- lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -4421,7 +4421,8 @@
 
 bool SLPVectorizerPass::tryToVectorizeList(ArrayRef<Value *> VL, BoUpSLP &R,
                                            ArrayRef<Value *> BuildVector,
-                                           bool AllowReorder) {
+                                           bool AllowReorder,
+                                           bool NeedExtraction) {
   if (VL.size() < 2)
     return false;
 
@@ -4515,11 +4516,12 @@
                    << "\n");
       ArrayRef<Value *> Ops = VL.slice(I, OpsWidth);
 
+      ArrayRef<Value *> EmptyArray;
       ArrayRef<Value *> BuildVectorSlice;
       if (!BuildVector.empty())
         BuildVectorSlice = BuildVector.slice(I, OpsWidth);
 
-      R.buildTree(Ops, BuildVectorSlice);
+      R.buildTree(Ops, NeedExtraction ? EmptyArray : BuildVectorSlice);
       // TODO: check if we can allow reordering for more cases.
       if (AllowReorder && R.shouldReorder()) {
         // Conceptually, there is nothing actually preventing us from trying to
@@ -5709,7 +5711,9 @@
     return false;
 
   DEBUG(dbgs() << "SLP: array mappable to vector: " << *IVI << "\n");
-  return tryToVectorizeList(BuildVectorOpds, R, BuildVector, false);
+  // Aggregate value is unlikely to be processed in vector register, we need to
+  // extract scalars into integer registers, so NeedExtraction is set true.
+  return tryToVectorizeList(BuildVectorOpds, R, BuildVector, false, true);
 }
 
 bool SLPVectorizerPass::vectorizeInsertElementInst(InsertElementInst *IEI,
Index: include/llvm/Transforms/Vectorize/SLPVectorizer.h
===================================================================
--- include/llvm/Transforms/Vectorize/SLPVectorizer.h
+++ include/llvm/Transforms/Vectorize/SLPVectorizer.h
@@ -96,11 +96,13 @@
 
   /// \brief Try to vectorize a list of operands.
   /// \@param BuildVector A list of users to ignore for the purpose of
-  ///                     scheduling and that don't need extracting.
+  ///                     scheduling and cost estimation when NeedExtraction
+  ///                     is false.
   /// \returns true if a value was vectorized.
   bool tryToVectorizeList(ArrayRef<Value *> VL, slpvectorizer::BoUpSLP &R,
                           ArrayRef<Value *> BuildVector = None,
-                          bool AllowReorder = false);
+                          bool AllowReorder = false,
+                          bool NeedExtraction = false);
 
   /// \brief Try to vectorize a chain that may start at the operands of \p I.
   bool tryToVectorize(Instruction *I, slpvectorizer::BoUpSLP &R);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41139.126637.patch
Type: text/x-patch
Size: 3667 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171212/c9989c72/attachment.bin>