[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added inline comments.


================
Comment at: clang/docs/LanguageExtensions.rst:2689
 
-  size_t __builtin_coro_size()
+  size_t __builtin_coro_size(bool alloc)
   void  *__builtin_coro_frame()
----------------
ychen wrote:
> ChuanqiXu wrote:
> > ychen wrote:
> > > lxfind wrote:
> > > > Do we need to change __builtin_coro_size? The argument will always be 1, right?
> > > > It only starts to change in LLVM intrinsics, if I read the impl correctly.
> > > Yeah, It is always 1 for Clang until the spec is fixed (then we could revert it back to 0).  Other clients using `__builtin_coro_size` may use 0 if the client doesn't care about overaligned frame or it could handle overaligned frame by itself. 
> > BTW, is it OK to edit the `builtin`s directly? Since builtin is different with intrinsic which is only visible in the internal of compiler, builtin could be used by any end users. Although I know there should be  little users who would use `__builtin_coro` APIs, I worry if there is any guide principle for editing the `builtin`s.
> > BTW, is it OK to edit the builtins directly? Since builtin is different with intrinsic which is only visible in the internal of compiler, builtin could be used by any end users. Although I know there should be little users who would use __builtin_coro APIs, I worry if there is any guide principle for editing the builtins.
> 
> I think it is ok to change these if it is justified like anything else.
> 
> builtins/intrinsics are interfaces on different levels. I'm trying to make __builtin_coro_size consistent with llvm.coro.size because I don't have a good reason for not doing that. (assume that we keep this opt-in overaligned frame handling in LLVM even after the spec is fixed since it helps solve a practical problem and the maintenance cost is low)
> 
> 
It doesn't make sense to me that we need to change the signature for `__builtin_coro_size` in this patch. In other words, why do we need to change `__builtin_coro_size `? What are problems that can't be solved if we don't change `__builtin_coro_size`? At least, if it is necessary to change `__builtin_coro_size`, we could make it in successive patches.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100739/new/

https://reviews.llvm.org/D100739