[llvm-branch-commits] [llvm-branch] r228940 - Merging r228899:

[Hans Wennborg]

Author: hans
Date: Thu Feb 12 11:51:17 2015
New Revision: 228940

URL: http://llvm.org/viewvc/llvm-project?rev=228940&view=rev
Log:
Merging r228899:
------------------------------------------------------------------------
r228899 | chandlerc | 2015-02-11 18:30:56 -0800 (Wed, 11 Feb 2015) | 28 lines

[slp] Fix a nasty bug in the SLP vectorizer that Joerg pointed out.
Apparently some code finally started to tickle this after my
canonicalization changes to instcombine.

The bug stems from trying to form a vector type out of scalars that
aren't compatible at all. In this example, from x86_mmx values. The code
in the vectorizer that checks for reasonable types whas checking for
aggregates or vectors, but there are lots of other types that should
just never reach the vectorizer.

Debugging this was made more confusing by the lie in an assert in
VectorType::get() -- it isn't that the types are *primitive*. The types
must be integer, pointer, or floating point types. No other types are
allowed.

I've improved the assert and added a helper to the vectorizer to handle
the element type validity checks. It now re-uses the VectorType static
function and then further excludes weird target-specific types that we
probably shouldn't be touching here (x86_fp80 and ppc_fp128). Neither of
these are really reachable anyways (neither 80-bit nor 128-bit things
will get vectorized) but it seems better to just eagerly exclude such
nonesense.

I've added a test case, but while it definitely covers two of the paths
through this code there may be more paths that would benefit from test
coverage. I'm not familiar enough with the SLP vectorizer to synthesize
test cases for all of these, but was able to update the code itself by
inspection.
------------------------------------------------------------------------

Added:
    llvm/branches/release_36/test/Transforms/SLPVectorizer/X86/bad_types.ll
      - copied unchanged from r228899, llvm/trunk/test/Transforms/SLPVectorizer/X86/bad_types.ll
Modified:
    llvm/branches/release_36/   (props changed)
    llvm/branches/release_36/lib/IR/Type.cpp
    llvm/branches/release_36/lib/Transforms/Vectorize/SLPVectorizer.cpp

Propchange: llvm/branches/release_36/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Feb 12 11:51:17 2015
@@ -1,3 +1,3 @@
 /llvm/branches/Apple/Pertwee:110850,110961
 /llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,226023,226029,226044,226046,226048,226058,226075,226170-226171,226182,226473,226664,226708,226711,226755,226809,227005,227085,227250,227260-227261,227290,227294,227299,227319,227339,227491,227584,227603,227628,227670,227809,227815,227903,227934,227972,227983,228049,228129,228168,228331,228411,228444,228490,228500,228507,228518,228525,228565,228656,228760-228761,228793
+/llvm/trunk:155241,226023,226029,226044,226046,226048,226058,226075,226170-226171,226182,226473,226664,226708,226711,226755,226809,227005,227085,227250,227260-227261,227290,227294,227299,227319,227339,227491,227584,227603,227628,227670,227809,227815,227903,227934,227972,227983,228049,228129,228168,228331,228411,228444,228490,228500,228507,228518,228525,228565,228656,228760-228761,228793,228899

Modified: llvm/branches/release_36/lib/IR/Type.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_36/lib/IR/Type.cpp?rev=228940&r1=228939&r2=228940&view=diff
==============================================================================
--- llvm/branches/release_36/lib/IR/Type.cpp (original)
+++ llvm/branches/release_36/lib/IR/Type.cpp Thu Feb 12 11:51:17 2015
@@ -708,9 +708,10 @@ VectorType::VectorType(Type *ElType, uns
 VectorType *VectorType::get(Type *elementType, unsigned NumElements) {
   Type *ElementType = const_cast<Type*>(elementType);
   assert(NumElements > 0 && "#Elements of a VectorType must be greater than 0");
-  assert(isValidElementType(ElementType) &&
-         "Elements of a VectorType must be a primitive type");
-  
+  assert(isValidElementType(ElementType) && "Element type of a VectorType must "
+                                            "be an integer, floating point, or "
+                                            "pointer type.");
+
   LLVMContextImpl *pImpl = ElementType->getContext().pImpl;
   VectorType *&Entry = ElementType->getContext().pImpl
     ->VectorTypes[std::make_pair(ElementType, NumElements)];

Modified: llvm/branches/release_36/lib/Transforms/Vectorize/SLPVectorizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_36/lib/Transforms/Vectorize/SLPVectorizer.cpp?rev=228940&r1=228939&r2=228940&view=diff
==============================================================================
--- llvm/branches/release_36/lib/Transforms/Vectorize/SLPVectorizer.cpp (original)
+++ llvm/branches/release_36/lib/Transforms/Vectorize/SLPVectorizer.cpp Thu Feb 12 11:51:17 2015
@@ -75,6 +75,18 @@ static const unsigned MinVecRegSize = 12
 
 static const unsigned RecursionMaxDepth = 12;
 
+/// \brief Predicate for the element types that the SLP vectorizer supports.
+///
+/// The most important thing to filter here are types which are invalid in LLVM
+/// vectors. We also filter target specific types which have absolutely no
+/// meaningful vectorization path such as x86_fp80 and ppc_f128. This just
+/// avoids spending time checking the cost model and realizing that they will
+/// be inevitably scalarized.
+static bool isValidElementType(Type *Ty) {
+  return VectorType::isValidElementType(Ty) && !Ty->isX86_FP80Ty() &&
+         !Ty->isPPC_FP128Ty();
+}
+
 /// \returns the parent basic block if all of the instructions in \p VL
 /// are in the same block or null otherwise.
 static BasicBlock *getSameBlock(ArrayRef<Value *> VL) {
@@ -1216,7 +1228,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Val
       Type *SrcTy = VL0->getOperand(0)->getType();
       for (unsigned i = 0; i < VL.size(); ++i) {
         Type *Ty = cast<Instruction>(VL[i])->getOperand(0)->getType();
-        if (Ty != SrcTy || Ty->isAggregateType() || Ty->isVectorTy()) {
+        if (Ty != SrcTy || !isValidElementType(Ty)) {
           BS.cancelScheduling(VL);
           newTreeEntry(VL, false);
           DEBUG(dbgs() << "SLP: Gathering casts with different src types.\n");
@@ -3130,7 +3142,7 @@ unsigned SLPVectorizer::collectStores(Ba
 
     // Check that the pointer points to scalars.
     Type *Ty = SI->getValueOperand()->getType();
-    if (Ty->isAggregateType() || Ty->isVectorTy())
+    if (!isValidElementType(Ty))
       continue;
 
     // Find the base pointer.
@@ -3171,7 +3183,7 @@ bool SLPVectorizer::tryToVectorizeList(A
 
   for (int i = 0, e = VL.size(); i < e; ++i) {
     Type *Ty = VL[i]->getType();
-    if (Ty->isAggregateType() || Ty->isVectorTy())
+    if (!isValidElementType(Ty))
       return false;
     Instruction *Inst = dyn_cast<Instruction>(VL[i]);
     if (!Inst || Inst->getOpcode() != Opcode0)
@@ -3391,7 +3403,7 @@ public:
       return false;
 
     Type *Ty = B->getType();
-    if (Ty->isVectorTy())
+    if (!isValidElementType(Ty))
       return false;
 
     ReductionOpcode = B->getOpcode();