[AddressLowering] Allow use-projection into phi.

[nate-chandler]

Resolves numerous regressions when enabling AddressLowering early in the pipeline.

Previously, if a value incoming into a phi had storage which itself was a use-projection out of some other storage, PhiStorageOptimizer bailed out. The result was unnecessary "moves" (i.e. copy_addr [take] [init] instructions).

Here, this bailout is removed. In order to do this, it is necessary to find (1) all values whose storage recursively project out of an incoming value (such a value may have storage which is either a use or a def projection) and (2) the block which dominates the defs of all these values.

Together, these values are used to compute liveness to determine interference. Previously, the live region was that between the uses of an incoming value and its defining block. Now, it is that between the uses of any of the values found in (1) and the dominating block found in (2).

[nate-chandler]

@swift-ci please test

[atrick]

Nice!

[atrick]

Mysterious assert alert. It looks like it's saying that incomingVal can never be a phi. Where do we guarantee that?

[nate-chandler]

Yeah, this is a result of

  if (PhiValue(incomingVal)) {
    // Do not coalesce a phi with other phis. This would require liveness
    // analysis of the whole phi web before coalescing phi operands.
    return nullptr;
  }

We should be able to remove this bail-out (and add all values incoming to PhiValue(value) to the values ValueWorklist here) in a follow-up.

[atrick]

Can you assert that projecteds is non-empty here? If, so, why do you need to update the LCA given that the projecteds must also update the LCA?

[nate-chandler]

Yeah, we can assert that--projecteds is empty if and only findDefProjections returns false. Asserted here and for findUseProjections.

I don't think we can skip updating the LCA here: it needs to dominate defInst->getParent(). The structure of chain projections is such that in order for a def projection to appear, it has to look something like this (projection_chain_structure):

parent:
  %val = ...
  cond_br ..., left, right

left:
  %d = disaggregate %val
  ...

right:
  %a = aggregate %val

...

  br merge( aggregate(...(aggregate(%a)...) )

In this case, so far so good: we'd update the LCA with %d = disaggregate %val which would make parent the LCA. If the branch that's the LCA of disaggregate and aggregate is different from the block containing the def %val, however, the LCA is after the def block:

grandparent:
  %val = ...
  cond_br ..., parent, other

parent:
  cond_br ..., left, right

left:
  %d = disaggregate %val
  ...

right:
  %a = aggregate %val

...

  br merge( aggregate(...(aggregate(%a)...) )

In the worst case, because the LCA is wrong, the walk where liveness is determined continues past the LCA (since we just compare the current block to the LCA before pushing its predecessors) resulting in redundant alloc_stacks. Without including this defInst->getParent() in the LCA computation, PhiStorageOptimizer incorrectly determines that there's interference in entry because the walk back from destroy_value %v continues all the way to entry (because LCA is incorrectly determined to be parent rather than trouble2).

sil [ossa] @g : $@convention(thin) <T> () -> () {
entry:
  %agg4 = apply undef<Box<Box<T>>>() : $@convention(thin) <T> () -> (@out T)
  cond_br undef, okay, trouble

okay:
  br merge(%agg4 : $Box<Box<T>>)


trouble:
  destroy_value %agg4 : $Box<Box<T>>
  br trouble2

trouble2:
  %v = apply undef<Box<T>>() : $@convention(thin) <T> () -> (@out T)
  cond_br undef, grandparent, other

grandparent:
  br parent

parent:
  cond_br undef, left, right

left:
  %t = destructure_struct %v : $Box<T>
  destroy_value %t : $T
  br left2

left2:
  %agg2 = apply undef<Box<Box<T>>>() : $@convention(thin) <T> () -> (@out T)
  br merge(%agg2 : $Box<Box<T>>)

right:
  %agg = struct $Box<Box<T>>(%v : $Box<T>)
  br merge(%agg : $Box<Box<T>>)

other:
  destroy_value %v : $Box<T>
  br other2

other2:
  %agg3 = apply undef<Box<Box<T>>>() : $@convention(thin) <T> () -> (@out T)
  br merge(%agg3 : $Box<Box<T>>)

merge(%p : @owned $Box<Box<T>>):
  destroy_value %p : $Box<Box<T>>
  br exit

exit:
  %retval = tuple ()
  return %retval : $()
}

This is now f190_vexing_incoming_def_projection in address_lowering_phi.sil.

[nate-chandler]

@swift-ci please test