[PATCH] D94137: [coroutine] update promise object's final layout index

[Xun Li via Phabricator via llvm-commits]

lxfind added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:750
       FrameData.setFieldIndex(
           PromiseAlloca, B.addFieldForAlloca(PromiseAlloca, /*header*/ true));
 
----------------
ychen wrote:
> lxfind wrote:
> > PromiseAlloca is added here as a header field, which means it should have a fixed index?
> > Could it be there are bugs in performOptimizedStructLayout that moved it around somehow?
> I'm inclined to say it should be fixed offset which is not changed after `performOptimizedStructLayout`.
> 
> 1) The use of `IsHeader` in addFieldForAllocas suggests that.
> 2) The comment below in OptimizedStructLayout.h
> 
> ```
> /// - Fields may be assigned a fixed offset in the layout.  If there are
> ///   gaps among the fixed-offset fields, the algorithm may attempt
> ///   to allocate flexible-offset fields into those gaps.  If that's
> ///   undesirable, the caller should "block out" those gaps by e.g.
> ///   just creating a single fixed-offset field that represents the
> ///   entire "header".
> ```
> 
You are right.
In that case, I wonder if it is still necessary to add PromiseAlloca as a header field?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94137/new/

https://reviews.llvm.org/D94137