Commit Graph

8 Commits

Author SHA1 Message Date
Nikita Popov 532dc62b90 [OpaquePtrs][Clang] Add -no-opaque-pointers to tests (NFC)
This adds -no-opaque-pointers to clang tests whose output will
change when opaque pointers are enabled by default. This is
intended to be part of the migration approach described in
https://discourse.llvm.org/t/enabling-opaque-pointers-by-default/61322/9.

The patch has been produced by replacing %clang_cc1 with
%clang_cc1 -no-opaque-pointers for tests that fail with opaque
pointers enabled. Worth noting that this doesn't cover all tests,
there's a remaining ~40 tests not using %clang_cc1 that will need
a followup change.

Differential Revision: https://reviews.llvm.org/D123115
2022-04-07 12:09:47 +02:00
Chuanqi Xu ec117158a3 [Coroutines] [Frontend] Lookup in std namespace first
Now in libcxx and clang, all the coroutine components are defined in
std::experimental namespace.
And now the coroutine TS is merged into C++20. So in the working draft
like N4892, we could find the coroutine components is defined in std
namespace instead of std::experimental namespace.
And the coroutine support in clang seems to be relatively stable. So I
think it may be suitable to move the coroutine component into the
experiment namespace now.

This patch would make clang lookup coroutine_traits in std namespace
first. For the compatibility consideration, clang would lookup in
std::experimental namespace if it can't find definitions in std
namespace. So the existing codes wouldn't be break after update
compiler.

And in case the compiler found std::coroutine_traits and
std::experimental::coroutine_traits at the same time, it would emit an
error for it.

The support for looking up std::experimental::coroutine_traits would be
removed in Clang16.

Reviewed By: lxfind, Quuxplusone

Differential Revision: https://reviews.llvm.org/D108696
2021-11-04 11:53:47 +08:00
Louis Dionne 79f8b5f0d0 Revert "[Coroutines] [Clang] Look up coroutine component in std namespace first"
This reverts commit 2fbd254aa4, which broke the libc++ CI. I'm reverting
to get things stable again until we've figured out a way forward.

Differential Revision: https://reviews.llvm.org/D108696
2021-09-03 16:01:09 -04:00
Chuanqi Xu 2fbd254aa4 [Coroutines] [Clang] Look up coroutine component in std namespace first
Summary: Now in libcxx and clang, all the coroutine components are
defined in std::experimental namespace.
And now the coroutine TS is merged into C++20. So in the working draft
like N4892, we could find the coroutine components is defined in std
namespace instead of std::experimental namespace.
And the coroutine support in clang seems to be relatively stable. So I
think it may be suitable to move the coroutine component into the
experiment namespace now.

But move the coroutine component into the std namespace may be an break
change. So I planned to split this change into two patch. One in clang
and other in libcxx.

This patch would make clang lookup coroutine_traits in std namespace
first. For the compatibility consideration, clang would lookup in
std::experimental namespace if it can't find definitions in std
namespace and emit a warning in this case. So the existing codes
wouldn't be break after update compiler.

Test Plan: check-clang, check-libcxx

Reviewed By: lxfind

Differential Revision: https://reviews.llvm.org/D108696
2021-09-03 10:22:55 +08:00
Yuanfang Chen 9ffd4924e8 [NFC][Coroutines] Fix two tests by removing hardcoded SSA value. 2021-05-09 19:06:16 -07:00
Xun Li c7a39c833a [Coroutine][Clang] Force emit lifetime intrinsics for Coroutines
tl;dr Correct implementation of Corouintes requires having lifetime intrinsics available.

Coroutine functions are functions that can be suspended and resumed latter. To do so, data that need to stay alive after suspension must be put on the heap (i.e. the coroutine frame).
The optimizer is responsible for analyzing each AllocaInst and figure out whether it should be put on the stack or the frame.
In most cases, for data that we are unable to accurately analyze lifetime, we can just conservatively put them on the heap.
Unfortunately, there exists a few cases where certain data MUST be put on the stack, not on the heap. Without lifetime intrinsics, we are unable to correctly analyze those data's lifetime.

To dig into more details, there exists cases where at certain code points, the current coroutine frame may have already been destroyed. Hence no frame access would be allowed beyond that point.
The following is a common code pattern called "Symmetric Transfer" in coroutine:
```
auto tmp = await_suspend();
__builtin_coro_resume(tmp.address());
return;
```
In the above code example, `await_suspend()` returns a new coroutine handle, which we will obtain the address and then resume that coroutine. This essentially "transfered" from the current coroutine to a different coroutine.
During the call to `await_suspend()`, the current coroutine may be destroyed, which should be fine because we are not accessing any data afterwards.
However when LLVM is emitting IR for the above code, it needs to emit an AllocaInst for `tmp`. It will then call the `address` function on tmp. `address` function is a member function of coroutine, and there is no way for the LLVM optimizer to know that it does not capture the `tmp` pointer. So when the optimizer looks at it, it has to conservatively assume that `tmp` may escape and hence put it on the heap. Furthermore, in some cases `address` call would be inlined, which will generate a bunch of store/load instructions that move the `tmp` pointer around. Those stores will also make the compiler to think that `tmp` might escape.
To summarize, it's really difficult for the mid-end to figure out that the `tmp` data is short-lived.
I made some attempt in D98638, but it appears to be way too complex and is basically doing the same thing as inserting lifetime intrinsics in coroutines.

Also, for reference, we already force emitting lifetime intrinsics in O0 for AlwaysInliner: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Passes/PassBuilder.cpp#L1893

Differential Revision: https://reviews.llvm.org/D99227
2021-03-25 13:46:20 -07:00
Xun Li 516803dc86 [Coroutines] Ensure co_await promise.final_suspend() does not throw
Summary:
This patch addresses https://bugs.llvm.org/show_bug.cgi?id=46256
The spec of coroutine requires that the expression co_­await promise.final_­suspend() shall not be potentially-throwing.
To check this, we recursively look at every call (including Call, MemberCall, OperatorCall and Constructor) in all code
generated by the final suspend, and ensure that the callees are declared with noexcept. We also look at any returned data
type that requires explicit destruction, and check their destructors for noexcept.

This patch does not check declarations with dependent types yet, which will be done in future patches.

Updated all tests to add noexcept to the required functions, and added a dedicated test for this patch.

This patch might start to cause existing codebase fail to compile because most people may not have been strict in tagging
all the related functions noexcept.

Reviewers: lewissbaker, modocache, junparser

Reviewed By: modocache

Subscribers: arphaman, junparser, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82029
2020-06-22 15:01:42 -07:00
Gor Nishanov 04491bd8f3 [coroutines] Promote cleanup.dest.slot allocas to registers to avoid storing it in the coroutine frame
Summary:
We don't want to store cleanup dest slot saved into the coroutine frame (as some of the cleanup code may
access them after coroutine frame destroyed).

This is an alternative to https://reviews.llvm.org/D37093

It is possible to do this for all functions, but, cursory check showed that in -O0, we get slightly longer function (by 1-3 instructions), thus, we are only limiting cleanup.dest.slot elimination to coroutines.

Reviewers: rjmccall, hfinkel, eric_niebler

Reviewed By: eric_niebler

Subscribers: EricWF, cfe-commits

Differential Revision: https://reviews.llvm.org/D39768

llvm-svn: 317981
2017-11-11 17:00:43 +00:00