[llvm] r299771 - [coroutines] Insert spills of PHI instructions correctly
Reid Kleckner via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 7 11:10:24 PDT 2017
There is some non-determinism in the order in which spills are inserted:
$ opt "-coro-split" "-S" <
'C:\src\llvm-project\llvm\test\Transforms\Coroutines\coro-spill-after-phi.ll'
-debug-only coro-frame |& grep -A4 Spills
------------- Spills--------------
%phi1 = phi i32 [ %.begin2, %begin.from.entry ], [ %.begin,
%begin.from.alt ]
user: %1 = call i32 @print(i32 %phi1)
%phi2 = phi i32 [ %.begin3, %begin.from.entry ], [ %.begin1,
%begin.from.alt ]
user: %2 = call i32 @print(i32 %phi2)
$ "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
------------- Spills--------------
%phi2 = phi i32 [ %.begin3, %begin.from.entry ], [ %.begin1,
%begin.from.alt ]
user: %2 = call i32 @print(i32 %phi2)
%phi1 = phi i32 [ %.begin2, %begin.from.entry ], [ %.begin,
%begin.from.alt ]
user: %1 = call i32 @print(i32 %phi1)
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.
On Fri, Apr 7, 2017 at 7:16 AM, Gor Nishanov via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: gornishanov
> Date: Fri Apr 7 09:16:49 2017
> New Revision: 299771
>
> URL: http://llvm.org/viewvc/llvm-project?rev=299771&view=rev
> Log:
> [coroutines] Insert spills of PHI instructions correctly
>
> Summary:
> Fix a bug where we were inserting a spill in between the PHIs in the
> beginning of the block.
> Consider this fragment:
>
> ```
> begin:
> %phi1 = phi i32 [ 0, %entry ], [ 2, %alt ]
> %phi2 = phi i32 [ 1, %entry ], [ 3, %alt ]
> %sp1 = call i8 @llvm.coro.suspend(token none, i1 false)
> switch i8 %sp1, label %suspend [i8 0, label %resume
> i8 1, label %cleanup]
> resume:
> call i32 @print(i32 %phi1)
> ```
> Unless we are spilling the argument or result of the invoke, we were
> always inserting the spill immediately following the instruction.
> The fix adds a check that if the spilled instruction is a PHI Node, select
> an appropriate insert point with `getFirstInsertionPt()` that
> skips all the PHI Nodes and EH pads.
>
> Reviewers: majnemer, rnk
>
> Reviewed By: rnk
>
> Subscribers: qcolombet, EricWF, llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D31799
>
> Added:
> llvm/trunk/test/Transforms/Coroutines/coro-spill-after-phi.ll
> Modified:
> llvm/trunk/lib/Transforms/Coroutines/CoroFrame.cpp
>
> Modified: llvm/trunk/lib/Transforms/Coroutines/CoroFrame.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> Transforms/Coroutines/CoroFrame.cpp?rev=299771&r1=
> 299770&r2=299771&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Transforms/Coroutines/CoroFrame.cpp (original)
> +++ llvm/trunk/lib/Transforms/Coroutines/CoroFrame.cpp Fri Apr 7
> 09:16:49 2017
> @@ -435,6 +435,10 @@ static Instruction *insertSpills(SpillIn
> // normal edge and insert the spill in the new block.
> auto NewBB = SplitEdge(II->getParent(), II->getNormalDest());
> InsertPt = NewBB->getTerminator();
> + } else if (dyn_cast<PHINode>(CurrentValue)) {
> + // Skip the PHINodes and EH pads instructions.
> + InsertPt =
> + &*cast<Instruction>(E.def())->getParent()->
> getFirstInsertionPt();
> } else {
> // For all other values, the spill is placed immediately after
> // the definition.
>
> Added: llvm/trunk/test/Transforms/Coroutines/coro-spill-after-phi.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> Transforms/Coroutines/coro-spill-after-phi.ll?rev=299771&view=auto
> ============================================================
> ==================
> --- llvm/trunk/test/Transforms/Coroutines/coro-spill-after-phi.ll (added)
> +++ llvm/trunk/test/Transforms/Coroutines/coro-spill-after-phi.ll Fri
> Apr 7 09:16:49 2017
> @@ -0,0 +1,60 @@
> +; Verifies that we insert spills of PHI instruction _after) all PHI Nodes
> +; RUN: opt < %s -coro-split -S | FileCheck %s
> +
> +define i8* @f(i1 %n) "coroutine.presplit"="1" {
> +entry:
> + %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
> + %size = call i32 @llvm.coro.size.i32()
> + %alloc = call i8* @malloc(i32 %size)
> + %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc)
> + br i1 %n, label %begin, label %alt
> +alt:
> + br label %begin
> +
> +begin:
> + %phi1 = phi i32 [ 0, %entry ], [ 2, %alt ]
> + %phi2 = phi i32 [ 1, %entry ], [ 3, %alt ]
> +
> + %sp1 = call i8 @llvm.coro.suspend(token none, i1 false)
> + switch i8 %sp1, label %suspend [i8 0, label %resume
> + i8 1, label %cleanup]
> +resume:
> + call i32 @print(i32 %phi1)
> + call i32 @print(i32 %phi2)
> + br label %cleanup
> +
> +cleanup:
> + %mem = call i8* @llvm.coro.free(token %id, i8* %hdl)
> + call void @free(i8* %mem)
> + br label %suspend
> +suspend:
> + call i1 @llvm.coro.end(i8* %hdl, i1 0)
> + ret i8* %hdl
> +}
> +
> +; Verifies that the both phis are stored correctly in the coroutine frame
> +; CHECK: %f.Frame = type { void (%f.Frame*)*, void (%f.Frame*)*, i1, i1,
> i32, i32 }
> +; CHECK-LABEL: @f(
> +; CHECK: store void (%f.Frame*)* @f.destroy, void (%f.Frame*)**
> %destroy.addr
> +; CHECK: %phi1 = select i1 %n, i32 0, i32 2
> +; CHECK: %phi2 = select i1 %n, i32 1, i32 3
> +; CHECK: %phi2.spill.addr = getelementptr inbounds %f.Frame, %f.Frame*
> %FramePtr, i32 0, i32 5
> +; CHECK: store i32 %phi2, i32* %phi2.spill.addr
> +; CHECK: %phi1.spill.addr = getelementptr inbounds %f.Frame, %f.Frame*
> %FramePtr, i32 0, i32 4
> +; CHECK: store i32 %phi1, i32* %phi1.spill.addr
> +; CHECK: ret i8* %hdl
> +
> +declare i8* @llvm.coro.free(token, i8*)
> +declare i32 @llvm.coro.size.i32()
> +declare i8 @llvm.coro.suspend(token, i1)
> +declare void @llvm.coro.resume(i8*)
> +declare void @llvm.coro.destroy(i8*)
> +
> +declare token @llvm.coro.id(i32, i8*, i8*, i8*)
> +declare i1 @llvm.coro.alloc(token)
> +declare i8* @llvm.coro.begin(token, i8*)
> +declare i1 @llvm.coro.end(i8*, i1)
> +
> +declare noalias i8* @malloc(i32)
> +declare i32 @print(i32)
> +declare void @free(i8*)
>
>
> _______________________________________________
> 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/20170407/ccfd7056/attachment.html>
More information about the llvm-commits
mailing list