<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>If any one decides to revert this, I'd like to request that you
attempt to do so by just turning off GVNHoist by default instead
of reverting r300200 if possible. I believe the current location
in the pass pipeline is the desired one, and reverting r300200
will just move it back to being done before inlining, which is
likely just hiding bugs.<br>
</p>
FWIW, before re-enabling this most recently I did correctness
testing on X86 and AArch64 of the test-suite, SPEC and several other
benchmarks, as well as bootstrapping clang, all without issue.<br>
<br>
<div class="moz-cite-prefix">On 4/23/2017 12:45 PM, Philip Reames
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:b6331c39-89b0-5e08-d4a4-12f41c6d9e85@philipreames.com">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">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.<br>
<br>
Philip<br>
<br>
On 04/21/2017 08:24 PM, Chandler Carruth via llvm-commits wrote:<br>
</div>
<blockquote
cite="mid:CAAwGriFDXmS4M2t78Ny9mW8Ax7i_WgieCuf3ix9ps1C8=rrRXQ@mail.gmail.com"
type="cite">
<div dir="ltr">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.</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Thu, Apr 13, 2017 at 8:49 AM Geoff Berry via
llvm-commits <<a moz-do-not-send="true"
href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">Author:
gberry<br>
Date: Thu Apr 13 10:36:25 2017<br>
New Revision: 300200<br>
<br>
URL: <a moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project?rev=300200&view=rev"
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=300200&view=rev</a><br>
Log:<br>
Re-apply "[GVNHoist] Move GVNHoist to function
simplification part of pipeline."<br>
<br>
This reverts commit r296872 now that PR32153 has been fixed.<br>
<br>
Added:<br>
llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp<br>
<br>
Modified:
llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp<br>
URL: <a moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp?rev=300200&r1=300199&r2=300200&view=diff"
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp?rev=300200&r1=300199&r2=300200&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp
(original)<br>
+++ llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp Thu
Apr 13 10:36:25 2017<br>
@@ -245,8 +245,6 @@ void PassManagerBuilder::populateFunctio<br>
FPM.add(createCFGSimplificationPass());<br>
FPM.add(createSROAPass());<br>
FPM.add(createEarlyCSEPass());<br>
- if (EnableGVNHoist)<br>
- FPM.add(createGVNHoistPass());<br>
FPM.add(createLowerExpectIntrinsicPass());<br>
}<br>
<br>
@@ -291,6 +289,8 @@ void PassManagerBuilder::addFunctionSimp<br>
// Break up aggregate allocas, using SSAUpdater.<br>
MPM.add(createSROAPass());<br>
MPM.add(createEarlyCSEPass()); // Catch
trivial redundancies<br>
+ if (EnableGVNHoist)<br>
+ MPM.add(createGVNHoistPass());<br>
// Speculative execution if the target has divergent
branches; otherwise nop.<br>
MPM.add(createSpeculativeExecutionIfHasBranchDivergencePass());<br>
MPM.add(createJumpThreadingPass()); // Thread
jumps.<br>
<br>
Added: llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll<br>
URL: <a moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll?rev=300200&view=auto"
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll?rev=300200&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll
(added)<br>
+++ llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll Thu
Apr 13 10:36:25 2017<br>
@@ -0,0 +1,38 @@<br>
+; RUN: opt -S -O2 < %s | FileCheck %s<br>
+<br>
+; Check that the inlined loads are hoisted.<br>
+; CHECK-LABEL: define i32 @fun(<br>
+; CHECK-LABEL: entry:<br>
+; CHECK: load i32, i32* @A<br>
+; CHECK: if.then:<br>
+<br>
+@A = external global i32<br>
+@B = external global i32<br>
+@C = external global i32<br>
+<br>
+define i32 @loadA() {<br>
+ %a = load i32, i32* @A<br>
+ ret i32 %a<br>
+}<br>
+<br>
+define i32 @fun(i1 %c) {<br>
+entry:<br>
+ br i1 %c, label %if.then, label %if.else<br>
+<br>
+if.then:<br>
+ store i32 1, i32* @B<br>
+ %call1 = call i32 @loadA()<br>
+ store i32 2, i32* @C<br>
+ br label %if.endif<br>
+<br>
+if.else:<br>
+ store i32 2, i32* @C<br>
+ %call2 = call i32 @loadA()<br>
+ store i32 1, i32* @B<br>
+ br label %if.endif<br>
+<br>
+if.endif:<br>
+ %ret = phi i32 [ %call1, %if.then ], [ %call2, %if.else ]<br>
+ ret i32 %ret<br>
+}<br>
+<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a moz-do-not-send="true"
href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a moz-do-not-send="true"
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"
rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
llvm-commits mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org" moz-do-not-send="true">llvm-commits@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
<p><br>
</p>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Geoff Berry
Employee of Qualcomm Datacenter Technologies, Inc.
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.</pre>
</body>
</html>