[llvm] r214638 - fix bug 20513 - Crash in SLP Vectorizer

Erik Eckstein eeckstein at apple.com
Sun Aug 3 00:03:38 PDT 2014


Hi Eric,

There is small a comment in the test file, but of course it should be also in the commit message.

The SLPVectorizer crashed because it inserted an instruction (insertelement) after building the tree but before the actual scheduling of a block.
I scheduled a block the first time the block was visited during vectorizeTree (which was OK the most of the time).
But it can happen that vectorizeTree does insert instructions also in _other_ blocks than the block of the current visited bundle.
So the fix was to schedule _all_ blocks prior to vectorizeTree.

I will work on improving my commit messages :-)

Thanks,
Erik

On 03 Aug 2014, at 00:53, Eric Christopher <echristo at gmail.com> wrote:

> Can you elaborate in your commit message? What was the problem? How does this fix it?
> 
> On Aug 2, 2014 12:51 PM, "Erik Eckstein" <eeckstein at apple.com> wrote:
> Author: eeckstein
> Date: Sat Aug  2 14:39:42 2014
> New Revision: 214638
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=214638&view=rev
> Log:
> fix bug 20513 - Crash in SLP Vectorizer
> 
> 
> Added:
>     llvm/trunk/test/Transforms/SLPVectorizer/X86/crash_scheduling.ll
> Modified:
>     llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp
> 
> Modified: llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp?rev=214638&r1=214637&r2=214638&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp (original)
> +++ llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp Sat Aug  2 14:39:42 2014
> @@ -806,7 +806,7 @@ private:
> 
>    /// Performs the "real" scheduling. Done before vectorization is actually
>    /// performed in a basic block.
> -  void scheduleBlock(BasicBlock *BB);
> +  void scheduleBlock(BlockScheduling *BS);
> 
>    /// List of users to ignore during scheduling and that don't need extracting.
>    ArrayRef<Value *> UserIgnoreList;
> @@ -1741,8 +1741,6 @@ Value *BoUpSLP::vectorizeTree(TreeEntry
>      setInsertPointAfterBundle(E->Scalars);
>      return Gather(E->Scalars, VecTy);
>    }
> -  BasicBlock *BB = VL0->getParent();
> -  scheduleBlock(BB);
> 
>    unsigned Opcode = getSameOpcode(E->Scalars);
> 
> @@ -2076,6 +2074,12 @@ Value *BoUpSLP::vectorizeTree(TreeEntry
>  }
> 
>  Value *BoUpSLP::vectorizeTree() {
> +
> +  // All blocks must be scheduled before any instructions are inserted.
> +  for (auto &BSIter : BlocksSchedules) {
> +    scheduleBlock(BSIter.second.get());
> +  }
> +
>    Builder.SetInsertPoint(F->getEntryBlock().begin());
>    vectorizeTree(&VectorizableTree[0]);
> 
> @@ -2548,12 +2552,12 @@ void BoUpSLP::BlockScheduling::resetSche
>    ReadyInsts.clear();
>  }
> 
> -void BoUpSLP::scheduleBlock(BasicBlock *BB) {
> -  DEBUG(dbgs() << "SLP: schedule block " << BB->getName() << "\n");
> -
> -  BlockScheduling *BS = BlocksSchedules[BB].get();
> -  if (!BS || !BS->ScheduleStart)
> +void BoUpSLP::scheduleBlock(BlockScheduling *BS) {
> +
> +  if (!BS->ScheduleStart)
>      return;
> +
> +  DEBUG(dbgs() << "SLP: schedule block " << BS->BB->getName() << "\n");
> 
>    BS->resetSchedule();
> 
> @@ -2598,8 +2602,8 @@ void BoUpSLP::scheduleBlock(BasicBlock *
>      while (BundleMember) {
>        Instruction *pickedInst = BundleMember->Inst;
>        if (LastScheduledInst->getNextNode() != pickedInst) {
> -        BB->getInstList().remove(pickedInst);
> -        BB->getInstList().insert(LastScheduledInst, pickedInst);
> +        BS->BB->getInstList().remove(pickedInst);
> +        BS->BB->getInstList().insert(LastScheduledInst, pickedInst);
>        }
>        LastScheduledInst = pickedInst;
>        BundleMember = BundleMember->NextInBundle;
> 
> Added: llvm/trunk/test/Transforms/SLPVectorizer/X86/crash_scheduling.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SLPVectorizer/X86/crash_scheduling.ll?rev=214638&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/SLPVectorizer/X86/crash_scheduling.ll (added)
> +++ llvm/trunk/test/Transforms/SLPVectorizer/X86/crash_scheduling.ll Sat Aug  2 14:39:42 2014
> @@ -0,0 +1,47 @@
> +; RUN: opt < %s -basicaa -slp-vectorizer -S -mtriple=x86_64-apple-macosx10.8.0 -mcpu=corei7
> +
> +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-apple-darwin13.3.0"
> +
> +define void @_foo(double %p1, double %p2, double %p3) #0 {
> +entry:
> +  %tab1 = alloca [256 x i32], align 16
> +  %tab2 = alloca [256 x i32], align 16
> +  br label %bb1
> +
> +
> +bb1:
> +  %mul19 = fmul double %p1, 1.638400e+04
> +  %mul20 = fmul double %p3, 1.638400e+04
> +  %add = fadd double %mul20, 8.192000e+03
> +  %mul21 = fmul double %p2, 1.638400e+04
> +  ; The SLPVectorizer crashed when scheduling this block after it inserted an
> +  ; insertelement instruction (during vectorizing the for.body block) at this position.
> +  br label %for.body
> +
> +for.body:
> +  %indvars.iv266 = phi i64 [ 0, %bb1 ], [ %indvars.iv.next267, %for.body ]
> +  %t.0259 = phi double [ 0.000000e+00, %bb1 ], [ %add27, %for.body ]
> +  %p3.addr.0258 = phi double [ %add, %bb1 ], [ %add28, %for.body ]
> +  %vecinit.i.i237 = insertelement <2 x double> undef, double %t.0259, i32 0
> +  %x13 = tail call i32 @_xfn(<2 x double> %vecinit.i.i237) #2
> +  %arrayidx = getelementptr inbounds [256 x i32]* %tab1, i64 0, i64 %indvars.iv266
> +  store i32 %x13, i32* %arrayidx, align 4, !tbaa !4
> +  %vecinit.i.i = insertelement <2 x double> undef, double %p3.addr.0258, i32 0
> +  %x14 = tail call i32 @_xfn(<2 x double> %vecinit.i.i) #2
> +  %arrayidx26 = getelementptr inbounds [256 x i32]* %tab2, i64 0, i64 %indvars.iv266
> +  store i32 %x14, i32* %arrayidx26, align 4, !tbaa !4
> +  %add27 = fadd double %mul19, %t.0259
> +  %add28 = fadd double %mul21, %p3.addr.0258
> +  %indvars.iv.next267 = add nuw nsw i64 %indvars.iv266, 1
> +  %exitcond = icmp eq i64 %indvars.iv.next267, 256
> +  br i1 %exitcond, label %return, label %for.body
> +
> +return:
> +  ret void
> +}
> +
> +declare i32 @_xfn(<2 x double>) #4
> +
> +!3 = metadata !{metadata !"int", metadata !4, i64 0}
> +!4 = metadata !{metadata !3, metadata !3, i64 0}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140803/a42a6770/attachment.html>


More information about the llvm-commits mailing list