[llvm] r302905 - [NewGVN] Don't incorrectly reset the memory leader.

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Fri May 12 22:12:43 PDT 2017 

On Fri, May 12, 2017 at 8:01 PM, Davide Italiano via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> I think this test is just exposing some underlying
> non-determinism/problem, but I can't take a look right now
> unforunately.
> Given NewGVN is not the default and the change is correct, I'm
> XFAIL'ing this for now, and I'll debug over the weekend (or monday at
> last). Sorry if this caused troubles, but I'm as puzzled as you are.
>


XFAIL is not a good choice for a flaky test, because if it passes then the
overall test run will still fail (with an unexpected passing test).

-- Sean Silva



>
> --
> Davide
>
> On Fri, May 12, 2017 at 5:07 PM, Craig Topper <craig.topper at gmail.com>
> wrote:
> > This test seems to be randomly failing on the bots for example
> >
> > http://lab.llvm.org:8011/builders/clang-x86_64-linux-
> selfhost-modules/builds/5700
> >
> >
> >
> > http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/5250
> >
> >
> >
> > http://lab.llvm.org:8011/builders/llvm-hexagon-elf/builds/6165
> >
> >
> > But it seems to pass on the previous build and the next build. But you
> can
> > go back through early builds and find a failure.  For example it also
> failed
> > on build 6160 on the hexagon bot above, but it passed on 6161-6164.
> >
> >
> >
> > ~Craig
> >
> > On Fri, May 12, 2017 at 8:22 AM, Davide Italiano via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >>
> >> Author: davide
> >> Date: Fri May 12 10:22:45 2017
> >> New Revision: 302905
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=302905&view=rev
> >> Log:
> >> [NewGVN] Don't incorrectly reset the memory leader.
> >>
> >> This code was missing a check for stores, so we were thinking the
> >> congruency class didn't have any memory members, and reset the
> >> memory leader.
> >>
> >> Differential Revision:  https://reviews.llvm.org/D33056
> >>
> >> Added:
> >>     llvm/trunk/test/Transforms/NewGVN/pr32934.ll
> >> Modified:
> >>     llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
> >>
> >> Modified: llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> Transforms/Scalar/NewGVN.cpp?rev=302905&r1=302904&r2=302905&view=diff
> >>
> >> ============================================================
> ==================
> >> --- llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp (original)
> >> +++ llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp Fri May 12 10:22:45
> 2017
> >> @@ -1409,7 +1409,7 @@ bool NewGVN::setMemoryClass(const Memory
> >>          NewClass->memory_insert(MP);
> >>          // This may have killed the class if it had no non-memory
> members
> >>          if (OldClass->getMemoryLeader() == From) {
> >> -          if (OldClass->memory_empty()) {
> >> +          if (OldClass->definesNoMemory()) {
> >>              OldClass->setMemoryLeader(nullptr);
> >>            } else {
> >>              OldClass->setMemoryLeader(getNextMemoryLeader(OldClass));
> >>
> >> Added: llvm/trunk/test/Transforms/NewGVN/pr32934.ll
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> Transforms/NewGVN/pr32934.ll?rev=302905&view=auto
> >>
> >> ============================================================
> ==================
> >> --- llvm/trunk/test/Transforms/NewGVN/pr32934.ll (added)
> >> +++ llvm/trunk/test/Transforms/NewGVN/pr32934.ll Fri May 12 10:22:45
> 2017
> >> @@ -0,0 +1,68 @@
> >> +; RUN: opt -S -newgvn %s | FileCheck %s
> >> +
> >> +; CHECK: define void @tinkywinky() {
> >> +; CHECK-NEXT: entry:
> >> +; CHECK-NEXT:   %d = alloca i32, align 4
> >> +; CHECK-NEXT:   store i32 0, i32* null, align 4
> >> +; CHECK-NEXT:   br label %for.cond
> >> +; CHECK: for.cond:                                         ; preds =
> >> %if.end, %entry
> >> +; CHECK-NEXT:   %0 = load i32, i32* null, align 4
> >> +; CHECK-NEXT:   %cmp = icmp slt i32 %0, 1
> >> +; CHECK-NEXT:   br i1 %cmp, label %for.body, label %while.cond
> >> +; CHECK: for.body:                                         ; preds =
> >> %for.cond
> >> +; CHECK-NEXT:   %1 = load i32, i32* @a, align 4
> >> +; CHECK-NEXT:   store i32 %1, i32* %d, align 4
> >> +; CHECK-NEXT:   br label %L
> >> +; CHECK: L:                                                ; preds =
> >> %if.then, %for.body
> >> +; CHECK-NEXT:   %tobool = icmp ne i32 %1, 0
> >> +; CHECK-NEXT:   br i1 %tobool, label %if.then, label %if.end
> >> +; CHECK: if.then:                                          ; preds = %L
> >> +; CHECK-NEXT:   call void (i8*, ...) @printf(i8* getelementptr inbounds
> >> ([2 x i8], [2 x i8]* @patatino, i32 0, i32 0))
> >> +; CHECK-NEXT:   br label %L
> >> +; CHECK: if.end:                                           ; preds = %L
> >> +; CHECK-NEXT:   br label %for.cond
> >> +; CHECK: while.cond:                                       ; preds =
> >> %while.body, %for.cond
> >> +; CHECK-NEXT:   br i1 undef, label %while.body, label %while.end
> >> +; CHECK: while.body:                                       ; preds =
> >> %while.cond
> >> +; CHECK-NEXT:   call void (i8*, ...) @printf(i8* getelementptr inbounds
> >> ([2 x i8], [2 x i8]* @patatino, i32 0, i32 0))
> >> +; CHECK-NEXT:   br label %while.cond
> >> +; CHECK: while.end:
> >> +; CHECK-NEXT:   %2 = load i32, i32* @a, align 4
> >> +; CHECK-NEXT:   store i32 %2, i32* undef, align 4
> >> +; CHECK-NEXT:   ret void
> >> +
> >> + at a = external global i32, align 4
> >> + at patatino = external unnamed_addr constant [2 x i8], align 1
> >> +define void @tinkywinky() {
> >> +entry:
> >> +  %d = alloca i32, align 4
> >> +  store i32 0, i32* null, align 4
> >> +  br label %for.cond
> >> +for.cond:
> >> +  %0 = load i32, i32* null, align 4
> >> +  %cmp = icmp slt i32 %0, 1
> >> +  br i1 %cmp, label %for.body, label %while.cond
> >> +for.body:
> >> +  %1 = load i32, i32* @a, align 4
> >> +  store i32 %1, i32* %d, align 4
> >> +  br label %L
> >> +L:
> >> +  %2 = load i32, i32* %d, align 4
> >> +  %tobool = icmp ne i32 %2, 0
> >> +  br i1 %tobool, label %if.then, label %if.end
> >> +if.then:
> >> +  call void (i8*, ...) @printf(i8* getelementptr inbounds ([2 x i8],
> [2 x
> >> i8]* @patatino, i32 0, i32 0))
> >> +  br label %L
> >> +if.end:
> >> +  br label %for.cond
> >> +while.cond:
> >> +  br i1 undef, label %while.body, label %while.end
> >> +while.body:
> >> +  call void (i8*, ...) @printf(i8* getelementptr inbounds ([2 x i8],
> [2 x
> >> i8]* @patatino, i32 0, i32 0))
> >> +  br label %while.cond
> >> +while.end:
> >> +  %3 = load i32, i32* @a, align 4
> >> +  store i32 %3, i32* undef, align 4
> >> +  ret void
> >> +}
> >> +declare void @printf(i8*, ...) #1
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> >
>
>
>
> --
> Davide
>
> "There are no solved problems; there are only problems that are more
> or less solved" -- Henri Poincare
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170512/6a9e1215/attachment.html>

More information about the llvm-commits mailing list