[libunwind] df9a23e - [libunwind] Use `_dl_find_object` if available
Author: Adrian Vogelsgesang
Date: 2022-08-09T16:19:13-07:00
New Revision: df9a23e2feda18f0308b6d4dbd591ebe6b605aa4
URL:
https://github.com/llvm/llvm-project/commit/df9a23e2feda18f0308b6d4dbd591ebe6b605aa4
DIFF:
https://github.com/llvm/llvm-project/commit/df9a23e2feda18f0308b6d4dbd591ebe6b605aa4.diff
LOG: [libunwind] Use `_dl_find_object` if available
As shown in P2544R0 [1] and the accompanying benchmark [2], the
current unwinding logic does not scale for multi-threaded programs.
This is because `dl_iterate_phdr` takes a global lock.
glibc 2.35 added `_dl_find_object` which directly returns the unwind
info for a given target address. `_dl_find_object` is fully lock-free
and hence allows parallel exception unwinding on multiple threads.
With this commit, libunwind now takes advantage of `_dl_find_object`.
Thereby, this commit improves libunwind's performance on benchmark [2]
for unwinding exception on 20 threads from 1103ms to 78ms.
(measured on Intel Xeon Silver 4114 with 20 physical cores)
[1] https://isocpp.org/files/papers/P2544R0.html
[2] https://github.com/neumannt/exceptionperformance
Detailed performance numbers from the benchmark:
Before:
> Testing unwinding performance: sqrt computation with occasional errors
>
> testing baseline using 1 2 4 8 16 20 threads
> failure rate 0%: 34 35 34 35 35 36
> testing exceptions using 1 2 4 8 16 20 threads
> failure rate 0%: 16 32 33 34 35 36
> failure rate 0.1%: 16 32 34 36 35 36
> failure rate 1%: 20 40 40 43 90 113
> failure rate 10%: 59 92 140 304 880 1103
> [...]
>
> Testing invocation overhead: recursive fib with occasional errors
>
> testing exceptions using 1 2 4 8 16 20 threads
> failure rate 0%: 19 32 37 38 39 36
> failure rate 0.1%: 22 32 40 40 39 34
> failure rate 1%: 20 28 38 39 48 40
> failure rate 10%: 25 39 44 50 92 113
After:
> Testing unwinding performance: sqrt computation with occasional errors
>
> testing baseline using 1 2 4 8 16 20 threads
> failure rate 0%: 19 30 35 38 39 35
> testing baseline using 1 2 4 8 16 20 threads
> failure rate 0%: 32 35 33 34 34 36
> testing exceptions using 1 2 4 8 16 20 threads
> failure rate 0%: 16 35 33 37 35 35
> failure rate 0.1%: 16 32 36 33 34 37
> failure rate 1%: 21 37 39 40 40 41
> failure rate 10%: 72 75 76 80 80 78
> [...]
>
> Testing invocation overhead: recursive fib with occasional errors
>
> testing baseline using 1 2 4 8 16 20 threads
> failure rate 0%: 18 35 37 34 38 37
> testing exceptions using 1 2 4 8 16 20 threads
> failure rate 0%: 19 33 40 40 41 39
> failure rate 0.1%: 21 33 39 38 39 38
> failure rate 1%: 20 36 39 40 41 40
> failure rate 10%: 25 45 41 42 44 43
Differential Revision: https://reviews.llvm.org/D130668
Added:
Modified:
libunwind/src/AddressSpace.hpp
Removed:
diff --git a/libunwind/src/AddressSpace.hpp b/libunwind/src/AddressSpace.hpp
index 36c9f5a9e36f9..f1ba94ed732e3 100644
--- a/libunwind/src/AddressSpace.hpp
+++ b/libunwind/src/AddressSpace.hpp
@@ -601,6 +601,61 @@ inline bool LocalAddressSpace::findUnwindSections(pint_t
targetAddr,
if (info.arm_section && info.arm_section_length)
return true;
#elif defined(_LIBUNWIND_USE_DL_ITERATE_PHDR)
+ // Use DLFO_STRUCT_HAS_EH_DBASE to determine the existence of
+ // `_dl_find_object`. Use _LIBUNWIND_SUPPORT_DWARF_INDEX, because libunwind
+ // support for _dl_find_object on other unwind formats is not implemented,
+ // yet.
+#if defined(DLFO_STRUCT_HAS_EH_DBASE) & defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)
+ // We expect to run on a platform which does not use a base address for
+ // exception information.
+#if DLFO_STRUCT_HAS_EH_DBASE
+#error dlfo_eh_dbase is not supported for DWARF-based unwinding
+#endif
+ // We expect `_dl_find_object` to return PT_GNU_EH_FRAME.
+#if DLFO_EH_SEGMENT_TYPE != PT_GNU_EH_FRAME
+#error _dl_find_object retrieves an unexpected section type
+#endif
+ // We look-up `dl_find_object` dynamically at runtime to ensure backwards
+ // compatibility with earlier version of glibc not yet providing it. On older
+ // systems, we gracefully fallback to `dl_iterate_phdr`. Cache the pointer
+ // so we only look it up once. Do manual lock to avoid _cxa_guard_acquire.
+ static decltype(_dl_find_object) *dlFindObject;
+ static bool dlFindObjectChecked = false;
+ if (!dlFindObjectChecked) {
+dlFindObject = reinterpret_cast(
+dlsym(RTLD_DEFAULT, "_dl_find_object"));
+dlFindObjectChecked = true;
+ }
+ // Try to find the unwind info using `dl_find_object`
+ dl_find_object findResult;
+ if (dlFindObject && dlFindObject((void *)targetAddr, &findResult) == 0) {
+if (findResult.dlfo_eh_frame == nullptr) {
+ // Found an entry for `targetAddr`, but there is no unwind info.
+ return false;
+}
+info.dso_base = reinterpret_cast(findResult.dlfo_map_start);
+info.text_segment_length = static_cast(
+(char *)findResult.dlfo_map_end - (char *)findRes
[clang] 9138900 - [LLDB] Add data formatter for std::coroutine_handle
Author: Adrian Vogelsgesang
Date: 2022-08-24T14:40:53-07:00
New Revision: 91389000abe8ef5d06d98cbbefd3fa03ac7e4480
URL:
https://github.com/llvm/llvm-project/commit/91389000abe8ef5d06d98cbbefd3fa03ac7e4480
DIFF:
https://github.com/llvm/llvm-project/commit/91389000abe8ef5d06d98cbbefd3fa03ac7e4480.diff
LOG: [LLDB] Add data formatter for std::coroutine_handle
This patch adds a formatter for `std::coroutine_handle`, both for libc++
and libstdc++. For the type-erased `coroutine_handle<>`, it shows the
`resume` and `destroy` function pointers. For a non-type-erased
`coroutine_handle` it also shows the `promise` value.
With this change, executing the `v t` command on the example from
https://clang.llvm.org/docs/DebuggingCoroutines.html now outputs
```
(task) t = {
handle = coro frame = 0xb2a0 {
resume = 0x5a10 (a.out`coro_task(int, int) at
llvm-example.cpp:36)
destroy = 0x6090 (a.out`coro_task(int, int) at
llvm-example.cpp:36)
}
}
```
instead of just
```
(task) t = {
handle = {
__handle_ = 0xb2a0
}
}
```
Note, how the symbols for the `resume` and `destroy` function pointer
reveal which coroutine is stored inside the `std::coroutine_handle`.
A follow-up commit will use this fact to infer the coroutine's promise
type and the representation of its internal coroutine state based on
the `resume` and `destroy` pointers.
The same formatter is used for both libc++ and libstdc++. It would
also work for MSVC's standard library, however it is not registered
for MSVC, given that lldb does not provide pretty printers for other
MSVC types, either.
The formatter is in a newly added `Coroutines.{h,cpp}` file because there
does not seem to be an already existing place where we could share
formatters across libc++ and libstdc++. Also, I expect this code to grow
as we improve debugging experience for coroutines further.
**Testing**
* Added API test
Differential Revision: https://reviews.llvm.org/D132415
Added:
lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/Makefile
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
Modified:
clang/docs/tools/clang-formatted-files.txt
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/packages/Python/lldbsuite/test/lldbutil.py
lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
Removed:
diff --git a/clang/docs/tools/clang-formatted-files.txt
b/clang/docs/tools/clang-formatted-files.txt
index f89e19ca7cd3a..c49acfaec58aa 100644
--- a/clang/docs/tools/clang-formatted-files.txt
+++ b/clang/docs/tools/clang-formatted-files.txt
@@ -4180,6 +4180,8 @@
lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.h
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.h
+lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
+lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.h
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 69bb5ac5629e4..1c090395e6c4c 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -292,8 +292,12 @@ def check_value(self, test_base, val, error_msg=None):
test_base.assertEqual(self.expect_type, val.GetDisplayTypeName(),
this_error_msg)
if self.expect_summary:
-test_base.assertEqual(self.expect_summary, val.GetSummary(),
- this_error_msg)
+if isinstance(self.expect_summary, re.Pattern):
+test_base.assertRegex(val.GetSummary(), self.expect_summary,
+ this_error_msg)
+else:
+test_base.assertEqual(self.expect_summary, val.GetSummary(),
+ this_error_msg)
if self.children is not None:
self.check_value_children(test_base, val, error_msg)
diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py
b/lldb/packages/Python/lldbsuite/test/lldbutil.py
index 8bd49c742cb04..7e64afb3639cd 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -988,6 +988,21 @@ def continue_to_breakpoin
[libunwind] c9cffdd - [libunwind] Fix usage of `_dl_find_object` on 32-bit x86
Author: Adrian Vogelsgesang Date: 2022-09-16T06:29:49-07:00 New Revision: c9cffdde393f646acf62d6160e7fa6613e038508 URL: https://github.com/llvm/llvm-project/commit/c9cffdde393f646acf62d6160e7fa6613e038508 DIFF: https://github.com/llvm/llvm-project/commit/c9cffdde393f646acf62d6160e7fa6613e038508.diff LOG: [libunwind] Fix usage of `_dl_find_object` on 32-bit x86 On 32-bit x86, `_dl_find_object` also returns a `dlfo_eh_dbase` address. So far, compiling against a version of `_dl_find_object` which returns a `dlfo_eh_dbase` was blocked using a `#if` + `#error`. This commit now removes this compile time assertion and simply ignores the returned `dlfo_eh_dbase`. All test cases are passing on a 32-bit build now. According to https://www.gnu.org/software/libc/manual/html_node/Dynamic-Linker-Introspection.html, `dlfo_eh_dbase` should be the base address for all DW_EH_PE_datarel relocations. However, glibc/elf/dl-find_object.h says that eh_dbase is the relocated DT_PLTGOT value. I don't understand how those two statements fit together, but to fix 32-bit x86, ignoring `dlfo_eh_dbase` seems to be good enough. Fixes #57733 Differential Revision: https://reviews.llvm.org/D133846 Added: Modified: libunwind/src/AddressSpace.hpp Removed: diff --git a/libunwind/src/AddressSpace.hpp b/libunwind/src/AddressSpace.hpp index 315289cd4211b..b0135b0c0519c 100644 --- a/libunwind/src/AddressSpace.hpp +++ b/libunwind/src/AddressSpace.hpp @@ -584,11 +584,6 @@ inline bool LocalAddressSpace::findUnwindSections(pint_t targetAddr, // support for _dl_find_object on other unwind formats is not implemented, // yet. #if defined(DLFO_STRUCT_HAS_EH_DBASE) & defined(_LIBUNWIND_SUPPORT_DWARF_INDEX) - // We expect to run on a platform which does not use a base address for - // exception information. -#if DLFO_STRUCT_HAS_EH_DBASE -#error dlfo_eh_dbase is not supported for DWARF-based unwinding -#endif // We expect `_dl_find_object` to return PT_GNU_EH_FRAME. #if DLFO_EH_SEGMENT_TYPE != PT_GNU_EH_FRAME #error _dl_find_object retrieves an unexpected section type ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] fda7778 - [clang-tidy] Update tests to include C++23 and C++26
Author: Adrian Vogelsgesang
Date: 2023-08-07T21:25:22Z
New Revision: fda777849b0088ba83e28683c53c5c8321ef2558
URL:
https://github.com/llvm/llvm-project/commit/fda777849b0088ba83e28683c53c5c8321ef2558
DIFF:
https://github.com/llvm/llvm-project/commit/fda777849b0088ba83e28683c53c5c8321ef2558.diff
LOG: [clang-tidy] Update tests to include C++23 and C++26
This commit changes the `c++xx-or-later` definitions to also include
C++23 and the upcoming C++26.
`readability/container-contains.cpp` to also test newer C++ versions.
Also, this commit adjusts a couple of test cases slightly:
* `container-contains.cpp` now also tests newer C++ versions.
Restricting it to C++20 was an oversight of mine when originally
writing this check.
* `unconventional-assign-operator.cpp`: The `return rhs` raised
a "non-const lvalue reference to type 'BadReturnStatement' cannot
bind to a temporary" error in C++23. The issue is circumenvented
by writing `return *&rhs`.
* `const-correctness-values.cpp` was also running into the same error
in C++23. The troublesome test cases were moved to a separate file.
Differential Revision: https://reviews.llvm.org/D157246
Added:
clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values-before-cxx23.cpp
Modified:
clang-tools-extra/test/clang-tidy/check_clang_tidy.py
clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
clang-tools-extra/test/clang-tidy/checkers/misc/unconventional-assign-operator.cpp
clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp
Removed:
diff --git a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
index a78996a0fce3b0..53ffca0bad8d06 100755
--- a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -265,15 +265,17 @@ def run(self):
def expand_std(std):
if std == "c++98-or-later":
-return ["c++98", "c++11", "c++14", "c++17", "c++20"]
+return ["c++98", "c++11", "c++14", "c++17", "c++20", "c++23", "c++2c"]
if std == "c++11-or-later":
-return ["c++11", "c++14", "c++17", "c++20"]
+return ["c++11", "c++14", "c++17", "c++20", "c++23", "c++2c"]
if std == "c++14-or-later":
-return ["c++14", "c++17", "c++20"]
+return ["c++14", "c++17", "c++20", "c++23", "c++2c"]
if std == "c++17-or-later":
-return ["c++17", "c++20"]
+return ["c++17", "c++20", "c++23", "c++2c"]
if std == "c++20-or-later":
-return ["c++20"]
+return ["c++20", "c++23", "c++2c"]
+if std == "c++23-or-later":
+return ["c++23", "c++2c"]
return [std]
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values-before-cxx23.cpp
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values-before-cxx23.cpp
new file mode 100644
index 00..3547ec080911eb
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values-before-cxx23.cpp
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17,c++20 %s
misc-const-correctness %t -- \
+// RUN: -config="{CheckOptions: {\
+// RUN: misc-const-correctness.TransformValues: true, \
+// RUN: misc-const-correctness.WarnPointersAsValues: false, \
+// RUN: misc-const-correctness.TransformPointersAsValues: false \
+// RUN: }}" -- -fno-delayed-template-parsing
+
+
+double &non_const_ref_return() {
+ double p_local0 = 0.0;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type
'double' can be declared 'const'
+ // CHECK-FIXES: double const p_local0
+ double np_local0 = 42.42;
+ return np_local0;
+}
+
+double *&return_non_const_pointer_ref() {
+ double *np_local0 = nullptr;
+ return np_local0;
+}
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
index 88f40462003e88..186e3cf5a179b2 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
@@ -1,9 +1,9 @@
// RUN: %check_clang_tidy %s misc-const-correctness %t -- \
// RUN: -config="{CheckOptions: {\
-// RUN: misc-const-correctness.TransformValues: true, \
-// RUN: misc-const-correctness.WarnPointersAsValues: false, \
-// RUN: misc-const-correctness.TransformPointersAsValues: false} \
-// RUN: }" -- -fno-delayed-template-parsing
+// RUN: misc-const-correctness.TransformValues: true, \
+// RUN: misc-const-correctness.WarnPointersAsValues: false, \
+// RUN: misc-const-correctness.TransformPointersAsValues: false \
+// RUN: }}" -- -fno-delayed-template-parsing
// --- Provide test samples for primitive builtins
[clang] [llvm] [Clang][Coroutines] Introducing the `[[clang::coro_inplace_task]]` attribute (PR #94693)
@@ -0,0 +1,84 @@
+// This file tests the coro_structured_concurrency attribute semantics.
+// RUN: %clang_cc1 -std=c++20 -disable-llvm-passes -emit-llvm %s -o - |
FileCheck %s
+
+#include "Inputs/coroutine.h"
+#include "Inputs/utility.h"
+
+template
+struct [[clang::coro_structured_concurrency]] Task {
vogelsgesang wrote:
Afaict, this particular `Task` can also be annotated using
`coro_only_destroy_when_complete`?
I guess many libraries would want to use both `coro_structured_concurrency` /
`coro_inplace_task` and `coro_only_destroy_when_complete`. Do those
optimizations interact well with each other? Does it make sense to add a test
case which tests both attributes together?
https://github.com/llvm/llvm-project/pull/94693
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][Coroutines] Introducing the `[[clang::coro_inplace_task]]` attribute (PR #94693)
@@ -0,0 +1,84 @@
+// This file tests the coro_structured_concurrency attribute semantics.
+// RUN: %clang_cc1 -std=c++20 -disable-llvm-passes -emit-llvm %s -o - |
FileCheck %s
+
+#include "Inputs/coroutine.h"
+#include "Inputs/utility.h"
+
+template
+struct [[clang::coro_structured_concurrency]] Task {
vogelsgesang wrote:
> through a handle you smuggled with the task, which this type does not allow
> to do
that's the point I am getting at. Most `task` types do not allow smuggling
handles out of the wrappers types. As such, I expect that many task types would
be annotated with both `coro_structured_concurrency` / `coro_inplace_task` and
`coro_only_destroy_when_complete`. I guess we should have a test case that both
attributes interact nicely with each other?
https://github.com/llvm/llvm-project/pull/94693
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][Coroutines] Introducing the `[[clang::coro_inplace_task]]` attribute (PR #94693)
https://github.com/vogelsgesang edited https://github.com/llvm/llvm-project/pull/94693 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][Coroutines] Introducing the `[[clang::coro_inplace_task]]` attribute (PR #94693)
https://github.com/vogelsgesang edited https://github.com/llvm/llvm-project/pull/94693 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_await_elidable]] (PR #99282)
@@ -825,6 +826,32 @@ ExprResult Sema::BuildOperatorCoawaitLookupExpr(Scope *S,
SourceLocation Loc) {
return CoawaitOp;
}
+static bool isAttributedCoroInplaceTask(const QualType &QT) {
+ auto *Record = QT->getAsCXXRecordDecl();
+ return Record && Record->hasAttr();
+}
+
+static bool isCoroInplaceCall(Expr *Operand) {
+ if (!Operand->isPRValue()) {
+return false;
+ }
+
+ return isAttributedCoroInplaceTask(Operand->getType());
+}
+
+template
+DesiredExpr *getExprWrappedByTemporary(Expr *E) {
vogelsgesang wrote:
seems unused?
https://github.com/llvm/llvm-project/pull/99282
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_await_elidable]] (PR #99282)
@@ -8108,6 +8108,24 @@ but do not pass them to the underlying coroutine or pass
them by value.
}];
}
+def CoroAwaitElidableDoc : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{
+The ``[[clang::coro_await_elidable]]`` is a class attribute which can be
applied
+to a coroutine return type.
+
+When a coroutine function that returns such a type calls another coroutine
function,
+the compiler performs heap allocation elision when the following conditions
are all met:
+- callee coroutine function returns a type that is annotated with
``[[clang::coro_await_elidable]]``.
+- In caller coroutine, the return value of the callee is a prvalue or an
xvalue, and
+- The temporary expression containing the callee coroutine object is
immediately co_awaited.
+
+The behavior is undefined if any of the following condition was met:
+- the caller coroutine is destroyed earlier than the callee coroutine.
vogelsgesang wrote:
Are we expecting this list to contain more than one entry soon? Otherwise, I
think we can simplify this
```suggestion
The behavior is undefined if the caller coroutine is destroyed earlier than the
callee coroutine.
```
https://github.com/llvm/llvm-project/pull/99282
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_await_elidable]] (PR #99282)
@@ -8108,6 +8108,24 @@ but do not pass them to the underlying coroutine or pass
them by value.
}];
}
+def CoroAwaitElidableDoc : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{
+The ``[[clang::coro_await_elidable]]`` is a class attribute which can be
applied
+to a coroutine return type.
+
+When a coroutine function that returns such a type calls another coroutine
function,
+the compiler performs heap allocation elision when the following conditions
are all met:
+- callee coroutine function returns a type that is annotated with
``[[clang::coro_await_elidable]]``.
+- In caller coroutine, the return value of the callee is a prvalue or an
xvalue, and
+- The temporary expression containing the callee coroutine object is
immediately co_awaited.
vogelsgesang wrote:
```suggestion
- The callee coroutine function returns a type that is annotated with
``[[clang::coro_await_elidable]]``.
- In the caller coroutine, the return value of the callee is a prvalue or an
xvalue.
- The temporary expression containing the callee coroutine object is
immediately co_awaited.
```
(why remove the "and" in the 2nd bullet point? For consistency. Because
otherwise, the 1st bullet point is also a self-contained sentence with a dot at
its end)
https://github.com/llvm/llvm-project/pull/99282
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] C++20 Coroutines: Introduce Frontend Attribute [[clang::coro_await_elidable]] (PR #99282)
@@ -1217,6 +1217,14 @@ def CoroDisableLifetimeBound : InheritableAttr {
let SimpleHandler = 1;
}
+def CoroAwaitElidable : InheritableAttr {
vogelsgesang wrote:
does this also warrant an entry in `clang/docs/ReleaseNotes.rst`?
https://github.com/llvm/llvm-project/pull/99282
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement C++26 Attributes for Structured Bindings (P0609R3) (PR #89906)
@@ -187,7 +187,7 @@ C++2c implementation status Trivial infinite loops are not Undefined Behavior https://wg21.link/P2809R3";>P2809R3 (DR) - No + Clang 19 vogelsgesang wrote: the wrong paper was marked as implemented https://github.com/llvm/llvm-project/pull/89906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Address post commit feedbacks in #89906 (PR #90495)
@@ -177,7 +177,7 @@ C++2c implementation status Attributes for Structured Bindings https://wg21.link/P0609R3";>P0609R3 - No + Clang 19 vogelsgesang wrote: none -> unreleased But maybe just fix this together when landing #90066... https://github.com/llvm/llvm-project/pull/90495 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Address post commit feedbacks in #89906 (PR #90495)
@@ -187,7 +187,7 @@ C++2c implementation status Trivial infinite loops are not Undefined Behavior https://wg21.link/P2809R3";>P2809R3 (DR) - Clang 19 + No vogelsgesang wrote: unreleased -> none https://github.com/llvm/llvm-project/pull/90495 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement C++26 P2748R5 "Disallow Binding a Returned Glvalue to a Temporary" (PR #89942)
@@ -167,7 +167,7 @@ C++2c implementation status Disallow Binding a Returned Glvalue to a Temporary https://wg21.link/P2748R5";>P2748R5 - No + Clang 19 vogelsgesang wrote: Should be `class="unreleased"` instead of `class="full"` https://github.com/llvm/llvm-project/pull/89942 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement C++26 P2748R5 "Disallow Binding a Returned Glvalue to a Temporary" (PR #89942)
@@ -167,7 +167,7 @@ C++2c implementation status Disallow Binding a Returned Glvalue to a Temporary https://wg21.link/P2748R5";>P2748R5 - No + Clang 19 vogelsgesang wrote: Ah, I just saw this was already fixed in the meantime - nevermind 🙂 https://github.com/llvm/llvm-project/pull/89942 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Introduce -defer-thinlto-prelink-coro-split that skips Coro passes in ThinLTO pre-link pipeline (PR #107153)
vogelsgesang wrote: My 2cts: I think it would be best for clang-users, if HALO / coroutine optimizations work out-of-the-box in as many cases as possible. Unfortunately, it seems we have an "either-or" decision here. Either, the "thin-lto + HALO" work out-of-the-box or the llc-code-generation works out-of-the-box. Afaict, thin-lto is the more common use case compared to calling llc directly. At least there seems to be documentation for [thin-lto](https://clang.llvm.org/docs/ThinLTO.html) but not for the llc use case? As a normal C++ programmer and clang user, I doubt that I would ever find the `-defer-thinlto-prelink-coro-split` setting, in particular given that this commit does not add any documentation for it. As such, I would be stuck without cross-TU link-time HALO optimizations. At the same time, I would expect people which use custom llc-based compilation to be more experienced with setting up an LLVM toolchain, and more enabled to figure out about the existince of the flag. All of this is to say: I think the default should be flipped. **By default, coro-split should be deferred to post-link** (such that the more common thin-lto setup can apply HALO out-of-the-box). More advanced users, who call llc directly, can still use a flag to defer coro-split. In addition, some documentation would be great (e.g. mentioning the newly introduced flag in https://clang.llvm.org/docs/ThinLTO.html and https://llvm.org/docs/Coroutines.html https://github.com/llvm/llvm-project/pull/107153 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Introduce -defer-thinlto-prelink-coro-split that skips Coro passes in ThinLTO pre-link pipeline (PR #107153)
vogelsgesang wrote: > Unfortunately, it seems we have an "either-or" decision here Although... Is this even an either-or decision? Could we add coro-split to the llc unconditionally, also for non-thin-lto pipelines? Afaict, coro-split is a no-op pass if coroutines were already split previously. Hence, codegen would simply work, even if a thin-lto-prelink pipeline is combined with a non-thin-lto post-link step. Doing so, I think we wouldn't need to introduce a flag at all? https://github.com/llvm/llvm-project/pull/107153 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -249,7 +249,10 @@ Attribute Changes in Clang (#GH106864) - Introduced a new attribute ``[[clang::coro_await_elidable]]`` on coroutine return types - to express elideability at call sites where the coroutine is co_awaited as a prvalue. + to express elideability at call sites where the coroutine is invoked under a safe elide context. + +- Introduced a new attribute ``[[clang::coro_must_await]]`` on function parameters to vogelsgesang wrote: What do you think about `[[clang::coro_await_elideable_argument]]`? I would slightly prefer this over `[[clang::coro_elideable_await_argument]]` because it is more similar to its sibling argument `[[clang::coro_await_elideable]]`, and the duality between the two arguments becomes more obvious already from their names. I think we should particularly try to avoid any name with `must` in its name. To me, `must` expresses a guarantee given by the compiler to the programmer (such as for the `[[musttail]]` attribute which will fail compilation in case the call cannot be tail-called), enforced by the compiler. However, this attribute behaves the other way around: It expresses a guarantee given by the programmer to the compiler, the compiler does nothing to check or enforce the correctness of it. https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8281,8 +8288,62 @@ Example:
co_await t;
}
-The behavior is undefined if the caller coroutine is destroyed earlier than the
-callee coroutine.
+Such elision replaces the heap allocated activation frame of the callee
coroutine
+with a local variable within the enclosing braces in the caller's stack frame.
+The local variable, like other variables in coroutines, may be collected into
the
+coroutine frame, which may be allocated on the heap. The behavior is undefined
+if the caller coroutine is destroyed earlier than the callee coroutine.
+
+}];
+}
+
+def CoroMustAwaitDoc : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{
+
+The ``[[clang::coro_must_await]]`` is a function parameter attribute. It works
+in conjunction with ``[[clang::coro_await_elidable]]`` to propagate a safe
elide
+context to a parameter or parameter pack if the function is called under a safe
+elide context.
+
+This is sometimes necessary on utility functions whose role is to compose or
+modify the behavior of a callee coroutine.
vogelsgesang wrote:
```suggestion
This is sometimes necessary on utility functions used to compose or
modify the behavior of a callee coroutine.
```
https://github.com/llvm/llvm-project/pull/108474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8261,12 +8261,19 @@ def CoroAwaitElidableDoc : Documentation {
The ``[[clang::coro_await_elidable]]`` is a class attribute which can be
applied
to a coroutine return type.
-When a coroutine function that returns such a type calls another coroutine
function,
-the compiler performs heap allocation elision when the call to the coroutine
function
-is immediately co_awaited as a prvalue. In this case, the coroutine frame for
the
-callee will be a local variable within the enclosing braces in the caller's
stack
-frame. And the local variable, like other variables in coroutines, may be
collected
-into the coroutine frame, which may be allocated on the heap.
+When a coroutine function returns such a type, a direct call expression therein
+that returns a prvalue of a type attributed ``[[clang::coro_await_elidable]]``
+is said to be under a safe elide context if one of the following is true:
vogelsgesang wrote:
> When a coroutine function returns such a type, a direct call expression
> therein that returns a prvalue of a type attributed
> ``[[clang::coro_await_elidable]]`` [...]
If I am reading the code correctly, it is not actually a requirement that the
containing / callee coroutine itself is also annotated as
`coro_await_elidable`. Only the called coroutine needs to be annotated as
`coro_await_elidable`.
I.e., we would also apply HALO for something like
```
class [[clang::coro_await_elidable]] ElidableTask { ... };
class NonElidableTask { ... };
ElidableTask foo();
NonElidableTask bar() {
co_await foo(); // foo()'s coroutine frame on this line is elidable
}
```
Or did I misread the code?
Assuming I understood code correctly, I would propose something like
```
A call to a coroutine function returning such a type is said to be safe-to-elide
if one of the following is true:
- it is the immediate right-hand side operand to a co_await expression.
- it is an argument to a [[clang::coro_must_await]] parameter or parameter pack
of another safe-to-elide function call.
Do note that the safe-to-elide context applies only to the call expression
itself,
and the context does not transitively include any of its subexpressions unless
``[[clang::coro_must_await]]`` is used to opt-in to transivitely applying
safe-to-elide.
The compiler performs heap allocation elision on call expressions on
safe-to-elide
calls, if the callee is a coroutine.
```
https://github.com/llvm/llvm-project/pull/108474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8281,8 +8288,62 @@ Example:
co_await t;
}
-The behavior is undefined if the caller coroutine is destroyed earlier than the
-callee coroutine.
+Such elision replaces the heap allocated activation frame of the callee
coroutine
+with a local variable within the enclosing braces in the caller's stack frame.
+The local variable, like other variables in coroutines, may be collected into
the
+coroutine frame, which may be allocated on the heap. The behavior is undefined
+if the caller coroutine is destroyed earlier than the callee coroutine.
+
+}];
+}
+
+def CoroMustAwaitDoc : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{
+
+The ``[[clang::coro_must_await]]`` is a function parameter attribute. It works
+in conjunction with ``[[clang::coro_await_elidable]]`` to propagate a safe
elide
+context to a parameter or parameter pack if the function is called under a safe
+elide context.
+
+This is sometimes necessary on utility functions whose role is to compose or
+modify the behavior of a callee coroutine.
+
+Example:
+
+.. code-block:: c++
+
+ template
+ class [[clang::coro_await_elidable]] Task { ... };
+
+ template
+ class [[clang::coro_await_elidable]] WhenAll { ... };
+
+ // `when_all` is a utility function that compose coroutines. It does not need
vogelsgesang wrote:
```suggestion
// `when_all` is a utility function that composes coroutines. It does not need
```
https://github.com/llvm/llvm-project/pull/108474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8281,8 +8288,62 @@ Example:
co_await t;
}
-The behavior is undefined if the caller coroutine is destroyed earlier than the
-callee coroutine.
+Such elision replaces the heap allocated activation frame of the callee
coroutine
+with a local variable within the enclosing braces in the caller's stack frame.
+The local variable, like other variables in coroutines, may be collected into
the
+coroutine frame, which may be allocated on the heap. The behavior is undefined
+if the caller coroutine is destroyed earlier than the callee coroutine.
+
+}];
+}
+
+def CoroMustAwaitDoc : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{
+
+The ``[[clang::coro_must_await]]`` is a function parameter attribute. It works
+in conjunction with ``[[clang::coro_await_elidable]]`` to propagate a safe
elide
+context to a parameter or parameter pack if the function is called under a safe
+elide context.
+
+This is sometimes necessary on utility functions whose role is to compose or
+modify the behavior of a callee coroutine.
+
+Example:
+
+.. code-block:: c++
+
+ template
+ class [[clang::coro_await_elidable]] Task { ... };
+
+ template
+ class [[clang::coro_await_elidable]] WhenAll { ... };
+
+ // `when_all` is a utility function that compose coroutines. It does not need
+ // to be a coroutine to propagate.
+ template
+ WhenAll when_all([[clang::coro_must_await]] Task tasks...);
+
+ Task foo();
+ Task bar();
+ Task example1() {
+// `when_all``, `foo``, and `bar` are all elide safe because `when_all` is
+// under a safe elide context and propagated such contexts to foo and bar.
vogelsgesang wrote:
```suggestion
// under a safe elide context and, thanks to the [[coro_must_await]]
attribute,
// this context is propagated to foo and bar.
```
https://github.com/llvm/llvm-project/pull/108474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8261,12 +8261,19 @@ def CoroAwaitElidableDoc : Documentation {
The ``[[clang::coro_await_elidable]]`` is a class attribute which can be
applied
to a coroutine return type.
vogelsgesang wrote:
```suggestion
to a coroutine return type. It provides a hint to the compiler to apply Heap
Allocation Elision
more aggressively.
```
I like it if the first paragraph of any documentation immediately gives me a
high-level picture, so I know if I should keep reading the rest of its
documentation or skip it. By immediately mentioning Heap Allocation Elision in
the first paragraph, the reader also gets more context when read the rest of
the paragraph, so he will hopefully be able to understand the following
paragraphs more easily
https://github.com/llvm/llvm-project/pull/108474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -84,4 +84,35 @@ Task nonelidable() {
co_return 1;
}
+// CHECK-LABEL: define{{.*}} @_Z8addTasksO4TaskIiES1_{{.*}} {
+Task addTasks([[clang::coro_must_await]] Task &&t1, Task &&t2) {
+ int i1 = co_await t1;
+ int i2 = co_await t2;
+ co_return i1 + i2;
+}
+
+// CHECK-LABEL: define{{.*}} @_Z10returnSamei{{.*}} {
+Task returnSame(int i) {
+ co_return i;
+}
+
+// CHECK-LABEL: define{{.*}} @_Z21elidableWithMustAwaitv{{.*}} {
+Task elidableWithMustAwait() {
+ // CHECK: call void @_Z10returnSamei(ptr {{.*}}, i32 noundef 2)
#[[ELIDE_SAFE]]
+ // CHECK-NOT: call void @_Z10returnSamei(ptr {{.*}}, i32 noundef 3)
#[[ELIDE_SAFE]]
+ co_return co_await addTasks(returnSame(2), returnSame(3));
+}
+
+template
+Task sumAll([[clang::coro_must_await]] Args && ... tasks);
+
+// CHECK-LABEL: define{{.*}} @_Z16elidableWithPackv{{.*}} {
+Task elidableWithPack() {
vogelsgesang wrote:
maybe also add a test case for recursive elision? I.e. for something like
```
co_await sumAll(addTasks(returnSame(1), returnSame(2)), returnSame(3));
```
https://github.com/llvm/llvm-project/pull/108474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -84,4 +84,35 @@ Task nonelidable() {
co_return 1;
}
+// CHECK-LABEL: define{{.*}} @_Z8addTasksO4TaskIiES1_{{.*}} {
+Task addTasks([[clang::coro_must_await]] Task &&t1, Task &&t2) {
+ int i1 = co_await t1;
+ int i2 = co_await t2;
+ co_return i1 + i2;
+}
+
+// CHECK-LABEL: define{{.*}} @_Z10returnSamei{{.*}} {
+Task returnSame(int i) {
+ co_return i;
+}
+
+// CHECK-LABEL: define{{.*}} @_Z21elidableWithMustAwaitv{{.*}} {
+Task elidableWithMustAwait() {
+ // CHECK: call void @_Z10returnSamei(ptr {{.*}}, i32 noundef 2)
#[[ELIDE_SAFE]]
+ // CHECK-NOT: call void @_Z10returnSamei(ptr {{.*}}, i32 noundef 3)
#[[ELIDE_SAFE]]
vogelsgesang wrote:
Can we somehow turn this into a positive test? Negative tests can easily
regress, e.g. if something else in the line would change.
Can we use a `$` (or some other mechanism) to check for the end of the line,
and there by explicitly check that a call to `_Z10returnSamei(ptr {{.*}}, i32
noundef 3)` does exist, but does not have the ELIDE_SAFE annotation?
```suggestion
// CHECK: call void @_Z10returnSamei(ptr {{.*}}, i32 noundef 3)$
```
https://github.com/llvm/llvm-project/pull/108474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -880,14 +898,12 @@ ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *Operand, } auto *RD = Promise->getType()->getAsCXXRecordDecl(); - bool AwaitElidable = - isCoroAwaitElidableCall(Operand) && - isAttributedCoroAwaitElidable( - getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType()); - - if (AwaitElidable) -if (auto *Call = dyn_cast(Operand->IgnoreImplicit())) - Call->setCoroElideSafe(); + + bool CurFnAwaitElidable = isAttributedCoroAwaitElidable( + getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType()); vogelsgesang wrote: shouldn't we actually check if the return type of àwait_transform` is marked as elideable, instead of the immediate argument to `co_await`? Or maybe we should check that both the immediate argument and the transformed awaitable are annotated? (Not really related to this commit. I just noticed this as I was reviewing this code change here. Fell free to ship this PR without addressing this comment. In case this is actually something we want to change, we should probably do so in a separate PR, anyway...) https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8261,12 +8261,19 @@ def CoroAwaitElidableDoc : Documentation {
The ``[[clang::coro_await_elidable]]`` is a class attribute which can be
applied
to a coroutine return type.
-When a coroutine function that returns such a type calls another coroutine
function,
-the compiler performs heap allocation elision when the call to the coroutine
function
-is immediately co_awaited as a prvalue. In this case, the coroutine frame for
the
-callee will be a local variable within the enclosing braces in the caller's
stack
-frame. And the local variable, like other variables in coroutines, may be
collected
-into the coroutine frame, which may be allocated on the heap.
+When a coroutine function returns such a type, a direct call expression therein
+that returns a prvalue of a type attributed ``[[clang::coro_await_elidable]]``
+is said to be under a safe elide context if one of the following is true:
+- it is the immediate right-hand side operand to a co_await expression.
+- it is an argument to a [[clang::coro_must_await]] parameter or parameter pack
vogelsgesang wrote:
```suggestion
- it is an argument to a ``[[clang::coro_must_await]]`` parameter or parameter
pack
```
https://github.com/llvm/llvm-project/pull/108474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
https://github.com/vogelsgesang commented: Thanks for implementing this! Looking forward to using it 🙂 https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
https://github.com/vogelsgesang edited https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
https://github.com/vogelsgesang edited https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -880,14 +898,12 @@ ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *Operand, } auto *RD = Promise->getType()->getAsCXXRecordDecl(); - bool AwaitElidable = - isCoroAwaitElidableCall(Operand) && - isAttributedCoroAwaitElidable( - getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType()); - - if (AwaitElidable) -if (auto *Call = dyn_cast(Operand->IgnoreImplicit())) - Call->setCoroElideSafe(); + + bool CurFnAwaitElidable = isAttributedCoroAwaitElidable( + getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType()); vogelsgesang wrote: ah, I see. So after this patch, we could actually check `await_transform` and also `operator co_await` and thereby get rid of the `CurFnAwaitElidable` check? Such that a `ElideableTask` coawaited within a NonElidableTask (as discussed in the other thread) would become elideable? https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8261,12 +8261,19 @@ def CoroAwaitElidableDoc : Documentation {
The ``[[clang::coro_await_elidable]]`` is a class attribute which can be
applied
to a coroutine return type.
-When a coroutine function that returns such a type calls another coroutine
function,
-the compiler performs heap allocation elision when the call to the coroutine
function
-is immediately co_awaited as a prvalue. In this case, the coroutine frame for
the
-callee will be a local variable within the enclosing braces in the caller's
stack
-frame. And the local variable, like other variables in coroutines, may be
collected
-into the coroutine frame, which may be allocated on the heap.
+When a coroutine function returns such a type, a direct call expression therein
+that returns a prvalue of a type attributed ``[[clang::coro_await_elidable]]``
+is said to be under a safe elide context if one of the following is true:
vogelsgesang wrote:
Ah, right. I misread the code.
In general: Is checking if the callee coroutine is annotated actually
necessary? To my understanding from the other thread, so far it was necessary
because of `await_transform` and `operator co_await`. After we make this
checking more fine-granular, are there any other reasons why we would still
need to check if the callee coroutine is annotated?
https://github.com/llvm/llvm-project/pull/108474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_await_elidable_argument]] (PR #108474)
@@ -880,14 +898,12 @@ ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *Operand, } auto *RD = Promise->getType()->getAsCXXRecordDecl(); - bool AwaitElidable = - isCoroAwaitElidableCall(Operand) && - isAttributedCoroAwaitElidable( - getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType()); - - if (AwaitElidable) -if (auto *Call = dyn_cast(Operand->IgnoreImplicit())) - Call->setCoroElideSafe(); + + bool CurFnAwaitElidable = isAttributedCoroAwaitElidable( + getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType()); vogelsgesang wrote: 👍 https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_await_elidable_argument]] (PR #108474)
https://github.com/vogelsgesang commented: Looks good to me, but at the same time: I am not experienced in the clang-frontend, so please wait for an approval of a more experienced clang-frontend contributor https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
