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

Mon May 15 12:05:49 PDT 2017

Galina didn't come back to me, so I'll try to reproduce this locally now.

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

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare