[llvm] r300200 - Re-apply "[GVNHoist] Move GVNHoist to function simplification part of pipeline."

[Philip Reames via llvm-commits]

If this does turn out to be the problematic commit, I'll ask that we do 
not re-enable GVNHoist without some llvm-dev discussion of what we've 
done to test it. I've literally lost track of the number of times this 
has been enabled and disabled by now.  There's clearly a problem here; 
it's time we discuss it and what needs done to address it going forward.

Philip

On 04/21/2017 08:24 PM, Chandler Carruth via llvm-commits wrote:
> FYI and just as a heads up, we're seeing buggy hoisting leading to 
> null pointers being accessed, and this commit is in the range and 
> seems very likely to be responsible. We're working on getting this 
> isolated enough to be sure, but wanted to mention it in case anyone 
> else is similarly looking at issues after this commit.
>
> On Thu, Apr 13, 2017 at 8:49 AM Geoff Berry via llvm-commits 
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>
>     Author: gberry
>     Date: Thu Apr 13 10:36:25 2017
>     New Revision: 300200
>
>     URL: http://llvm.org/viewvc/llvm-project?rev=300200&view=rev
>     Log:
>     Re-apply "[GVNHoist] Move GVNHoist to function simplification part
>     of pipeline."
>
>     This reverts commit r296872 now that PR32153 has been fixed.
>
>     Added:
>         llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll
>     Modified:
>         llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp
>
>     Modified: llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp
>     URL:
>     http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp?rev=300200&r1=300199&r2=300200&view=diff
>     ==============================================================================
>     --- llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp (original)
>     +++ llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp Thu Apr
>     13 10:36:25 2017
>     @@ -245,8 +245,6 @@ void PassManagerBuilder::populateFunctio
>        FPM.add(createCFGSimplificationPass());
>        FPM.add(createSROAPass());
>        FPM.add(createEarlyCSEPass());
>     -  if (EnableGVNHoist)
>     -    FPM.add(createGVNHoistPass());
>        FPM.add(createLowerExpectIntrinsicPass());
>      }
>
>     @@ -291,6 +289,8 @@ void PassManagerBuilder::addFunctionSimp
>        // Break up aggregate allocas, using SSAUpdater.
>        MPM.add(createSROAPass());
>        MPM.add(createEarlyCSEPass());              // Catch trivial
>     redundancies
>     +  if (EnableGVNHoist)
>     +    MPM.add(createGVNHoistPass());
>        // Speculative execution if the target has divergent branches;
>     otherwise nop.
>      MPM.add(createSpeculativeExecutionIfHasBranchDivergencePass());
>        MPM.add(createJumpThreadingPass());         // Thread jumps.
>
>     Added: llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll
>     URL:
>     http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll?rev=300200&view=auto
>     ==============================================================================
>     --- llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll (added)
>     +++ llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll Thu Apr 13
>     10:36:25 2017
>     @@ -0,0 +1,38 @@
>     +; RUN: opt -S -O2 < %s | FileCheck %s
>     +
>     +; Check that the inlined loads are hoisted.
>     +; CHECK-LABEL: define i32 @fun(
>     +; CHECK-LABEL: entry:
>     +; CHECK: load i32, i32* @A
>     +; CHECK: if.then:
>     +
>     + at A = external global i32
>     + at B = external global i32
>     + at C = external global i32
>     +
>     +define i32 @loadA() {
>     +   %a = load i32, i32* @A
>     +   ret i32 %a
>     +}
>     +
>     +define i32 @fun(i1 %c) {
>     +entry:
>     +  br i1 %c, label %if.then, label %if.else
>     +
>     +if.then:
>     +  store i32 1, i32* @B
>     +  %call1 = call i32 @loadA()
>     +  store i32 2, i32* @C
>     +  br label %if.endif
>     +
>     +if.else:
>     +  store i32 2, i32* @C
>     +  %call2 = call i32 @loadA()
>     +  store i32 1, i32* @B
>     +  br label %if.endif
>     +
>     +if.endif:
>     +  %ret = phi i32 [ %call1, %if.then ], [ %call2, %if.else ]
>     +  ret i32 %ret
>     +}
>     +
>
>
>     _______________________________________________
>     llvm-commits mailing list
>     llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>     http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> 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/20170423/00755fd1/attachment.html>