[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2022-09-23 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen abandoned this revision. ychen added a comment. Herald added a project: All. 327141f is landed as an alternative. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2863615 , @ychen wrote: > That's just my conclusion based on @rjmccall's suggestion > (https://reviews.llvm.org/D100739#2717582) and my following responses. I guess you got the conclusion from this: > 2d. Use the

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-07 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2863581 , @ChuanqiXu wrote: > In D97915#2862419 , @ychen wrote: > >>> It looks not hard to implement. And we don't need to refactor the CodeGen >>> part a lot. In this way, I think

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-07 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2862419 , @ychen wrote: >> It looks not hard to implement. And we don't need to refactor the CodeGen >> part a lot. In this way, I think the main effort to support `::operator >> new(size_t, align_t)` would be in the

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-07 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2861178 , @ChuanqiXu wrote: > In D97915#2861036 , @ychen wrote: > >> In D97915#2860984 , @ChuanqiXu >> wrote: >> >>> In D97915#2860916

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-07 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2861036 , @ychen wrote: > In D97915#2860984 , @ChuanqiXu wrote: > >> In D97915#2860916 , @ychen wrote: >> >>> In D97915#2859237

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-06 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2860984 , @ChuanqiXu wrote: > In D97915#2860916 , @ychen wrote: > >> In D97915#2859237 , @ChuanqiXu >> wrote: >> >>> In D97915#2848816

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-06 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2860916 , @ychen wrote: > In D97915#2859237 , @ChuanqiXu wrote: > >> In D97915#2848816 , @ychen wrote: >> Thanks for clarifying.

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-06 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2859237 , @ChuanqiXu wrote: > In D97915#2848816 , @ychen wrote: > >>> Thanks for clarifying. Let's solve the semantics problem first. >>> With the introduction about 'raw frame', I

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-06 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 356838. ychen marked 2 inline comments as done. ychen added a comment. - Move `raw frame` description to `Switched-Resume` section. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-06 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2848816 , @ychen wrote: >> Thanks for clarifying. Let's solve the semantics problem first. >> With the introduction about 'raw frame', I think it's necessary to introduce >> this concept in the section

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-01 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. I am a little busy this week. I would try to look into this next week if possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-29 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 355401. ychen added a comment. - Add description of `raw frame` in `Coroutines.rst`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files:

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-29 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. > Thanks for clarifying. Let's solve the semantics problem first. > With the introduction about 'raw frame', I think it's necessary to introduce > this concept in the section 'Switched-Resume Lowering' or even the section > 'Introduction' in the document. Add a section to

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-24 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2835147 , @ychen wrote: > In D97915#2832667 , @ChuanqiXu wrote: > >> In D97915#2832446 , @ychen wrote: >> >>> In D97915#2829581

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-23 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 353855. ychen added a comment. - Update intrinsics documentation. - Inline `EmitBuiltinAlignTo` as `emitAlignUpTo`. - Address other comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-23 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2832667 , @ChuanqiXu wrote: > In D97915#2832446 , @ychen wrote: > >> In D97915#2829581 , @ChuanqiXu >> wrote: >> >>> A remained question.

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-22 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2832446 , @ychen wrote: > In D97915#2829581 , @ChuanqiXu wrote: > >> A remained question. >> >> - what's the semantics if user specified their allocation/deallocation >>

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-22 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 353567. ychen marked an inline comment as done. ychen added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files:

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-22 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen marked 5 inline comments as done. ychen added a comment. In D97915#2829581 , @ChuanqiXu wrote: > A remained question. > > - what's the semantics if user specified their allocation/deallocation > functions? > > Previously, we discussed for the

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-20 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. A remained question. - what's the semantics if user specified their allocation/deallocation functions? Previously, we discussed for the ::operator new and ::operator delete. But what would we do for user specified allocation/deallocation functions? It looks like we

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-17 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added inline comments. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:437-475 +void emitDynamicAlignedDealloc(CodeGenFunction , + llvm::BasicBlock *AlignedFreeBB, + llvm::CallInst *CoroFree) { + llvm::CallInst

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-17 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 352908. ychen marked 2 inline comments as done. ychen added a comment. - Not use `void *` in EmitBuiltinAlignTo signature. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-17 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 352810. ychen added a comment. - Rebase correctly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files: clang/lib/CodeGen/CGBuiltin.cpp

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-16 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. It misses the part in llvm now. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:437-475 +void emitDynamicAlignedDealloc(CodeGenFunction , + llvm::BasicBlock *AlignedFreeBB, + llvm::CallInst

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-16 Thread Yuanfang Chen via Phabricator via cfe-commits
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, +

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-16 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 352617. ychen marked 3 inline comments as done. ychen added a comment. - Remove AlignedAllocator & AlignedDeallocator. - Use 'OverAllocateFrame' - Get rid of `if (HasAlignArg)` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-16 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. It looks not easy to review completely. After a rough review, the first suggest is to remove the contents that should be in 'D102147 ', it makes the complex problem more complex I think. I would try to do more detailed review and

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2819871 , @ychen wrote: > - Merge D102145 by @ChuanqiXu's request. Thanks a lot. Could you add me as a reviewer? So that I can see this patch in my main page. Repository: rG

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-15 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 352189. ychen added a comment. - Merge D102145 by @ChuanqiXu's request. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files:

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/include/clang/AST/StmtCXX.h:356-359 Expr *Allocate = nullptr; Expr *Deallocate = nullptr; +Expr *AlignedAllocate = nullptr; +Expr *AlignedDeallocate = nullptr; ychen wrote: > ChuanqiXu wrote: >

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-13 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added inline comments. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:528-547 CGF.EmitBlock(FreeBB); CGF.EmitStmt(Deallocate); - -auto *AfterFreeBB = CGF.createBasicBlock("after.coro.free"); -CGF.EmitBlock(AfterFreeBB); +

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-13 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added inline comments. Comment at: clang/include/clang/AST/StmtCXX.h:356-359 Expr *Allocate = nullptr; Expr *Deallocate = nullptr; +Expr *AlignedAllocate = nullptr; +Expr *AlignedDeallocate = nullptr; ChuanqiXu wrote: > ychen wrote: > >

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-13 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added inline comments. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:528-547 CGF.EmitBlock(FreeBB); CGF.EmitStmt(Deallocate); - -auto *AfterFreeBB = CGF.createBasicBlock("after.coro.free"); -CGF.EmitBlock(AfterFreeBB); +

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/include/clang/AST/StmtCXX.h:356-359 Expr *Allocate = nullptr; Expr *Deallocate = nullptr; +Expr *AlignedAllocate = nullptr; +Expr *AlignedDeallocate = nullptr; ychen wrote: > ChuanqiXu wrote: >

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-12 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 345053. ychen marked an inline comment as done. ychen added a comment. - Address feedbacks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files:

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-12 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen marked an inline comment as done. ychen added inline comments. Comment at: clang/include/clang/AST/StmtCXX.h:356-359 Expr *Allocate = nullptr; Expr *Deallocate = nullptr; +Expr *AlignedAllocate = nullptr; +Expr *AlignedDeallocate = nullptr;

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-12 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/include/clang/AST/StmtCXX.h:356-359 Expr *Allocate = nullptr; Expr *Deallocate = nullptr; +Expr *AlignedAllocate = nullptr; +Expr *AlignedDeallocate = nullptr; Can't we merge these?

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-11 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2747142 , @ChuanqiXu wrote: >> which should be correct. It is implemented by >> CodeGenFunction::EmitBuiltinAlignTo. > > I agree it is correct. I just want to say we should comment it to avoid > confusing. Happy to do

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-11 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 344602. ychen added a comment. - Rebase on updated D102145 (use `llvm.coro.raw.frame.ptr.addr` during allocation) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > which should be correct. It is implemented by > CodeGenFunction::EmitBuiltinAlignTo. I agree it is correct. I just want to say we should comment it to avoid confusing. Since the patch could handle the case if the frontend tries to search `::operator new(size_t,

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-09 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 343959. ychen added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files: clang/include/clang/AST/StmtCXX.h clang/lib/AST/StmtCXX.cpp

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-09 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 343956. ychen added a comment. - rebase on D102145 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files:

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-05 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen planned changes to this revision. ychen added a comment. Plan to rebase this together with the following patch for two lookups (aligned and non-aligned new/delete, and generate code accordingly) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-03 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2728377 , @ChuanqiXu wrote: > This code snippets confused me before: > > coro.alloc.align: ; preds = > %coro.check.align > %mask = sub i64 %11, 1 > %intptr = ptrtoint i8* %call to

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-30 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. This code snippets confused me before: coro.alloc.align: ; preds = %coro.check.align %mask = sub i64 %11, 1 %intptr = ptrtoint i8* %call to i64 %over_boundary = add i64 %intptr, %mask %inverted_mask = xor i64 %mask, -1

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-30 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2727787 , @ychen wrote: > In D97915#2727759 , @ChuanqiXu wrote: > >> May I ask a question may be too simple? What if the user specify the >> alignment for promise (or any other

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-29 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 341756. ychen added a comment. - Add missed `Shape.CoroRawFramePtrOffsets.clear();` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files:

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-29 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2727759 , @ChuanqiXu wrote: > May I ask a question may be too simple? What if the user specify the > alignment for promise (or any other local variables) to 128 or even 256? > Since it looks like all the discuss before

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-29 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. May I ask a question may be too simple? What if the user specify the alignment for promise (or any other local variables) to 128 or even 256? Since it looks like all the discuss before assumes that the largest alignment requirement is 64. Repository: rG LLVM

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-29 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. For coroutine `f0` in `test/CodeGenCoroutines/coro-alloc.cpp` The allocation looks like this: ; Function Attrs: noinline nounwind optnone mustprogress define dso_local void @f0() #0 { entry: %0 = alloca %struct.global_new_delete_tag, align 1 %1 = alloca

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-29 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 341592. ychen added a comment. - fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files: clang/lib/CodeGen/CGBuiltin.cpp clang/lib/CodeGen/CGCoroutine.cpp

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-29 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 341591. ychen added a comment. - fix a bug. ready for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files: clang/lib/CodeGen/CGBuiltin.cpp

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-29 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen planned changes to this revision. ychen added a comment. Found a bug. Will fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 ___ cfe-commits mailing

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-29 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. @rjmccall the patch is on the large side. I'll submit a separate patch for the Sema part about searching for two allocators. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-29 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 341418. ychen added a comment. - Handle deallocation. - Fix tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files: clang/lib/CodeGen/CGBuiltin.cpp

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-04-23 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen abandoned this revision. ychen added a comment. Pursue D100739 instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-17 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2632493 , @lxfind wrote: > I am not sure how this would work, maybe I am missing something. > But this patch tries to round up the frame pointer by looking at the > difference between the alignment of new and the

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-17 Thread Xun Li via Phabricator via cfe-commits
lxfind added a comment. I am not sure how this would work, maybe I am missing something. But this patch tries to round up the frame pointer by looking at the difference between the alignment of new and the alignment of the frame. The alignment of new only gives you the guaranteed alignment for

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-07 Thread Lewis Baker via Phabricator via cfe-commits
lewissbaker added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450 +Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero); +return RValue::get(Builder.CreateAdd(SizeCall, Extra)); } rjmccall wrote: > lewissbaker wrote: > > ychen wrote:

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450 +Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero); +return RValue::get(Builder.CreateAdd(SizeCall, Extra)); } lewissbaker wrote: > ychen wrote: > > rjmccall wrote: >

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-05 Thread Lewis Baker via Phabricator via cfe-commits
lewissbaker added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450 +Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero); +return RValue::get(Builder.CreateAdd(SizeCall, Extra)); } ychen wrote: > rjmccall wrote: > > ychen wrote: > > >

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D97915#2607567 , @ychen wrote: > In D97915#2607338 , @rjmccall wrote: > >> Let's try to avoid adding a new builtin for what we acknowledge is a >> workaround. Builtins become part of

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-05 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2607338 , @rjmccall wrote: > Let's try to avoid adding a new builtin for what we acknowledge is a > workaround. Builtins become part of the language supported by the compiler, > so we shouldn't add them casually. If

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Let's try to avoid adding a new builtin for what we acknowledge is a workaround. Builtins become part of the language supported by the compiler, so we shouldn't add them casually. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-05 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2605398 , @ChuanqiXu wrote: > I am a little confusing about the problem. The example in the link tells the > align of the `promise` instead of the `frame`. The address of `promise` and > `frame` is not same. It looks

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. I am a little confusing about the problem. The example in the link tells the align of the `promise` instead of the `frame`. The address of `promise` and `frame` is not same. It looks like you're trying to do: + +---+

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450 +Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero); +return RValue::get(Builder.CreateAdd(SizeCall, Extra)); } rjmccall wrote: > ychen wrote: > > ychen wrote: > > >

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450 +Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero); +return RValue::get(Builder.CreateAdd(SizeCall, Extra)); } ychen wrote: > ychen wrote: > > rjmccall wrote: > > >

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450 +Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero); +return RValue::get(Builder.CreateAdd(SizeCall, Extra)); } ychen wrote: > rjmccall wrote: > > Okay, so you're

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450 +Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero); +return RValue::get(Builder.CreateAdd(SizeCall, Extra)); } rjmccall wrote: > Okay, so you're implicitly increasing the

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4450 +Value *Extra = Builder.CreateSelect(Cmp, Diff, Zero); +return RValue::get(Builder.CreateAdd(SizeCall, Extra)); } Okay, so you're implicitly increasing the coroutine size

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D97915#2603912 , @lxfind wrote: > Could you describe in more detail what problem this patch solves? Yes, updated description. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 328232. ychen edited the summary of this revision. ychen added a comment. - Add docs for `coro.align` - clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-04 Thread Xun Li via Phabricator via cfe-commits
lxfind added a comment. Could you describe in more detail what problem this patch solves? Also, since you are adding a new intrinsics, please also update the coroutine documentation regarding this new intrinsics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-03 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. I didn't add any new tests. Instead, added checks for the new code sequence: 1. check that `llvm.assume` is there to make sure the adjusted address is correctly aligned. 2. check that `llvm.coro.align` and the associated instructions are there to over-allocate heap

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-03 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 328036. ychen added a comment. Herald added a subscriber: jdoerfert. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 Files: clang/lib/CodeGen/CGBuiltin.cpp

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-03-03 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen created this revision. ychen added reviewers: rjmccall, lxfind, GorNishanov. Herald added a subscriber: hiraditya. ychen requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. by over-allocating and emitting `alignTo` code