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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits