[6.1] AliasAnalysis: consider memory effects of a consume/destroy of a class on it's let-fields

SwiftCompilerSources/Sources/Optimizer/Analysis/AliasAnalysis.swift

@@ -378,15 +378,6 @@ struct AliasAnalysis {
       // The address has unknown escapes. So we have to take the global effects of the called function(s).
       memoryEffects = calleeAnalysis.getSideEffects(ofApply: apply).memory
     }
-    // Do some magic for `let` variables. Function calls cannot modify let variables.
-    // The only exception is that the let variable is directly passed to an indirect out of the apply.
-    // TODO: make this a more formal and verified approach.
-    if memoryEffects.write {
-      let accessBase = memLoc.address.accessBase
-      if accessBase.isLet && !accessBase.isIndirectResult(of: apply) {
-        return SideEffects.Memory(read: memoryEffects.read, write: false)
-      }
-    }
     return memoryEffects
   }
 
@@ -440,11 +431,7 @@ struct AliasAnalysis {
                                          initialWalkingDirection: memLoc.walkingDirection,
                                          complexityBudget: getComplexityBudget(for: inst.parentFunction), context)
     {
-      var effects = inst.memoryEffects
-      if memLoc.isLetValue {
-        effects.write = false
-      }
-      return effects
+      return inst.memoryEffects
     }
     return .noEffects
   }

SwiftCompilerSources/Sources/Optimizer/TestPasses/MemBehaviorDumper.swift

@@ -65,6 +65,7 @@ private extension Instruction {
          is EndCOWMutationInst,
          is CopyValueInst,
          is DestroyValueInst,
+         is StrongReleaseInst,
          is IsUniqueInst,
          is EndBorrowInst,
          is LoadInst,

test/SILOptimizer/load-copy-to-borrow.sil

@@ -201,7 +201,7 @@ sil [ossa] @dont_copy_let_properties_with_guaranteed_base : $@convention(thin) (
 bb0(%x : @guaranteed $ClassLet):
   %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
 
-  %p = ref_element_addr %x : $ClassLet, #ClassLet.aLet
+  %p = ref_element_addr [immutable] %x : $ClassLet, #ClassLet.aLet
   %v = load [copy] %p : $*Klass
   %b = begin_borrow %v : $Klass
   apply %f(%b) : $@convention(thin) (@guaranteed Klass) -> ()
@@ -225,7 +225,7 @@ sil [ossa] @dont_copy_let_properties_with_guaranteed_base_and_forwarding_uses :
 bb0(%x : @guaranteed $ClassLet):
   %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
 
-  %p = ref_element_addr %x : $ClassLet, #ClassLet.aLet
+  %p = ref_element_addr [immutable] %x : $ClassLet, #ClassLet.aLet
   %v = load [copy] %p : $*Klass
   %c = unchecked_ref_cast %v : $Klass to $Klass
   %b = begin_borrow %c : $Klass
@@ -249,30 +249,7 @@ bb0(%x : @guaranteed $SubclassLet):
   %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
 
   %u = upcast %x : $SubclassLet to $ClassLet
-  %p = ref_element_addr %u : $ClassLet, #ClassLet.aLet
-  %v = load [copy] %p : $*Klass
-  %b = begin_borrow %v : $Klass
-  apply %f(%b) : $@convention(thin) (@guaranteed Klass) -> ()
-  end_borrow %b : $Klass
-  destroy_value %v : $Klass
-
-  return undef : $()
-}
-
-// CHECK-LABEL: sil [ossa] @dont_copy_let_global :
-// CHECK:         global_addr
-// CHECK-NEXT:    load_borrow
-// CHECK-NEXT:    begin_borrow
-// CHECK-NEXT:    apply
-// CHECK-NEXT:    end_borrow
-// CHECK-NEXT:    end_borrow
-// CHECK-NEXT:    return
-// CHECK-NEXT: } // end sil function 'dont_copy_let_global'
-sil [ossa] @dont_copy_let_global : $@convention(thin) () -> () {
-bb0:
-  %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
-
-  %p = global_addr @a_let_global : $*Klass
+  %p = ref_element_addr [immutable] %u : $ClassLet, #ClassLet.aLet
   %v = load [copy] %p : $*Klass
   %b = begin_borrow %v : $Klass
   apply %f(%b) : $@convention(thin) (@guaranteed Klass) -> ()
@@ -295,7 +272,7 @@ sil [ossa] @dont_copy_let_properties_with_guaranteed_base_structural : $@convent
 bb0(%x : @guaranteed $ClassLet):
   %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
 
-  %p = ref_element_addr %x : $ClassLet, #ClassLet.aLetTuple
+  %p = ref_element_addr [immutable] %x : $ClassLet, #ClassLet.aLetTuple
   %q = tuple_element_addr %p : $*(Klass, Klass), 1
   %v = load [copy] %q : $*Klass
   %b = begin_borrow %v : $Klass
@@ -371,7 +348,7 @@ bb0(%x : @owned $ClassLet):
   %f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
 
   %a = begin_borrow %x : $ClassLet
-  %p = ref_element_addr %a : $ClassLet, #ClassLet.aLet
+  %p = ref_element_addr [immutable] %a : $ClassLet, #ClassLet.aLet
   %v = load [copy] %p : $*Klass
   apply %f(%v) : $@convention(thin) (@guaranteed Klass) -> ()
   destroy_value %v : $Klass
@@ -411,7 +388,7 @@ bb0(%x : @owned $ClassLet):
   %f = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
 
   %a = begin_borrow %x : $ClassLet
-  %p = ref_element_addr %a : $ClassLet, #ClassLet.aLet
+  %p = ref_element_addr [immutable] %a : $ClassLet, #ClassLet.aLet
   %v = load [copy] %p : $*Klass
   %v_cast = unchecked_ref_cast %v : $Klass to $Builtin.NativeObject
   apply %f(%v_cast) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
@@ -1147,7 +1124,7 @@ bb0(%0 : @guaranteed $FakeOptional<ClassLet>):
   switch_enum %0 : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt: bb1, case #FakeOptional.none!enumelt: bb2
 
 bb1(%1 : @guaranteed $ClassLet):
-  %2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
+  %2 = ref_element_addr [immutable] %1 : $ClassLet, #ClassLet.aLet
   %3 = load [copy] %2 : $*Klass
   apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
   destroy_value %3 : $Klass
@@ -1173,7 +1150,7 @@ bb0(%0 : @owned $FakeOptional<ClassLet>):
   switch_enum %0a : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt: bb1, case #FakeOptional.none!enumelt: bb2
 
 bb1(%1 : @guaranteed $ClassLet):
-  %2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
+  %2 = ref_element_addr [immutable] %1 : $ClassLet, #ClassLet.aLet
   %3 = load [copy] %2 : $*Klass
   apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
   destroy_value %3 : $Klass
@@ -1203,7 +1180,7 @@ bb0(%0 : $*FakeOptional<ClassLet>):
   switch_enum %0a : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt: bb1, case #FakeOptional.none!enumelt: bb2
 
 bb1(%1 : @guaranteed $ClassLet):
-  %2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
+  %2 = ref_element_addr [immutable] %1 : $ClassLet, #ClassLet.aLet
   %3 = load [copy] %2 : $*Klass
   apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
   destroy_value %3 : $Klass
@@ -1242,7 +1219,7 @@ bb0c(%0c : @guaranteed $FakeOptional<ClassLet>):
   switch_enum %0d : $FakeOptional<ClassLet>, case #FakeOptional.some!enumelt: bb1, case #FakeOptional.none!enumelt: bb2
 
 bb1(%1 : @guaranteed $ClassLet):
-  %2 = ref_element_addr %1 : $ClassLet, #ClassLet.aLet
+  %2 = ref_element_addr [immutable] %1 : $ClassLet, #ClassLet.aLet
   %3 = load [copy] %2 : $*Klass
   apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
   destroy_value %3 : $Klass
@@ -1278,7 +1255,7 @@ bb1:
 bb2:
   %0b = begin_borrow %0 : $FakeOptional<ClassLet>
   %0b2 = unchecked_enum_data %0b : $FakeOptional<ClassLet>, #FakeOptional.some!enumelt
-  %2 = ref_element_addr %0b2 : $ClassLet, #ClassLet.aLet
+  %2 = ref_element_addr [immutable] %0b2 : $ClassLet, #ClassLet.aLet
   %3 = load [copy] %2 : $*Klass
   apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> ()
   destroy_value %3 : $Klass
@@ -1522,12 +1499,9 @@ bbEnd:
   return %9999 : $()
 }
 
-// Just make sure that we do not crash on this code and convert the 2nd load
-// [copy] to a load_borrow.
+// Just make sure that we do not crash on this code
 //
 // CHECK-LABEL: sil [ossa] @improper_dead_end_block_crasher_test : $@convention(thin) (Builtin.RawPointer) -> () {
-// CHECK: load_borrow
-// CHECK: load_borrow
 // CHECK: } // end sil function 'improper_dead_end_block_crasher_test'
 sil [ossa] @improper_dead_end_block_crasher_test : $@convention(thin) (Builtin.RawPointer) -> () {
 bb0(%0 : $Builtin.RawPointer):

test/SILOptimizer/mem-behavior.sil

@@ -19,6 +19,11 @@ class C {
   @_hasStorage @_hasInitialValue final var prop: Builtin.Int32 { get }
 }
 
+class CL {
+  @_hasStorage let x: String
+}
+
+
 class Parent {
   @_hasStorage var child: C { get set }
 }
@@ -34,6 +39,7 @@ sil @nouser_func : $@convention(thin) () -> ()
 sil @in_ptr : $@convention(thin) (@in Builtin.RawPointer) -> ()
 sil @init_C : $@convention(thin) () -> @out C
 sil @read_C : $@convention(thin) (@in_guaranteed C) -> ()
+sil @consume : $@convention(thin) (@owned CL) -> ()
 
 sil @store_to_int : $@convention(thin) (Int32, @inout Int32) -> () {
 [%1: write v**]
@@ -835,23 +841,6 @@ bb0(%0 : $*C, %1 : $*C):
 sil_global hidden [let] @globalC : $C
 sil_global hidden @globalCVar : $C
 
-// CHECK-LABEL: @testGlobalLet
-// CHECK:      PAIR #1.
-// CHECK-NEXT:     %3 = apply %2() : $@convention(thin) () -> ()
-// CHECK-NEXT:     %0 = global_addr @globalC : $*C
-// CHECK-NEXT:   r=1,w=0
-sil hidden @testGlobalLet : $@convention(thin) () -> () {
-bb0:
-  %0 = global_addr @globalC : $*C
-  %1 = load %0 : $*C
-  %2 = function_ref @nouser_func : $@convention(thin) () -> ()
-  %3 = apply %2() : $@convention(thin) () -> ()
-  %4 = function_ref @read_C : $@convention(thin) (@in_guaranteed C) -> ()
-  %5 = apply %4(%0) : $@convention(thin) (@in_guaranteed C) -> ()
-  %8 = tuple ()
-  return %8 : $()
-}
-
 // CHECK-LABEL: @testInitGlobalLet
 // CHECK:      PAIR #0.
 // CHECK-NEXT:     %2 = apply %1(%0) : $@convention(thin) () -> @out C
@@ -896,25 +885,6 @@ bb0:
   return %1 : $Builtin.RawPointer
 }
 
-// CHECK-LABEL: @testGlobalViaAddressorLet
-// CHECK:      PAIR #2.
-// CHECK-NEXT:     %5 = apply %4() : $@convention(thin) () -> ()
-// CHECK-NEXT:     %2 = pointer_to_address %1 : $Builtin.RawPointer to [strict] $*C
-// CHECK-NEXT:   r=1,w=0
-sil @testGlobalViaAddressorLet : $@convention(thin) () -> () {
-bb0:
-  %0 = function_ref @addressor_of_globalC : $@convention(thin) () -> Builtin.RawPointer
-  %1 = apply %0() : $@convention(thin) () -> Builtin.RawPointer
-  %2 = pointer_to_address %1 : $Builtin.RawPointer to [strict] $*C
-  %3 = load %2 : $*C
-  %4 = function_ref @nouser_func : $@convention(thin) () -> ()
-  %5 = apply %4() : $@convention(thin) () -> ()
-  %6 = function_ref @read_C : $@convention(thin) (@in_guaranteed C) -> ()
-  %7 = apply %6(%2) : $@convention(thin) (@in_guaranteed C) -> ()
-  %8 = tuple ()
-  return %8 : $()
-}
-
 // CHECK-LABEL: @testGlobalViaAddressorVar
 // CHECK:      PAIR #2.
 // CHECK-NEXT:     %5 = apply %4() : $@convention(thin) () -> ()
@@ -1769,3 +1739,45 @@ bb0(%0 :  $*X):
   return %2 : $C
 }
 
+// CHECK-LABEL: @test_release_of_class_with_let
+// CHECK:       PAIR #0.
+// CHECK-NEXT:     strong_release %0 : $CL
+// CHECK-NEXT:     %1 = ref_element_addr [immutable] %0 : $CL, #CL.x
+// CHECK-NEXT:   r=1,w=1
+sil @test_release_of_class_with_let : $@convention(thin) (@owned CL) -> () {
+bb0(%0 : $CL):
+  %1 = ref_element_addr [immutable] %0 : $CL, #CL.x
+  strong_release %0 : $CL
+  %3 = tuple ()
+  return %3 : $()
+}
+
+// CHECK-LABEL: @test_consume_of_class_with_let
+// CHECK:       PAIR #0.
+// CHECK-NEXT:      %3 = apply %2(%0) : $@convention(thin) (@owned CL) -> ()
+// CHECK-NEXT:      %1 = ref_element_addr [immutable] %0 : $CL, #CL.x
+// CHECK-NEXT:    r=1,w=1
+sil @test_consume_of_class_with_let : $@convention(thin) (@owned CL) -> () {
+bb0(%0 : $CL):
+  %1 = ref_element_addr [immutable] %0 : $CL, #CL.x
+  %4 = function_ref @consume : $@convention(thin) (@owned CL) -> ()
+  apply %4(%0) : $@convention(thin) (@owned CL) -> ()
+  %3 = tuple ()
+  return %3 : $()
+}
+
+// CHECK-LABEL: @test_ossa_consume_of_class_with_let
+// CHECK:       PAIR #1.
+// CHECK-NEXT:      %5 = apply %4(%0) : $@convention(thin) (@owned CL) -> ()
+// CHECK-NEXT:      %2 = ref_element_addr [immutable] %1 : $CL, #CL.x
+// CHECK-NEXT:    r=1,w=1
+sil [ossa] @test_ossa_consume_of_class_with_let : $@convention(thin) (@owned CL) -> () {
+bb0(%0 : @owned $CL):
+  %1 = begin_borrow %0 : $CL
+  %2 = ref_element_addr [immutable] %1 : $CL, #CL.x
+  end_borrow %1 : $CL
+  %4 = function_ref @consume : $@convention(thin) (@owned CL) -> ()
+  apply %4(%0) : $@convention(thin) (@owned CL) -> ()
+  %3 = tuple ()
+  return %3 : $()
+}

test/SILOptimizer/temp_rvalue_opt.sil

@@ -25,9 +25,15 @@ struct Str {
   var _value: Builtin.Int64
 }
 
+class C {
+  @_hasStorage let x: String
+}
+
+
 sil @unknown : $@convention(thin) () -> ()
 sil @load_string : $@convention(thin) (@in_guaranteed String) -> String
 sil @guaranteed_user : $@convention(thin) (@guaranteed Klass) -> ()
+sil @consume : $@convention(thin) (@owned C) -> ()
 
 sil @inguaranteed_user_without_result : $@convention(thin) (@in_guaranteed Klass) -> () {
 bb0(%0 : $*Klass):
@@ -761,3 +767,38 @@ bb0(%0 : $Klass, %1 : $Klass):
   dealloc_stack %3 : $*String
   return %323 : $String
 }
+
+// CHECK-LABEL: sil @dont_remove_copy_from_released_object
+// CHECK:         copy_addr
+// CHECK-NEXT:    strong_release %0
+// CHECK:       } // end sil function 'dont_remove_copy_from_released_object'
+sil @dont_remove_copy_from_released_object : $@convention(thin) (@owned C) -> @owned String {
+bb0(%0 : $C):
+  %1 = ref_element_addr [immutable] %0 : $C, #C.x
+  %2 = alloc_stack $String
+  copy_addr %1 to [init] %2 : $*String
+  strong_release %0 : $C
+  %5 = load %2 : $*String
+  retain_value %5 : $String
+  destroy_addr %2 : $*String
+  dealloc_stack %2 : $*String
+  return %5 : $String
+}
+
+// CHECK-LABEL: sil @dont_remove_copy_from_consumed_object
+// CHECK:         copy_addr
+// CHECK:         apply
+// CHECK:       } // end sil function 'dont_remove_copy_from_consumed_object'
+sil @dont_remove_copy_from_consumed_object : $@convention(thin) (@owned C) -> @owned String {
+bb0(%0 : $C):
+  %1 = ref_element_addr [immutable] %0 : $C, #C.x
+  %2 = alloc_stack $String
+  copy_addr %1 to [init] %2 : $*String
+  %4 = function_ref @consume : $@convention(thin) (@owned C) -> ()
+  apply %4(%0) : $@convention(thin) (@owned C) -> ()
+  %5 = load %2 : $*String
+  retain_value %5 : $String
+  destroy_addr %2 : $*String
+  dealloc_stack %2 : $*String
+  return %5 : $String
+}