Values, not variables. SILGen can't do that kind of optimization on a local "let" or "var" because it's intentionally statement-by-statement. It also can't do that kind of optimization on a "var" because, for all it knows, the variable might have escaped and thus not really have a statically-knowable last use; that's why SILGen emits them all as boxes and lets the mandatory optimizer promote them to the stack. But there are several reasons why a local r-value might get emitted into memory and then need to get promoted to be a scalar, and of course we won't emit that as a copy.
John.
···
On Oct 7, 2016, at 10:46 PM, Andrew Trick <atrick@apple.com> wrote:
On Oct 7, 2016, at 10:36 PM, Michael Gottesman <mgottesman@apple.com <mailto:mgottesman@apple.com>> wrote:
On Oct 7, 2016, at 10:26 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:
On Oct 7, 2016, at 10:08 PM, Michael Gottesman <mgottesman@apple.com <mailto:mgottesman@apple.com>> wrote:
On Oct 7, 2016, at 9:25 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:
On Oct 7, 2016, at 6:04 PM, Michael Gottesman via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
I wonder whether it might make more sense for load [borrow] to be a different instruction.
There's a couple reasons for that first. The first is that it's the only load which introduces
a scope, which is a really big difference structurally. The second is that it's the only load
which returns a non-owned value, which will be a typing difference when we record
ownership in the type system.I am fine with a load_borrow. If this is the only change left that you want can I just send out a proposal with that small change and start implementing. I am nervous about perfection being the enemy of the good (and I want to start implementing this weekend if possible *evil smile*).
There’s a lot in the proposal that makes sense to discuss for completeness but isn’t motivated by a particular need. Please separate functionality. We only need load [copy] at first right? When do those need to be promoted to load_borrow?
These are needed for the ARC optimizer to eliminate retain, release operations, i.e. a:
%0 = load [copy] %x_ptr
destroy_value %1
=>
%0 = load [borrow] %x_ptr
borrow_end(%0, %x_ptr)
These constructs will be needed by engineers to update passes like ARC. By implementing such modifiers now, we can begin to implement support in the various passes for these new instructions via sil-opt/etc in parallel to other semantic ARC work.
load [trivial] is an optimization, so that should follow a functionally complete implementation.
Yes you are correct that given that we are exploding the load [copy] in the eliminator, the trivial load is not *strictly* needed. But as soon as we start upgrading passes, we are going to want this. Again assuming that parallel work can be done, it makes sense to set the stage for optimizer work that will occur in parallel to further semantic ARC work.
load [take] should definitely not exist until there’s some motivation.
If you look at the frontend, there are places where the frontend wants to emit a take. Unless we are willing to use unqualified loads for those cases (which we can not if we are trying to prove that no unqualified loads are emitted by the frontend), then we must have a load [take].
Did I provide the motivation that you requested?
Yes. My general request is for each commit to be easy to review and the functionality obvious to test. I’m convinced we’ll eventually want the variants. Although I still want to understand better when we need to [take] values out of memory.
Just as a quick example, the API for emitLoad in SILGenFunction:
ManagedValue emitLoad(SILLocation loc, SILValue addr,
const TypeLowering &rvalueTL,
SGFContext C, IsTake_t isTake,
bool isGuaranteedValid = false);Notice the IsTake_t parameter. I see that code path used in several locations in SILGenFunction.
I guess it’s doing this to forward locals variables at their last use, avoiding a copy. Although probably not theoretically necessary, I guess it would be silly not to do this.