<div dir="ltr">There is some non-determinism in the order in which spills are inserted:<div><div>$ opt "-coro-split" "-S" < 'C:\src\llvm-project\llvm\test\Transforms\Coroutines\coro-spill-after-phi.ll' -debug-only coro-frame |& grep -A4 Spills</div><div>------------- Spills--------------</div><div> %phi1 = phi i32 [ %.begin2, %begin.from.entry ], [ %.begin, %begin.from.alt ]</div><div> user: %1 = call i32 @print(i32 %phi1)</div><div> %phi2 = phi i32 [ %.begin3, %begin.from.entry ], [ %.begin1, %begin.from.alt ]</div><div> user: %2 = call i32 @print(i32 %phi2)</div><div><br></div><div>$ "C:/src/llvm-project/build/./bin\opt.EXE" "-coro-split" "-S" < 'C:\src\llvm-project\llvm\test\Transforms\Coroutines\coro-spill-after-phi.ll' -debug-only coro-frame |& grep -A4 Spills</div><div>------------- Spills--------------</div><div> %phi2 = phi i32 [ %.begin3, %begin.from.entry ], [ %.begin1, %begin.from.alt ]</div><div> user: %2 = call i32 @print(i32 %phi2)</div><div> %phi1 = phi i32 [ %.begin2, %begin.from.entry ], [ %.begin, %begin.from.alt ]</div><div> user: %1 = call i32 @print(i32 %phi1)</div></div><div><br></div><div>So, sometimes we spill phi2 first, sometimes phi1. The spill insertion code is probably iterating over something non-determinstic, like a DenseMap or a std::map of pointers. Can you take a look at that? I'll make the test pass for now and put in a FIXME.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 7, 2017 at 7:16 AM, Gor Nishanov via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: gornishanov<br>
Date: Fri Apr 7 09:16:49 2017<br>
New Revision: 299771<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=299771&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=299771&view=rev</a><br>
Log:<br>
[coroutines] Insert spills of PHI instructions correctly<br>
<br>
Summary:<br>
Fix a bug where we were inserting a spill in between the PHIs in the beginning of the block.<br>
Consider this fragment:<br>
<br>
```<br>
begin:<br>
%phi1 = phi i32 [ 0, %entry ], [ 2, %alt ]<br>
%phi2 = phi i32 [ 1, %entry ], [ 3, %alt ]<br>
%sp1 = call i8 @llvm.coro.suspend(token none, i1 false)<br>
switch i8 %sp1, label %suspend [i8 0, label %resume<br>
i8 1, label %cleanup]<br>
resume:<br>
call i32 @print(i32 %phi1)<br>
```<br>
Unless we are spilling the argument or result of the invoke, we were always inserting the spill immediately following the instruction.<br>
The fix adds a check that if the spilled instruction is a PHI Node, select an appropriate insert point with `getFirstInsertionPt()` that<br>
skips all the PHI Nodes and EH pads.<br>
<br>
Reviewers: majnemer, rnk<br>
<br>
Reviewed By: rnk<br>
<br>
Subscribers: qcolombet, EricWF, llvm-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D31799" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D31799</a><br>
<br>
Added:<br>
llvm/trunk/test/Transforms/<wbr>Coroutines/coro-spill-after-<wbr>phi.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/<wbr>Coroutines/CoroFrame.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/<wbr>Coroutines/CoroFrame.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Coroutines/CoroFrame.cpp?rev=299771&r1=299770&r2=299771&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Transforms/Coroutines/<wbr>CoroFrame.cpp?rev=299771&r1=<wbr>299770&r2=299771&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Transforms/<wbr>Coroutines/CoroFrame.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/<wbr>Coroutines/CoroFrame.cpp Fri Apr 7 09:16:49 2017<br>
@@ -435,6 +435,10 @@ static Instruction *insertSpills(SpillIn<br>
// normal edge and insert the spill in the new block.<br>
auto NewBB = SplitEdge(II->getParent(), II->getNormalDest());<br>
InsertPt = NewBB->getTerminator();<br>
+ } else if (dyn_cast<PHINode>(<wbr>CurrentValue)) {<br>
+ // Skip the PHINodes and EH pads instructions.<br>
+ InsertPt =<br>
+ &*cast<Instruction>(E.def())-><wbr>getParent()-><wbr>getFirstInsertionPt();<br>
} else {<br>
// For all other values, the spill is placed immediately after<br>
// the definition.<br>
<br>
Added: llvm/trunk/test/Transforms/<wbr>Coroutines/coro-spill-after-<wbr>phi.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Coroutines/coro-spill-after-phi.ll?rev=299771&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Transforms/Coroutines/coro-<wbr>spill-after-phi.ll?rev=299771&<wbr>view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Transforms/<wbr>Coroutines/coro-spill-after-<wbr>phi.ll (added)<br>
+++ llvm/trunk/test/Transforms/<wbr>Coroutines/coro-spill-after-<wbr>phi.ll Fri Apr 7 09:16:49 2017<br>
@@ -0,0 +1,60 @@<br>
+; Verifies that we insert spills of PHI instruction _after) all PHI Nodes<br>
+; RUN: opt < %s -coro-split -S | FileCheck %s<br>
+<br>
+define i8* @f(i1 %n) "coroutine.presplit"="1" {<br>
+entry:<br>
+ %id = call token @<a href="http://llvm.coro.id" rel="noreferrer" target="_blank">llvm.coro.id</a>(i32 0, i8* null, i8* null, i8* null)<br>
+ %size = call i32 @llvm.coro.size.i32()<br>
+ %alloc = call i8* @malloc(i32 %size)<br>
+ %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc)<br>
+ br i1 %n, label %begin, label %alt<br>
+alt:<br>
+ br label %begin<br>
+<br>
+begin:<br>
+ %phi1 = phi i32 [ 0, %entry ], [ 2, %alt ]<br>
+ %phi2 = phi i32 [ 1, %entry ], [ 3, %alt ]<br>
+<br>
+ %sp1 = call i8 @llvm.coro.suspend(token none, i1 false)<br>
+ switch i8 %sp1, label %suspend [i8 0, label %resume<br>
+ i8 1, label %cleanup]<br>
+resume:<br>
+ call i32 @print(i32 %phi1)<br>
+ call i32 @print(i32 %phi2)<br>
+ br label %cleanup<br>
+<br>
+cleanup:<br>
+ %mem = call i8* @llvm.coro.free(token %id, i8* %hdl)<br>
+ call void @free(i8* %mem)<br>
+ br label %suspend<br>
+suspend:<br>
+ call i1 @llvm.coro.end(i8* %hdl, i1 0)<br>
+ ret i8* %hdl<br>
+}<br>
+<br>
+; Verifies that the both phis are stored correctly in the coroutine frame<br>
+; CHECK: %f.Frame = type { void (%f.Frame*)*, void (%f.Frame*)*, i1, i1, i32, i32 }<br>
+; CHECK-LABEL: @f(<br>
+; CHECK: store void (%f.Frame*)* @f.destroy, void (%f.Frame*)** %destroy.addr<br>
+; CHECK: %phi1 = select i1 %n, i32 0, i32 2<br>
+; CHECK: %phi2 = select i1 %n, i32 1, i32 3<br>
+; CHECK: %phi2.spill.addr = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 5<br>
+; CHECK: store i32 %phi2, i32* %phi2.spill.addr<br>
+; CHECK: %phi1.spill.addr = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 4<br>
+; CHECK: store i32 %phi1, i32* %phi1.spill.addr<br>
+; CHECK: ret i8* %hdl<br>
+<br>
+declare i8* @llvm.coro.free(token, i8*)<br>
+declare i32 @llvm.coro.size.i32()<br>
+declare i8 @llvm.coro.suspend(token, i1)<br>
+declare void @llvm.coro.resume(i8*)<br>
+declare void @llvm.coro.destroy(i8*)<br>
+<br>
+declare token @<a href="http://llvm.coro.id" rel="noreferrer" target="_blank">llvm.coro.id</a>(i32, i8*, i8*, i8*)<br>
+declare i1 @llvm.coro.alloc(token)<br>
+declare i8* @llvm.coro.begin(token, i8*)<br>
+declare i1 @llvm.coro.end(i8*, i1)<br>
+<br>
+declare noalias i8* @malloc(i32)<br>
+declare i32 @print(i32)<br>
+declare void @free(i8*)<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>