[PATCH] D24373: [coroutines] Adding builtins for coroutine intrinsics and backendutil support.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 3 11:40:42 PDT 2016


rsmith added inline comments.


> LanguageExtensions.rst:1886
> +  bool __builtin_coro_done(void *addr);
> +  void __builtin_coro_promise(void *addr, int alignment, bool from_promise)
> +

Return type here is `void*`, not `void`.

> LanguageExtensions.rst:1916
> +
> +Other coroutine builtins are either for the internal clang use or for the use
> +during development of the coroutine feature. See `Coroutines in LLVM

Remove both "the"s here.

> O2-coro.c:1
> +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -emit-llvm %s -o - -O3 | FileCheck %s
> +#include "Inputs/coro.h"

Please do not test the output of LLVM optimization in a clang unit test. It seems fine to just remove this test, you have the relevant coverage elsewhere.

> coro-builtins.c:5
> +void f() {
> +  // CHECK: %0 = call token @llvm.coro.id(i32 32, i8* null, i8* null, i8* null)
> +  __builtin_coro_id(32, 0, 0, 0);

Use a pattern here rather than hardcoding `%0`; it's not reasonable to assume that this will always be the first instruction in the function.

Please also pass actual arguments here so we can test they're passed to the correct parameter.

https://reviews.llvm.org/D24373





More information about the cfe-commits mailing list