Skip to content

Commit 9de71a3

Browse files
aschwaighoferaschwaighofer
authored
Merge pull request #3336 from aschwaighofer/coro_lowering_lifetime_bugs_20210726
Disable optimizations based on lifetime intrinsics
2 parents a885791 + 9edc5ee commit 9de71a3

File tree

3 files changed

+224
-5
lines changed

3 files changed

+224
-5
lines changed

‎llvm/lib/Transforms/Coroutines/CoroFrame.cpp

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,8 +1240,10 @@ namespace {
12401240
struct AllocaUseVisitor : PtrUseVisitor<AllocaUseVisitor> {
12411241
using Base = PtrUseVisitor<AllocaUseVisitor>;
12421242
AllocaUseVisitor(const DataLayout &DL, const DominatorTree &DT,
1243-
const CoroBeginInst &CB, const SuspendCrossingInfo &Checker)
1244-
: PtrUseVisitor(DL), DT(DT), CoroBegin(CB), Checker(Checker) {}
1243+
const CoroBeginInst &CB, const SuspendCrossingInfo &Checker,
1244+
bool ShouldUseLifetimeStartInfo)
1245+
: PtrUseVisitor(DL), DT(DT), CoroBegin(CB), Checker(Checker),
1246+
ShouldUseLifetimeStartInfo(ShouldUseLifetimeStartInfo) {}
12451247

12461248
void visit(Instruction &I) {
12471249
Users.insert(&I);
@@ -1389,6 +1391,7 @@ struct AllocaUseVisitor : PtrUseVisitor<AllocaUseVisitor> {
13891391
SmallPtrSet<Instruction *, 4> Users{};
13901392
SmallPtrSet<IntrinsicInst *, 2> LifetimeStarts{};
13911393
bool MayWriteBeforeCoroBegin{false};
1394+
bool ShouldUseLifetimeStartInfo{true};
13921395

13931396
mutable llvm::Optional<bool> ShouldLiveOnFrame{};
13941397

@@ -1397,7 +1400,7 @@ struct AllocaUseVisitor : PtrUseVisitor<AllocaUseVisitor> {
13971400
// more precise. We look at every pair of lifetime.start intrinsic and
13981401
// every basic block that uses the pointer to see if they cross suspension
13991402
// points. The uses cover both direct uses as well as indirect uses.
1400-
if (!LifetimeStarts.empty()) {
1403+
if (ShouldUseLifetimeStartInfo && !LifetimeStarts.empty()) {
14011404
for (auto *I : Users)
14021405
for (auto *S : LifetimeStarts)
14031406
if (Checker.isDefinitionAcrossSuspend(*S, I))
@@ -2489,8 +2492,15 @@ static void collectFrameAllocas(Function &F, coro::Shape &Shape,
24892492
continue;
24902493
}
24912494
DominatorTree DT(F);
2495+
// The code that uses lifetime.start intrinsic does not work for functions
2496+
// with loops without exit. Disable it on ABIs we know to generate such
2497+
// code.
2498+
bool ShouldUseLifetimeStartInfo =
2499+
(Shape.ABI != coro::ABI::Async && Shape.ABI != coro::ABI::Retcon &&
2500+
Shape.ABI != coro::ABI::RetconOnce);
24922501
AllocaUseVisitor Visitor{F.getParent()->getDataLayout(), DT,
2493-
*Shape.CoroBegin, Checker};
2502+
*Shape.CoroBegin, Checker,
2503+
ShouldUseLifetimeStartInfo};
24942504
Visitor.visitPtr(*AI);
24952505
if (!Visitor.getShouldLiveOnFrame())
24962506
continue;
@@ -2679,7 +2689,10 @@ void coro::buildCoroutineFrame(Function &F, Shape &Shape) {
26792689
}
26802690
}
26812691

2682-
sinkLifetimeStartMarkers(F, Shape, Checker);
2692+
if (Shape.ABI != coro::ABI::Async && Shape.ABI != coro::ABI::Retcon &&
2693+
Shape.ABI != coro::ABI::RetconOnce)
2694+
sinkLifetimeStartMarkers(F, Shape, Checker);
2695+
26832696
if (Shape.ABI != coro::ABI::Async || !Shape.CoroSuspends.empty())
26842697
collectFrameAllocas(F, Shape, Checker, FrameData.Allocas);
26852698
LLVM_DEBUG(dumpAllocas(FrameData.Allocas));

‎llvm/test/Transforms/Coroutines/coro-async-addr-lifetime-infinite-loop-bug.ll

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
; RUN: opt < %s -enable-coroutines -passes='default<O0>' -S | FileCheck --check-prefixes=CHECK %s
2+
; RUN: opt < %s -enable-coroutines -O0 -S | FileCheck --check-prefixes=CHECK %s
3+
4+
target datalayout = "p:64:64:64"
5+
6+
%async.task = type { i64 }
7+
%async.actor = type { i64 }
8+
%async.fp = type <{ i32, i32 }>
9+
10+
%async.ctxt = type { i8*, void (i8*, %async.task*, %async.actor*)* }
11+
12+
; The async callee.
13+
@my_other_async_function_fp = external global <{ i32, i32 }>
14+
declare void @my_other_async_function(i8* %async.ctxt)
15+
16+
@my_async_function_fp = constant <{ i32, i32 }>
17+
<{ i32 trunc ( ; Relative pointer to async function
18+
i64 sub (
19+
i64 ptrtoint (void (i8*)* @my_async_function to i64),
20+
i64 ptrtoint (i32* getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @my_async_function_fp, i32 0, i32 1) to i64)
21+
)
22+
to i32),
23+
i32 128 ; Initial async context size without space for frame
24+
}>
25+
26+
define swiftcc void @my_other_async_function_fp.apply(i8* %fnPtr, i8* %async.ctxt) {
27+
%callee = bitcast i8* %fnPtr to void(i8*)*
28+
tail call swiftcc void %callee(i8* %async.ctxt)
29+
ret void
30+
}
31+
32+
declare void @escape(i64*)
33+
declare void @store_resume(i8*)
34+
define i8* @resume_context_projection(i8* %ctxt) {
35+
entry:
36+
%resume_ctxt_addr = bitcast i8* %ctxt to i8**
37+
%resume_ctxt = load i8*, i8** %resume_ctxt_addr, align 8
38+
ret i8* %resume_ctxt
39+
}
40+
41+
; The address of alloca escapes but the analysis based on lifetimes fails to see
42+
; that it can't localize this alloca.
43+
; CHECK: define swiftcc void @my_async_function(i8* swiftasync %async.ctxt) {
44+
; CHECK: entry:
45+
; CHECK-NOT: ret
46+
; CHECK-NOT: [[ESCAPED_ADDR:%.*]] = alloca i64, align 8
47+
; CHECK: ret
48+
define swiftcc void @my_async_function(i8* swiftasync %async.ctxt) {
49+
entry:
50+
%escaped_addr = alloca i64
51+
52+
%id = call token @llvm.coro.id.async(i32 128, i32 16, i32 0,
53+
i8* bitcast (<{i32, i32}>* @my_async_function_fp to i8*))
54+
%hdl = call i8* @llvm.coro.begin(token %id, i8* null)
55+
%ltb = bitcast i64* %escaped_addr to i8*
56+
call void @llvm.lifetime.start.p0i8(i64 4, i8* %ltb)
57+
call void @escape(i64* %escaped_addr)
58+
br label %callblock
59+
60+
61+
callblock:
62+
63+
%callee_context = call i8* @context_alloc()
64+
65+
%resume.func_ptr = call i8* @llvm.coro.async.resume()
66+
call void @store_resume(i8* %resume.func_ptr)
67+
%resume_proj_fun = bitcast i8*(i8*)* @resume_context_projection to i8*
68+
%callee = bitcast void(i8*)* @asyncSuspend to i8*
69+
%res = call {i8*, i8*, i8*} (i32, i8*, i8*, ...) @llvm.coro.suspend.async(i32 0,
70+
i8* %resume.func_ptr,
71+
i8* %resume_proj_fun,
72+
void (i8*, i8*)* @my_other_async_function_fp.apply,
73+
i8* %callee, i8* %callee_context)
74+
br label %callblock
75+
}
76+
77+
declare { i8*, i8*, i8*, i8* } @llvm.coro.suspend.async.sl_p0i8p0i8p0i8p0i8s(i32, i8*, i8*, ...)
78+
declare i8* @llvm.coro.prepare.async(i8*)
79+
declare token @llvm.coro.id.async(i32, i32, i32, i8*)
80+
declare i8* @llvm.coro.begin(token, i8*)
81+
declare i1 @llvm.coro.end.async(i8*, i1, ...)
82+
declare i1 @llvm.coro.end(i8*, i1)
83+
declare {i8*, i8*, i8*} @llvm.coro.suspend.async(i32, i8*, i8*, ...)
84+
declare i8* @context_alloc()
85+
declare void @llvm.coro.async.context.dealloc(i8*)
86+
declare swiftcc void @asyncSuspend(i8*)
87+
declare i8* @llvm.coro.async.resume()
88+
declare void @llvm.coro.async.size.replace(i8*, i8*)
89+
declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #0
90+
declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) #0
91+
attributes #0 = { argmemonly nofree nosync nounwind willreturn }

‎llvm/test/Transforms/Coroutines/coro-async-addr-lifetime-start-bug.ll

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
; RUN: opt < %s -enable-coroutines -passes='default<O0>' -S | FileCheck --check-prefixes=CHECK %s
2+
; RUN: opt < %s -enable-coroutines -O0 -S | FileCheck --check-prefixes=CHECK %s
3+
4+
target datalayout = "p:64:64:64"
5+
6+
%async.task = type { i64 }
7+
%async.actor = type { i64 }
8+
%async.fp = type <{ i32, i32 }>
9+
10+
%async.ctxt = type { i8*, void (i8*, %async.task*, %async.actor*)* }
11+
12+
; The async callee.
13+
@my_other_async_function_fp = external global <{ i32, i32 }>
14+
declare void @my_other_async_function(i8* %async.ctxt)
15+
16+
@my_async_function_fp = constant <{ i32, i32 }>
17+
<{ i32 trunc ( ; Relative pointer to async function
18+
i64 sub (
19+
i64 ptrtoint (void (i8*)* @my_async_function to i64),
20+
i64 ptrtoint (i32* getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @my_async_function_fp, i32 0, i32 1) to i64)
21+
)
22+
to i32),
23+
i32 128 ; Initial async context size without space for frame
24+
}>
25+
26+
define swiftcc void @my_other_async_function_fp.apply(i8* %fnPtr, i8* %async.ctxt) {
27+
%callee = bitcast i8* %fnPtr to void(i8*)*
28+
tail call swiftcc void %callee(i8* %async.ctxt)
29+
ret void
30+
}
31+
32+
declare void @escape(i64*)
33+
declare void @store_resume(i8*)
34+
declare i1 @exitLoop()
35+
define i8* @resume_context_projection(i8* %ctxt) {
36+
entry:
37+
%resume_ctxt_addr = bitcast i8* %ctxt to i8**
38+
%resume_ctxt = load i8*, i8** %resume_ctxt_addr, align 8
39+
ret i8* %resume_ctxt
40+
}
41+
42+
define swiftcc void @my_async_function(i8* swiftasync %async.ctxt) {
43+
entry:
44+
%escaped_addr = alloca i64
45+
46+
%id = call token @llvm.coro.id.async(i32 128, i32 16, i32 0,
47+
i8* bitcast (<{i32, i32}>* @my_async_function_fp to i8*))
48+
%hdl = call i8* @llvm.coro.begin(token %id, i8* null)
49+
%ltb = bitcast i64* %escaped_addr to i8*
50+
call void @llvm.lifetime.start.p0i8(i64 4, i8* %ltb)
51+
br label %callblock
52+
53+
54+
callblock:
55+
56+
%callee_context = call i8* @context_alloc()
57+
58+
%resume.func_ptr = call i8* @llvm.coro.async.resume()
59+
call void @store_resume(i8* %resume.func_ptr)
60+
%resume_proj_fun = bitcast i8*(i8*)* @resume_context_projection to i8*
61+
%callee = bitcast void(i8*)* @asyncSuspend to i8*
62+
%res = call {i8*, i8*, i8*} (i32, i8*, i8*, ...) @llvm.coro.suspend.async(i32 0,
63+
i8* %resume.func_ptr,
64+
i8* %resume_proj_fun,
65+
void (i8*, i8*)* @my_other_async_function_fp.apply,
66+
i8* %callee, i8* %callee_context)
67+
call void @escape(i64* %escaped_addr)
68+
%exitCond = call i1 @exitLoop()
69+
70+
;; We used to move the lifetime.start intrinsic here =>
71+
;; This exposes two bugs:
72+
; 1.) The code should use the basic block start not end as insertion point
73+
; More problematically:
74+
; 2.) The code marks the stack object as not alive for part of the loop.
75+
76+
br i1 %exitCond, label %loop_exit, label %loop
77+
%res2 = call {i8*, i8*, i8*} (i32, i8*, i8*, ...) @llvm.coro.suspend.async(i32 0,
78+
i8* %resume.func_ptr,
79+
i8* %resume_proj_fun,
80+
void (i8*, i8*)* @my_other_async_function_fp.apply,
81+
i8* %callee, i8* %callee_context)
82+
83+
%exitCond2 = call i1 @exitLoop()
84+
br i1 %exitCond2, label %loop_exit, label %loop
85+
86+
loop:
87+
br label %callblock
88+
89+
loop_exit:
90+
call void @llvm.lifetime.end.p0i8(i64 4, i8* %ltb)
91+
call i1 (i8*, i1, ...) @llvm.coro.end.async(i8* %hdl, i1 false)
92+
unreachable
93+
}
94+
95+
; CHECK: define {{.*}} void @my_async_function.resume.0(
96+
; CHECK-NOT: call void @llvm.lifetime.start.p0i8(i64 4, i8* %3)
97+
; CHECK: br i1 %exitCond, label %loop_exit, label %loop
98+
; CHECK: lifetime.end
99+
; CHECK: }
100+
101+
declare { i8*, i8*, i8*, i8* } @llvm.coro.suspend.async.sl_p0i8p0i8p0i8p0i8s(i32, i8*, i8*, ...)
102+
declare i8* @llvm.coro.prepare.async(i8*)
103+
declare token @llvm.coro.id.async(i32, i32, i32, i8*)
104+
declare i8* @llvm.coro.begin(token, i8*)
105+
declare i1 @llvm.coro.end.async(i8*, i1, ...)
106+
declare i1 @llvm.coro.end(i8*, i1)
107+
declare {i8*, i8*, i8*} @llvm.coro.suspend.async(i32, i8*, i8*, ...)
108+
declare i8* @context_alloc()
109+
declare void @llvm.coro.async.context.dealloc(i8*)
110+
declare swiftcc void @asyncSuspend(i8*)
111+
declare i8* @llvm.coro.async.resume()
112+
declare void @llvm.coro.async.size.replace(i8*, i8*)
113+
declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #0
114+
declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) #0
115+
attributes #0 = { argmemonly nofree nosync nounwind willreturn }

0 commit comments

Comments
 (0)