llvm / llvm/llvm-project

CoroSplit incorrectly assumed poison after splitting retcon coroutines with zero suspensions

Open
#208,304 1 comment 0 reactions 0 assignees View on GitHub
coroutines
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

As of `cefd20c496`, `CoroSplit` would incorrectly assert that the detached frame buffer to be poisoned for `retcon` coroutines with no suspensions.

Currently I am using this IR.
```llvm
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
declare token @llvm.coro.id.retcon(i32, i32, ptr, ptr, ptr, ptr)
declare ptr @llvm.coro.begin(token, ptr)
declare void @llvm.coro.end(ptr, i1, token)
declare noalias ptr @malloc(i64)
declare void @free(ptr)
define ptr @prototype(ptr dereferenceable(8) %buf, i1 %unwind) {
ret ptr null
}
define ptr @f(ptr dereferenceable(8) %buf) {
entry:
; We can set size to 0 and align to 1 because this coroutine has 0 suspends,
; meaning it will always be elided and requires no frame storage.
%id = call token @llvm.coro.id.retcon(i32 0, i32 1, ptr %buf, ptr @prototype, ptr @malloc, ptr @free)
%hdl = call ptr @llvm.coro.begin(token %id, ptr %buf)

; Access the frame (buffer) via handle.
; %hdl is equivalent to %buf (dereferenceable(8)).
; GEP offset is 1 (4 bytes for i32), so we access bytes 4-7, which is within the 8-byte buffer.
%val_ptr = getelementptr inbounds i32, ptr %hdl, i64 1
store i32 42, ptr %val_ptr, align 4

call void @llvm.coro.end(ptr %hdl, i1 false, token none)
ret ptr null
}
```

I ran `CoroEarly` and `CoroSplit` on it and I spotted a problem.

```sh
opt -passes='coro-early,coro-split' -S repro.ll -o repro_split.ll
```

The problem is with the `poison`

```llvm
define ptr @f(ptr dereferenceable(8) %buf) {
entry:
; We can set size to 0 and align to 1 because this coroutine has 0 suspends,
; meaning it will always be elided and requires no frame storage.
%id = call token @llvm.coro.id.retcon(i32 0, i32 1, ptr %buf, ptr @prototype, ptr @malloc, ptr @free)
br label %AllocaSpillBB

AllocaSpillBB: ; preds = %entry
br label %PostSpill

PostSpill: ; preds = %AllocaSpillBB
; BUG: %hdl replaced by poison instead of %buf,
; but %buf should have been used because it has enough frame storage and
; it is dereferenceable for 8 bytes.
%val_ptr = getelementptr inbounds i32, ptr poison, i64 1
store i32 42, ptr %val_ptr, align 4
br label %CoroEnd

CoroEnd: ; preds = %PostSpill
ret ptr null
}
```

Contributor guide

Open the contributing guide

Research direction

Reproduce the issue with the provided repro.ll using opt -passes='coro-early,coro-split' -S repro.ll -o repro_split.ll, then inspect the CoroSplit transformation around the generated AllocaSpillBB and PostSpill blocks. Done means the zero-suspension retcon coroutine uses %buf rather than poison when the buffer is sufficiently dereferenceable, while preserving the existing transformation behavior.

Written by the indexing model from the issue text.

Assessment

Domain
compilers
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.