[PATCH] D57797: Variable auto-init: fix __block initialization

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/CodeGen/CGDecl.cpp:1643
     CharUnits Size = getContext().getTypeSizeInChars(type);
     if (!Size.isZero()) {
       switch (trivialAutoVarInit) {
----------------
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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797