[PATCH] D14670: Fix bug 25440: GVN assertion after coercing loads

[Daniel Berlin via llvm-commits]

I'm almost positive it's because of this code:

  Value *Val = nullptr;
  if (DT->dominates(Vals.BB, BB)) {
    Val = Vals.Val;
    if (isa<Constant>(Val)) return Val;
  }

  LeaderTableEntry* Next = Vals.Next;
  while (Next) {
    if (DT->dominates(Next->BB, BB)) {
      if (isa<Constant>(Next->Val)) return Next->Val;
      if (!Val) Val = Next->Val;
    }

    Next = Next->Next;
  }



IE If it's in the same block as anything in the leader table, it assumes
they are ordered from first to last. but they may not be once you insert
new instructions.



On Fri, Nov 13, 2015 at 9:26 PM, <weimingz at codeaurora.org> wrote:

> Hmmm. That's interesting.
> I will dig more and try to find out why.
>
>
> > Err, then the algorithm is broken somewhere.
> >
> > This is definitely not the right fix from what i've seen so far.
> > Can you see what decides to insert this?
> >
> > I suspect, based on your failure mode, that the leader table is not
> > ordered
> > locally by dominance, only by bb, and so when it calls findLeader, it
> > grabs
> > the first thing it finds, and does a broken thing.
> >
> > It would only work in the processInstruction case because
> > processInstrucion
> > goes in dominance order.
> >
> > If so, that would be bad (and hard to fix well without serious surgery),
> > and we should probably declare it broken, revert your set of patches, and
> > just abort PRE  (IE the other option i suggested where we set
> > success=false
> > and bailing)
> >
> >
> >
> > On Fri, Nov 13, 2015 at 5:30 PM, Weiming Zhao <weimingz at codeaurora.org>
> > wrote:
> >
> >> weimingz added a comment.
> >>
> >> This patch adds a reduced test from SPASS.
> >>
> >> we need the check:
> >>
> >>   if (Num >= NextNum)
> >>           addToLeaderTable(Num, I, I->getParent());
> >>
> >> Otherwise, BB %wile.end (see line 91) will become :
> >>
> >> while.end:                                        ; preds =
> >> %while.cond.16
> >>
> >>   %add.ptr = getelementptr inbounds i8, i8* %v2, i32 undef
> >>   store i8* %add.ptr, i8** @dfg_text, align 4
> >>   %sub.ptr.sub26 = sub i32 0, %0  ==> use here
> >>   %0 = ptrtoint i8* %add.ptr to i32  ==> def here
> >>   switch i32 undef, label %sw.default [
> >>     i32 65, label %while.bodythread-pre-split
> >>
> >>
> >> http://reviews.llvm.org/D14670
> >>
> >>
> >>
> >>
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151113/f2edafda/attachment.html>