rjmccall added a comment.

In D100739#2717259 <https://reviews.llvm.org/D100739#2717259>, @ychen wrote:

> In D100739#2717227 <https://reviews.llvm.org/D100739#2717227>, @rjmccall 
> wrote:
>
>> Yes, if you can dynamically choose to use an aligned allocator, that's 
>> clearly just much better.
>
> Right now:
>
> Intrinsic::coro_size_aligned :   overaligned frame: over-allocate, adjust 
> start address;    non-overaligned frame: no-op
> Intrinsic::coro_size :                overaligned frame: no-op;               
>                                   non-overaligned frame: no-op
>
> Do you mean to remove `Intrinsic::coro_size_aligned` and make 
> Intrinsic::coro_size :                overaligned frame: over-allocate, 
> adjust start address;    non-overaligned frame: no-op
>
> that makes sense to me. Just want to confirm first.

That's not really what I meant, no.  I meant it would be better to find a way 
to use an allocator that promises to return a well-aligned value when possible. 
 We've talked about this before; that will require the C++ committee to update 
the design.

I think the cleanest design for coro_size/align would be that they reflect the 
unadjusted requirements, and the frontend is expected to emit code which 
satisfies those requirements.  In the absence of an aligned allocator, that 
means generating code like:

  if (llvm.coro.alloc()) {
    size_t size = llvm.coro.size(), align = llvm.coro.align();
    if (align > NEW_ALIGN)
      size += align - NEW_ALIGN + sizeof(void*);
    frame = operator new(size);
    if (align > NEW_ALIGN) {
      auto rawFrame = frame;
      frame = (frame + align - 1) & ~(align - 1);
      *(void**) (frame + size) = rawFrame;
    }
  }

and then on the deallocation side:

  if (llvm.coro.alloc()) {
    size_t size = llvm.coro.size(), align = llvm.coro.align();
    if (align > NEW_ALIGN)
      frame = *(void**) (frame + size);
    operator delete(frame);
  }

That's all quite annoying, but it does extend quite nicely to cover the 
presence of an aligned allocator when the committee gets around to ratifying 
that this is what should happen:

  if (llvm.coro.alloc()) {
    size_t size = llvm.coro.size(), align = llvm.coro.align();
    if (align > NEW_ALIGN)
      frame = operator new(std::align_val_t(align), size);
    else
      frame = operator new(size);
  }

and then on the deallocation side:

  if (llvm.coro.alloc()) {
    size_t size = llvm.coro.size(), align = llvm.coro.align();
    if (align > NEW_ALIGN)
      operator delete(frame, std::align_val_t(align));
    else
      operator delete(frame);
  }


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

Reply via email to