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

Tue May 16 10:03:00 PDT 2017

Hi Davide,

Sorry for getting back to you late. Got cold and was virtually away during
the weekend and yesterday.

None of these builders is mine, and none of them is in the LLVM Lab. You
would have to contact the owners directly to see if the remote access could
be arranged.
So, it is good that you now have a way to reproduce the problem locally.

Thanks

Galina

On Fri, May 12, 2017 at 8:03 PM, Davide Italiano <davide at freebsd.org> wrote:

> Galina,
> does this test on any of the bots you own? If so, can I access it to
> debug/take a look at the logs/etc.. ?
> I tried very hard to reproduce the problem locally without luck.
>
> On Fri, May 12, 2017 at 8:01 PM, Davide Italiano <davide at freebsd.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.
> >
> > --
> > 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
>
>
>
> --
> Davide
>
> "There are no solved problems; there are only problems that are more
> or less solved" -- Henri Poincare
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170516/320fb96d/attachment.html>