Page MenuHomePhabricator

[C++20] [Coroutines] Disable to take the address of labels in coroutines
Needs ReviewPublic

Authored by ChuanqiXu on Aug 15 2022, 8:00 PM.

Details

Summary

Closing https://github.com/llvm/llvm-project/issues/56436

We can't support the GNU address of label extension in coroutines well in current architecture. Since the coroutines are going to split into pieces in the middle end so the address of labels are ambiguous that time.

To avoid any further misunderstanding, we try to emit an error here.

Diff Detail

Event Timeline

ChuanqiXu created this revision.Aug 15 2022, 8:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 15 2022, 8:00 PM
ChuanqiXu requested review of this revision.Aug 15 2022, 8:00 PM
ChuanqiXu edited the summary of this revision. (Show Details)Aug 15 2022, 8:01 PM
ChuanqiXu updated this revision to Diff 452880.Aug 15 2022, 8:25 PM

Add ReleaseNotes.

looks good to me on a high-level, but I don't know clang well enough to confidently approve this change

clang/lib/Sema/SemaCoroutine.cpp
1107

Corotuines -> Coroutines

clang/test/SemaCXX/addr-label-in-coroutines.cpp
6

unused

ChuanqiXu updated this revision to Diff 452925.Aug 16 2022, 2:16 AM

Address comments.

ChuanqiXu marked 2 inline comments as done.Aug 16 2022, 2:17 AM
ychen added a comment.EditedAug 16 2022, 9:22 PM

Since the coroutines are going to split into pieces in the middle end so the address of labels are ambiguous that time.

Do we split in the middle or copy the whole computed goto CFG? I'd imagine the branch on a dynamic block address is like n-way branch which is not feasible to split. Does/Would it fix the problem by letting the ramp/resume function each have its own dispatch table? I skimmed through the blockaddress handling during function cloning which looks insufficient to handle this issue: https://github.com/llvm/llvm-project/blob/e20d210eef92f3952de0e89ef2f01a146480a13b/llvm/lib/Transforms/Utils/CloneFunction.cpp#L177-L182, it says "It is only legal to clone a function if a block address within that function is never referenced outside of the function." . This is not true when the dispatch table is a global variable.

Since the coroutines are going to split into pieces in the middle end so the address of labels are ambiguous that time.

Do we split in the middle or copy the whole computed goto CFG?

Yes.

I'd imagine the branch on a dynamic block address is like n-way branch which is not feasible to split. Does/Would it fix the problem by letting the ramp/resume function each have its own dispatch table? I skimmed through the blockaddress handling during function cloning which looks insufficient to handle this issue: https://github.com/llvm/llvm-project/blob/e20d210eef92f3952de0e89ef2f01a146480a13b/llvm/lib/Transforms/Utils/CloneFunction.cpp#L177-L182, it says "It is only legal to clone a function if a block address within that function is never referenced outside of the function." . This is not true when the dispatch table is a global variable.

The key problem here is that the compiler don't know the idea 'dispatch table'. The dispatch table is just a global variable. And coroutines should just leave the global variable there just like other global variables.

"It is only legal to clone a function if a block address within that function is never referenced outside of the function."

This also shows the problem. The coroutines are multiple functions indeed.