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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
