Skip to content

Commit

Permalink
[Coroutines] Remove unused coroutine builtin/intrinsics llvm.coro.par…
Browse files Browse the repository at this point in the history
…am (NFC-ish)

I found that the coroutine intrinsic llvm.coro.param in documentation
(https://llvm.org/docs/Coroutines.html#id101) didn't get used actually
since there isn't lowering codes in LLVM. I also checked the
implementation of libstdc++ and libc++. Both of them didn't use
llvm.coro.param. So I am pretty sure that the llvm.coro.param intrinsic
is unused. I think it would be better t to remove it to avoid possible
misleading understandings.

Note: according to [class.copy.elision]/p1.3, this optimization is
allowed by the C++ language specification. Let's make it someday.

Reviewed By: rjmccall

Differential Revision: https://reviews.llvm.org/D115222
  • Loading branch information
ChuanqiXu9 committed Dec 9, 2021
1 parent 6d7c9c3 commit 352e36e
Show file tree
Hide file tree
Showing 8 changed files with 0 additions and 100 deletions.
1 change: 0 additions & 1 deletion clang/docs/LanguageExtensions.rst
Expand Up @@ -3007,7 +3007,6 @@ an appropriate value during the emission.
void *__builtin_coro_begin(void *memory)
void __builtin_coro_end(void *coro_frame, bool unwind)
char __builtin_coro_suspend(bool final)
bool __builtin_coro_param(void *original, void *copy)
Note that there is no builtin matching the `llvm.coro.save` intrinsic. LLVM
automatically will insert one if the first argument to `llvm.coro.suspend` is
Expand Down
1 change: 0 additions & 1 deletion clang/include/clang/Basic/Builtins.def
Expand Up @@ -1612,7 +1612,6 @@ LANGBUILTIN(__builtin_coro_alloc, "b", "n", COR_LANG)
LANGBUILTIN(__builtin_coro_begin, "v*v*", "n", COR_LANG)
LANGBUILTIN(__builtin_coro_end, "bv*Ib", "n", COR_LANG)
LANGBUILTIN(__builtin_coro_suspend, "cIb", "n", COR_LANG)
LANGBUILTIN(__builtin_coro_param, "bv*v*", "n", COR_LANG)

// OpenCL v2.0 s6.13.16, s9.17.3.5 - Pipe functions.
// We need the generic prototype, since the packet type could be anything.
Expand Down
2 changes: 0 additions & 2 deletions clang/lib/CodeGen/CGBuiltin.cpp
Expand Up @@ -4702,8 +4702,6 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
return EmitCoroutineIntrinsic(E, Intrinsic::coro_end);
case Builtin::BI__builtin_coro_suspend:
return EmitCoroutineIntrinsic(E, Intrinsic::coro_suspend);
case Builtin::BI__builtin_coro_param:
return EmitCoroutineIntrinsic(E, Intrinsic::coro_param);

// OpenCL v2.0 s6.13.16.2, Built-in pipe read and write functions
case Builtin::BIread_pipe:
Expand Down
7 changes: 0 additions & 7 deletions clang/test/CodeGenCoroutines/coro-builtins.c
Expand Up @@ -5,9 +5,7 @@ void *myAlloc(long long);
// CHECK-LABEL: f(
void f(int n) {
// CHECK: %n.addr = alloca i32
// CHECK: %n_copy = alloca i32
// CHECK: %promise = alloca i32
int n_copy;
int promise;

// CHECK: %[[PROM_ADDR:.+]] = bitcast i32* %promise to i8*
Expand Down Expand Up @@ -45,9 +43,4 @@ void f(int n) {

// CHECK-NEXT: call i8 @llvm.coro.suspend(token none, i1 true)
__builtin_coro_suspend(1);

// CHECK-NEXT: %[[N_ADDR:.+]] = bitcast i32* %n.addr to i8*
// CHECK-NEXT: %[[N_COPY_ADDR:.+]] = bitcast i32* %n_copy to i8*
// CHECK-NEXT: call i1 @llvm.coro.param(i8* %[[N_ADDR]], i8* %[[N_COPY_ADDR]])
__builtin_coro_param(&n, &n_copy);
}
83 changes: 0 additions & 83 deletions llvm/docs/Coroutines.rst
Expand Up @@ -1644,89 +1644,6 @@ a call to ``llvm.coro.suspend.retcon`` after resuming abnormally.
In a yield-once coroutine, it is undefined behavior if the coroutine
executes a call to ``llvm.coro.suspend.retcon`` after resuming in any way.

.. _coro.param:

'llvm.coro.param' Intrinsic
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
::

declare i1 @llvm.coro.param(i8* <original>, i8* <copy>)

Overview:
"""""""""

The '``llvm.coro.param``' is used by a frontend to mark up the code used to
construct and destruct copies of the parameters. If the optimizer discovers that
a particular parameter copy is not used after any suspends, it can remove the
construction and destruction of the copy by replacing corresponding coro.param
with `i1 false` and replacing any use of the `copy` with the `original`.

Arguments:
""""""""""

The first argument points to an `alloca` storing the value of a parameter to a
coroutine.

The second argument points to an `alloca` storing the value of the copy of that
parameter.

Semantics:
""""""""""

The optimizer is free to always replace this intrinsic with `i1 true`.

The optimizer is also allowed to replace it with `i1 false` provided that the
parameter copy is only used prior to control flow reaching any of the suspend
points. The code that would be DCE'd if the `coro.param` is replaced with
`i1 false` is not considered to be a use of the parameter copy.

The frontend can emit this intrinsic if its language rules allow for this
optimization.

Example:
""""""""
Consider the following example. A coroutine takes two parameters `a` and `b`
that has a destructor and a move constructor.

.. code-block:: c++

struct A { ~A(); A(A&&); bool foo(); void bar(); };

task<int> f(A a, A b) {
if (a.foo())
return 42;

a.bar();
co_await read_async(); // introduces suspend point
b.bar();
}

Note that, uses of `b` is used after a suspend point and thus must be copied
into a coroutine frame, whereas `a` does not have to, since it never used
after suspend.

A frontend can create parameter copies for `a` and `b` as follows:

.. code-block:: text
task<int> f(A a', A b') {
a = alloca A;
b = alloca A;
// move parameters to its copies
if (coro.param(a', a)) A::A(a, A&& a');
if (coro.param(b', b)) A::A(b, A&& b');
...
// destroy parameters copies
if (coro.param(a', a)) A::~A(a);
if (coro.param(b', b)) A::~A(b);
}
The optimizer can replace coro.param(a',a) with `i1 false` and replace all uses
of `a` with `a'`, since it is not used after suspend.

The optimizer must replace coro.param(b', b) with `i1 true`, since `b` is used
after suspend and therefore, it has to reside in the coroutine frame.

Coroutine Transformation Passes
===============================
CoroEarly
Expand Down
1 change: 0 additions & 1 deletion llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
Expand Up @@ -625,7 +625,6 @@ class TargetTransformInfoImplBase {
case Intrinsic::coro_frame:
case Intrinsic::coro_size:
case Intrinsic::coro_suspend:
case Intrinsic::coro_param:
case Intrinsic::coro_subfn_addr:
// These intrinsics don't actually represent code after lowering.
return 0;
Expand Down
4 changes: 0 additions & 4 deletions llvm/include/llvm/IR/Intrinsics.td
Expand Up @@ -1282,10 +1282,6 @@ def int_coro_alloca_alloc : Intrinsic<[llvm_token_ty],
def int_coro_alloca_get : Intrinsic<[llvm_ptr_ty], [llvm_token_ty], []>;
def int_coro_alloca_free : Intrinsic<[], [llvm_token_ty], []>;

def int_coro_param : Intrinsic<[llvm_i1_ty], [llvm_ptr_ty, llvm_ptr_ty],
[IntrNoMem, ReadNone<ArgIndex<0>>,
ReadNone<ArgIndex<1>>]>;

// Coroutine Manipulation Intrinsics.

def int_coro_resume : Intrinsic<[], [llvm_ptr_ty], [Throws]>;
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/Transforms/Coroutines/Coroutines.cpp
Expand Up @@ -141,7 +141,6 @@ static bool isCoroutineIntrinsicName(StringRef Name) {
"llvm.coro.id.retcon",
"llvm.coro.id.retcon.once",
"llvm.coro.noop",
"llvm.coro.param",
"llvm.coro.prepare.async",
"llvm.coro.prepare.retcon",
"llvm.coro.promise",
Expand Down

0 comments on commit 352e36e

Please sign in to comment.