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 coroutine size to allow you to > round up to get an aligned frame. But how do you round back down to get the > actual pointer that you need to delete? This just doesn't work. > > You really ought to just be using the aligned `operator new` instead when the > required alignment is too high. If that means checking the alignment > "dynamically" before calling `operator new` / `operator delete`, so be it. > In practice, it will not be dynamic because lowering will replace the > `coro.align` call with a constant, at which point the branch will be foldable. > > I don't know what to suggest if the aligned `operator new` isn't reliably > available on the target OS. You could outline a function to pick the best > allocator/deallocator, I suppose. Thanks for the review! > Okay, so you're implicitly increasing the coroutine size to allow you to > round up to get an aligned frame. But how do you round back down to get the > actual pointer that you need to delete? This just doesn't work. Hmm, you're right that I missed the `delete` part, thanks. The adjusted amount is constant, I could just dial it down in `coro.free`, right? > You really ought to just be using the aligned operator new instead when the > required alignment is too high. If that means checking the alignment > "dynamically" before calling operator new / operator delete, so be it. In > practice, it will not be dynamic because lowering will replace the coro.align > call with a constant, at which point the branch will be foldable. That's my intuition at first. But spec is written in a way suggesting (IMHO) that the aligned version should not be used? What if the user specify their own allocator, then which one they should use? 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
