[LVA] Record store as read of source for reference types.

[zoecarver]

After processing a store, if the store is alive, stops tracking the source if it's a reference type.

In the following case:

    xx = alloc_ref
    e = ref_element_addr xx
    store undef to e
    yy = alloc_ref
    h = ref_element_addr yy
    store xx to h
    use h

The first store will be eliminated because there are no reads of any of the projected aliases (xx). However, that's not true, the (second) store counts as a read. If this were a struct, the current assumption would be correct because we couldn't GEP a non-address struct (so there is no way that the source of a store could have a GEP on it). But, reference types do allow GEPs so, this is not a safe assumption.

[eeckstein]

I don't understand the intention here. Is this a bug fix?
Are you trying to handle the case where a class reference escapes through a store?
If yes, I would be surprised if this is not handled by escape/alias analysis already.

Maybe you clarify the description/comment.

[eeckstein]

This test will not compile on 32-bit architectures, where Int is 32 bit. To fix this, use Int64 instead of plain Int.

[zoecarver]

@eeckstein yes, precisely. That is the issue this solves. The reproducer/test that I've added fails because of the issue brought up in #31136 but, I suspect given the correct input we could get this to fail on master as well.

It is handled by analysis but (AFAICT) analysis never runs on stores/store operands.

[zoecarver]

@eeckstein here's another example of the same issue (not with a reference type so, this patch doesn't fix it):

class F { }

struct S {
   var f : F
}
struct U {
  var s : S
}

sil hidden [noinline] @test : $@convention(thin) (S, @inout F) -> () {
bb0(%0 : $S, %f : $*F):
  %2 = alloc_stack $S, var, name "vs"
  store %0 to %2 : $*S
  %4 = address_to_pointer %2 : $*S to $Builtin.RawPointer
  %9 = alloc_stack $U
  %10 = unchecked_addr_cast %9 : $*U to $*Builtin.RawPointer
  store %4 to %10 : $*Builtin.RawPointer
  %12 = load %9 : $*U
  %15 = struct_extract %12 : $U, #U.s
  %16 = struct_extract %15 : $S, #S.f
  dealloc_stack %9 : $*U
  store %16 to %f : $*F
  dealloc_stack %2 : $*S
  %19 = tuple ()
  return %19 : $()
}

The second store isn't counted as a read of %2 (or %4) so the first store is removed incorrectly. This fails on master.

[zoecarver]

This should be unconditional.

[zoecarver]

We lose this optimization when the source is a reference type. I think that's a rare case but, I would like to add this back as a special-case optimization in another patch.

[eeckstein]

This is the wrong approach. A store is never reading memory. So we should not model it as such. The problem here is about that an address is escaping.

I'm not sure if there is really a bug currently. The SIL in your example is invalid, because it lets the address of a stack location escape after the dealloc_stack

[zoecarver]

@eeckstein you're right. This is not the correct fix; it's not a read. I'll close this PR but, I think the issue is actually valid.

What's escaping (in the most recent example) is a pointer to type F, that's not ever (necessarily) deallocated. %0 holds a pointer to F which escapes out of the function in the store to %f. AFAIK the pointer in %0 could be live before and after the function. When it's no longer stored out of the function, the program may have a different behavior.

This is a very contrived example but, it shows a technically incorrect behavior. If something like #31136 lands, the problem will become much more common.

sil @store_is_read_of_src : $@convention(thin) (S, @inout F) -> () {
 bb0(%0 : $S, %f : $*F):
   %2 = alloc_stack $S, var, name "vs"
   store %0 to %2 : $*S
   %4 = address_to_pointer %2 : $*S to $Builtin.RawPointer
   %9 = alloc_stack $U
   %10 = unchecked_addr_cast %9 : $*U to $*Builtin.RawPointer
   store %4 to %10 : $*Builtin.RawPointer
   %12 = load %9 : $*U
   %15 = struct_extract %12 : $U, #U.s
   %16 = struct_extract %15 : $S, #S.f
   dealloc_stack %9 : $*U
   store %16 to %f : $*F
   dealloc_stack %2 : $*S
   %19 = tuple ()
   return %19 : $()
 }