SLPVectorizer to handle GEP with differing constant index types

[JesperAntonsson]

Summary

This is a solution to bug 27617.

Bug description: The SLPVectorizer asserts on encountering GEPs with different index types, such as i8 and i64.

The patch includes a simple relaxation of the assert to allow constants being of different types, along with a regression test that will provoke the unrelaxed assert.

[JesperAntonsson]

Thanks! This is my first contribution and I don't have commit rights. Could you help me commit, please?

BR, Jesper Antonsson

From: Nadav Rotem [mailto:[e-mail]]
Sent: den 26 maj 2016 18:47
To: [e-mail]; Jesper Antonsson
Cc: Jesper Antonsson; [e-mail]; [e-mail]
Subject: Re: [PATCH] D20685: SLPVectorizer to handle GEP with differing constant index types

Looks good to me. Please commit.

[mzolotukhin]

Hi Jesper,

This test doesn't work to me. Could you please verify that this patch is for the current trunk?

Also, some more specific remarks inline.

Thanks,
Michael

test/Transforms/SLPVectorizer/X86/gep_mismatch.ll

2

We usually try to minimize number of passes running on a test. For instance, if the assertion happened in SLPVectorizer, we probably only need slp-vectorizer: -indvars and -O1 can be removed (i.e. you run opt -indvars on the test and the output is the new test that doesn't need -indvars in the command line).

7

I got LLVM ERROR: Unknown specifier in datalayout string on this. Could you please check if it's correct?

40

#3 means that we need attribute #3, but the test contains no attributes at all. Were they lost when you were reducing the test?

[JesperAntonsson]

Corrected test case in accordance to the review comments from Michael Zolotukhin. (Thanks Michael!)

[mzolotukhin]

Hi Jesper,

I committed your patch in rL272206, thank you for fixing it!

Michael

lib/Transforms/Vectorize/SLPVectorizer.cpp

for (int Lane = 0, LE = Entry->Scalars.size(); Lane != LE; ++Lane) {
         ExternalUses.push_back(ExternalUser(Scalar, U, Lane));
       }
     }
   }
 }
 
 
 void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth) {
-  bool SameTy = getSameType(VL); (void)SameTy;
+   bool SameTy = allConstant(VL) || getSameType(VL); (void)SameTy;
   bool isAltShuffle = false;
   assert(SameTy && "Invalid types!");
 
   if (Depth == RecursionMaxDepth) {
     DEBUG(dbgs() << "SLP: Gathering due to max recursion depth.\n");
     newTreeEntry(VL, false);
     return;
   }

test/Transforms/SLPVectorizer/X86/gep_mismatch.ll

+; RUN: opt < %s -S -mcpu=x86-64 -mtriple=x86_64-linux -slp-vectorizer
+
+; This code has GEPs with different index types, which should not
+; matter for the SLPVectorizer.
+
+target triple = "x86_64--linux"
+
+define void @matmul() #0 {
+  br label %bb1
+
+bb1:                                        ; preds = %bb1, %0
+  %ls1.ph = phi float* [ %_tmp1, %bb1 ], [ undef, %0 ]
+  %ls2.ph = phi float* [ %_tmp2, %bb1 ], [ undef, %0 ]
+  store float undef, float* %ls1.ph
+  %_tmp1 = getelementptr float, float* %ls1.ph, i32 1
+  %_tmp2 = getelementptr float, float* %ls2.ph, i64 4
+  br i1 false, label %bb1, label %bb2
+
+bb2:                                        ; preds = %bb1
+  ret void
+}
+
+attributes #0 = { "target-cpu"="x86-64" }