Page MenuHomePhabricator

[Coroutines] Offering llvm.coro.align intrinsic
ClosedPublic

Authored by ChuanqiXu on Jan 17 2022, 11:32 PM.

Details

Summary

It is a known problem that we can't align the switch-based coroutine frame if the alignment exceeds std::max_align_t (which is 16 usually).

Previously, the switch coroutine is used for C++. And I image that the problem could be solved at the language side and we could implement the corresponding proposal.

However, MLIR uses switch-based coroutine now. And they meet the same question: https://github.com/llvm/llvm-project/issues/53148. Luckily, the semantics of MLIR is less constraint than C++. So we could solve the problem in MLIR by offering a llvm.coro.align intrinsic. And the intrinsic should be able to be reused when we try to solve the problem in C++.

Diff Detail

Event Timeline

ChuanqiXu created this revision.Jan 17 2022, 11:32 PM
ChuanqiXu requested review of this revision.Jan 17 2022, 11:32 PM
ezhulenev accepted this revision.Jan 18 2022, 3:49 AM

thank you! For our MLIR use case this is exactly what we need.

This revision is now accepted and ready to land.Jan 18 2022, 3:49 AM
This revision was landed with ongoing or failed builds.Jan 18 2022, 5:53 PM
This revision was automatically updated to reflect the committed changes.
ychen added a comment.Jan 18 2022, 7:19 PM

LGTM. A nit: it would be nice the name the test files with some suffix like align16, align64, etc.

LGTM. A nit: it would be nice the name the test files with some suffix like align16, align64, etc.

Do you mean renaming test filename from coro-align-02.ll to something like coro-align8 or coro-align16?

ychen added a comment.Jan 18 2022, 7:26 PM

LGTM. A nit: it would be nice the name the test files with some suffix like align16, align64, etc.

Do you mean renaming test filename from coro-align-02.ll to something like coro-align8 or coro-align16?

Yep. As long as it hints at the differences among these similar tests.

LGTM. A nit: it would be nice the name the test files with some suffix like align16, align64, etc.

Do you mean renaming test filename from coro-align-02.ll to something like coro-align8 or coro-align16?

Yep. As long as it hints at the differences among these similar tests.

Got it. I would do it.