ychen added inline comments.
================ Comment at: clang/lib/CodeGen/CGCoroutine.cpp:743-744 + CGM.getIntrinsic(llvm::Intrinsic::coro_align, SizeTy)); + auto *AlignedUpAddr = EmitBuiltinAlignTo(AlignedAllocateCall, CoroAlign, + S.getAlignedAllocate(), true); + // rawFrame = frame; ---------------- ChuanqiXu wrote: > Maybe we could calculate it in place instead of trying to call a function > which is not designed for llvm::value*. It looks like the calculation isn't > too complex. I'm open to not calling `EmitBuiltinAlignTo`, which basically inline the useful parts of `EmitBuiltinAlignTo`. The initial intention is code sharing and easy readability. What's the benefit of not calling it? ================ Comment at: clang/lib/CodeGen/CGCoroutine.cpp:753 + } + EmitBlock(InitBB); ---------------- ChuanqiXu wrote: > Does here miss a branch to InitBB? `EmitBlock` would handle the case. ================ Comment at: clang/lib/CodeGen/CodeGenFunction.h:1920-1921 + llvm::Value *EmitBuiltinAlignTo(void *Args, const Expr *E, bool AlignUp); + public: ---------------- ChuanqiXu wrote: > We shouldn't add this interface. The actual type for the first argument is > BuiltinAlignArgs*, which defined in .cpp files. The signature is confusing. This is a private function, supposedly only meaningful for the implementation. In that situation do you think it's bad? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
