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
   }
@@ -628,6 +615,12 @@ private enum ImmutableScope {
           return nil
         }
         object = tailAddr.instance
+      case .global(let global):
+        if global.isLet && !basedAddress.parentFunction.canInitializeGlobal {
+          self = .wholeFunction
+          return
+        }
+        return nil
       default:
         return nil
       }
@@ -907,6 +900,14 @@ private extension Type {
   }
 }
 
+private extension Function {
+  var canInitializeGlobal: Bool {
+    return isGlobalInitOnceFunction ||
+           // In non -parse-as-library mode globals are initialized in the `main` function.
+           name == "main"
+  }
+}
+
 //===--------------------------------------------------------------------===//
 //                              Bridging
 //===--------------------------------------------------------------------===//

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/copy-to-borrow-optimization.sil

@@ -199,7 +199,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) -> ()
@@ -223,7 +223,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
@@ -247,7 +247,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
+  %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) -> ()
@@ -293,7 +293,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
@@ -369,7 +369,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
@@ -409,7 +409,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) -> ()
@@ -1145,7 +1145,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
@@ -1171,7 +1171,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
@@ -1201,7 +1201,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
@@ -1240,7 +1240,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
@@ -1276,7 +1276,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
@@ -1520,12 +1520,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**]
@@ -857,7 +863,22 @@ bb0:
 // CHECK-NEXT:     %2 = apply %1(%0) : $@convention(thin) () -> @out C
 // CHECK-NEXT:     %0 = global_addr @globalC : $*C
 // CHECK-NEXT:   r=1,w=1
-sil hidden @testInitGlobalLet : $@convention(thin) () -> () {
+sil hidden [global_init_once_fn] @testInitGlobalLet : $@convention(thin) () -> () {
+bb0:
+  %0 = global_addr @globalC : $*C
+  %1 = function_ref @init_C : $@convention(thin) () -> @out C
+  %2 = apply %1(%0) : $@convention(thin) () -> @out C
+  %3 = load %0 : $*C
+  %8 = tuple ()
+  return %8 : $()
+}
+
+// CHECK-LABEL: @main
+// CHECK:      PAIR #0.
+// CHECK-NEXT:     %2 = apply %1(%0) : $@convention(thin) () -> @out C
+// CHECK-NEXT:     %0 = global_addr @globalC : $*C
+// CHECK-NEXT:   r=1,w=1
+sil [global_init_once_fn] @main : $@convention(thin) () -> () {
 bb0:
   %0 = global_addr @globalC : $*C
   %1 = function_ref @init_C : $@convention(thin) () -> @out C
@@ -1788,3 +1809,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.x
+  strong_release %0
+  %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.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
+  %2 = ref_element_addr [immutable] %1, #CL.x
+  end_borrow %1
+  %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.x
+  %2 = alloc_stack $String
+  copy_addr %1 to [init] %2
+  strong_release %0
+  %5 = load %2
+  retain_value %5
+  destroy_addr %2
+  dealloc_stack %2
+  return %5
+}
+
+// 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.x
+  %2 = alloc_stack $String
+  copy_addr %1 to [init] %2
+  %4 = function_ref @consume : $@convention(thin) (@owned C) -> ()
+  apply %4(%0) : $@convention(thin) (@owned C) -> ()
+  %5 = load %2
+  retain_value %5
+  destroy_addr %2
+  dealloc_stack %2
+  return %5
+}