jfb marked 3 inline comments as done.
jfb added inline comments.

================
Comment at: lib/CodeGen/CGDecl.cpp:1643
     CharUnits Size = getContext().getTypeSizeInChars(type);
     if (!Size.isZero()) {
       switch (trivialAutoVarInit) {
----------------
rjmccall wrote:
> jfb wrote:
> > rjmccall wrote:
> > > Can't you just drill to the byref storage right here and avoid all the 
> > > other complexity and subtle ordering interactions?
> > We're in the lambda that does the initialization here. The tricky order 
> > part is that code that calls the lambda does:
> > 
> > - Block (which was missing the early auto-init)
> > - Trivial initializer (which has auto-init, then early exit)
> > - Constant aggregate / constexpr (which might auto-init if it wasn't 
> > constant, and then early-exit)
> > - The other stuff
> > 
> > I don't think here is the right place to do anything... and I'm not sure 
> > what you're suggesting.
> Escaping `__block` variables are basically a fixed-layout header followed by 
> storage of the variable's formal type.  Anything you do at this point in the 
> function to auto-initialize the header is a waste of time and code size 
> because it is precisely at this point in the function that we perform a bunch 
> of stores to initialize that header.   You are mitigating nothing by covering 
> the header.  So what I'm saying is that, in this lambda which is meant to 
> initialize the user-exposed storage of a variable, you should just make sure 
> you're pointing at the user-exposed storage of the variable by calling 
> `emitBlockByrefAddress(false)` (which just does a GEP), and then you can 
> initialize only that.
Thanks, I now understand what you meant. It's super simple indeed, sorry I took 
a while to grok. Updated patch should do this now.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to