Use typed boxes to simplify capture representation in SILGen

[jckarter]

Now that SIL has @box T types, we no longer need to capture mutable variables as pairs of box and address. Here's a series of patches that simplifies the way SILGen emits captures so that they're only passed as boxes, along with follow-up changes to capture promotion, SILCombine, and DI to preserve their existing behavior. Until now, we also claimed that capture addresses were passed @inout, which is semantically incorrect—mutable captures are allowed to be mutated simultaneously from the closure and the original context in well-typed, well-synchronized ways. I introduced a new parameter convention @inout_aliasable to represent these semantics for @noescape mutable captures. I'd like some review of the various pieces before mashing into master:

@rjmccall, do the semantics of @inout_aliasable make sense to you? I'm not thrilled with the name, since we were already throwing aliasable around with the different meaning of "allow type-punned aliases through this pointer", which I don't want to allow.

@lattner, I changed SILGen to emit mark_function_escape instructions on variable addresses before they're captured; that way, DI still works on captured variables without having to do concurrent analysis of the box and address value uses. I also adjusted the diagnostics to use the more specific "captured by a closure" diagnostic when this happens, instead of the generic "used before initialized" diagnostic. Is there a better approach you'd prefer?

@rudkx, do the changes to capture promotion look reasonable? Are there any other passes you're aware of that would need updating to avoid optimizer regressions?

@adrian-prantl, there's a regression in debug info that I wasn't sure how to address, since the argument addresses are no longer passed directly. I could have SILGen emit debug_addr instructions to annotate the projections that get emitted on the callee side. Is that the right thing to do, or do we need extra information to flag these as arguments or captures instead of regular variables?

[rjmccall]

The semantics seems good to me. I think "aliased", "aliasing", or "aliasable" is right term for this, and we should chose a stronger and more specific name for things that are allowed to violate TBAA.

[rudkx]

@jckarter I don't know of other optimizations that need to be updated for this, but it would be good to do a benchmark run as a sanity check before it gets merged to master.

[gribozavr]

The combined patch XFAILs some tests. Is that intended?

[jckarter]

@gribozavr Yes, as I noted in the patch description, it regresses debug information for boxed captures. I wasn't sure what the proper way to fix this was, so I'm awaiting @adrian-prantl's feedback.

[lattner]

On Nov 30, 2015, at 10:32 AM, Joe Groff [email protected] wrote:

Now that SIL has @box T types, we no longer need to capture mutable variables as pairs of box and address. Here's a series of patches that simplifies the way SILGen emits captures so that they're only passed as boxes, along with follow-up changes to capture promotion, SILCombine, and DI to preserve their existing behavior. Until now, we also claimed that capture addresses were passed @inout, which is semantically incorrect—mutable captures are allowed to be mutated simultaneously from the closure and the original context in well-typed, well-synchronized ways. I introduced a new parameter convention @inout_aliasable to represent these semantics for @NoEscape mutable captures. I'd like some review of the various pieces before mashing into master:

@lattner https://github.com/lattner, I changed SILGen to emit mark_function_escape instructions on variable addresses before they're captured; that way, DI still works on captured variables without having to do concurrent analysis of the box and address value uses. I also adjusted the diagnostics to use the more specific "captured by a closure" diagnostic when this happens, instead of the generic "used before initialized" diagnostic. Is there a better approach you'd prefer?

The DI and related SILGen changes look great to me. Thanks for doing this, it is an really nice cleanup too. One bad thing is that it invalidates some slides from our SIL talk :-)

-Chris

[jckarter]

I guess we have to give the talk again at Barcelona next year then. Thanks for the review!

[jckarter]

Thanks for the feedback everyone! With @adrian-prantl's recent fixes the proper debug info appears to fall out. Going public seems to have messed with the connection in the pull request; I'll finish this up out of band.