ComputeSideEffects: compute properties even if function has an effect attribute

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeSideEffects.swift

@@ -37,43 +37,109 @@ let computeSideEffects = FunctionPass(name: "compute-side-effects") {
     return
   }
 
-  if function.effectAttribute != .none {
-    // Don't try to infer side effects if there are defined effect attributes.
-    return
-  }
-
   var collectedEffects = CollectedEffects(function: function, context)
 
-  // First step: collect effects from all instructions.
-  //
+  // Collect effects from all instructions.
   for block in function.blocks {
     for inst in block.instructions {
       collectedEffects.addInstructionEffects(inst)
     }
   }
-
-  // Second step: If an argument has unknown uses, we must add all previously collected
-  // global effects to the argument, because we don't know to which "global" side-effect
-  // instruction the argument might have escaped.
+  // If an argument has unknown uses, we must add all previously collected
+  // global effects to the argument, because we don't know to which "global"
+  // side-effect instruction the argument might have escaped.
   for argument in function.arguments {
     collectedEffects.addEffectsForEscapingArgument(argument: argument)
     collectedEffects.addEffectsForConsumingArgument(argument: argument)
   }
 
+  let globalEffects: SideEffects.GlobalEffects
+  do {
+    let computed = collectedEffects.globalEffects
+
+    // Combine computed global effects with effects defined by the function's effect attribute, if it has one.
+
+    // The defined and computed global effects of a function with an effect attribute should be treated as
+    // worst case global effects of the function.
+    // This means a global effect should only occur iff it is computed AND defined to occur.
+
+    let defined = function.definedGlobalEffects
+
+    globalEffects = SideEffects.GlobalEffects(
+      memory: SideEffects.Memory(read: defined.memory.read && computed.memory.read,
+                                 write: defined.memory.write && computed.memory.write),
+      ownership: SideEffects.Ownership(copy: defined.ownership.copy && computed.ownership.copy,
+                                       destroy: defined.ownership.destroy && computed.ownership.destroy),
+      allocates: defined.allocates && computed.allocates,
+      isDeinitBarrier: defined.isDeinitBarrier && computed.isDeinitBarrier
+    )
+  }
+
+  // Obtain the argument effects of the function.
+  var argumentEffects = collectedEffects.argumentEffects
+
+  // `[readnone]` and `[readonly]` functions can still access the value fields
+  // of their indirect arguments, permitting v** read and write effects. If
+  // additional read or write effects are computed, we can replace.
+  switch function.effectAttribute {
+  case .readNone:
+    for i in (0..<argumentEffects.count) {
+      // Even a `[readnone]` function can read from indirect arguments.
+      if !function.argumentConventions[i].isIndirectIn {
+        argumentEffects[i].read = nil
+      } else if argumentEffects[i].read?.mayHaveClassProjection ?? false {
+        argumentEffects[i].read = SmallProjectionPath(.anyValueFields)
+      }
+
+      // Even a `[readnone]` function can write to indirect results.
+      if !function.argument(at: i).isIndirectResult {
+        argumentEffects[i].write = nil
+      } else if argumentEffects[i].write?.mayHaveClassProjection ?? false {
+        argumentEffects[i].write = SmallProjectionPath(.anyValueFields)
+      }
+
+      argumentEffects[i].copy = nil
+      argumentEffects[i].destroy = nil
+    }
+
+  case .readOnly:
+    for i in (0..<argumentEffects.count) {
+      // Even a `[readonly]` function can write to indirect results.
+      if !function.argument(at: i).isIndirectResult {
+        argumentEffects[i].write = nil
+      } else if argumentEffects[i].write?.mayHaveClassProjection ?? false {
+        argumentEffects[i].write = SmallProjectionPath(.anyValueFields)
+      }
+
+      argumentEffects[i].destroy = nil
+    }
+
+  case .releaseNone:
+    for i in (0..<argumentEffects.count) {
+      // A `[releasenone]` function can do anything except destroy an argument.
+      argumentEffects[i].destroy = nil
+    }
+
+  case .none:
+    // The user makes no additional guarantees about the effects of the function.
+    break
+  }
+
+
   // Don't modify the effects if they didn't change. This avoids sending a change notification
   // which can trigger unnecessary other invalidations.
   if let existingEffects = function.effects.sideEffects,
-     existingEffects.arguments == collectedEffects.argumentEffects,
-     existingEffects.global == collectedEffects.globalEffects {
+     existingEffects.arguments == argumentEffects,
+     existingEffects.global == globalEffects {
     return
   }
 
   // Finally replace the function's side effects.
   function.modifyEffects(context) { (effects: inout FunctionEffects) in
     let globalEffects = function.isProgramTerminationPoint ?
-                            collectedEffects.globalEffects.forProgramTerminationPoints
-                          : collectedEffects.globalEffects
-    effects.sideEffects = SideEffects(arguments: collectedEffects.argumentEffects, global: globalEffects)
+      globalEffects.forProgramTerminationPoints
+      : globalEffects
+    effects.sideEffects = SideEffects(arguments: argumentEffects, global: globalEffects)
   }
 }
 

test/SILOptimizer/side_effects.sil

@@ -1281,3 +1281,119 @@ bb0(%0: $any Error):
   %r = tuple ()
   return %r : $()
 }
+
+// Base case: @destroy_not_escaped_closure_test. It has every global and argument effect, so we can see which are removed.
+// CHECK-LABEL: sil [readnone] @test_effect_attribute_readnone
+// CHECK-NEXT:  [global: copy,deinit_barrier]
+// CHECK-NEXT:  {{^[^[]}}
+// CHECK-LABEL: } // end sil function 'test_effect_attribute_readnone'
+sil [readnone] @test_effect_attribute_readnone : $@convention(thin)(@owned @callee_guaranteed () -> ()) -> Builtin.Int1 {
+bb0(%0 : $@callee_guaranteed () -> ()):
+  %1 = destroy_not_escaped_closure %0: $@callee_guaranteed () -> ()
+  return %1 : $Builtin.Int1
+}
+
+// CHECK-LABEL: sil [readonly] @test_effect_attribute_readonly
+// CHECK-NEXT:  [%0: read v**.c*.v**, copy v**.c*.v**]
+// CHECK-NEXT:  [global: read,copy,deinit_barrier]
+// CHECK-NEXT:  {{^[^[]}}
+// CHECK-LABEL: } // end sil function 'test_effect_attribute_readonly'
+sil [readonly] @test_effect_attribute_readonly : $@convention(thin)(@owned @callee_guaranteed () -> ()) -> Builtin.Int1 {
+bb0(%0 : $@callee_guaranteed () -> ()):
+  %1 = destroy_not_escaped_closure %0: $@callee_guaranteed () -> ()
+  return %1 : $Builtin.Int1
+}
+
+// CHECK-LABEL: sil [releasenone] @test_effect_attribute_releasenone
+// CHECK-NEXT:  [%0: read v**.c*.v**, write v**.c*.v**, copy v**.c*.v**]
+// CHECK-NEXT:  [global: read,write,copy,allocate,deinit_barrier]
+// CHECK-NEXT:  {{^[^[]}}
+// CHECK-LABEL: } // end sil function 'test_effect_attribute_releasenone'
+sil [releasenone] @test_effect_attribute_releasenone : $@convention(thin)(@owned @callee_guaranteed () -> ()) -> Builtin.Int1 {
+bb0(%0 : $@callee_guaranteed () -> ()):
+  %1 = destroy_not_escaped_closure %0: $@callee_guaranteed () -> ()
+  return %1 : $Builtin.Int1
+}
+
+// These tests are adapted from the test case in rdar://155870190
+sil @test_effect_attribute_no_barrier : $@convention(thin) () -> () {
+  [global: ]
+}
+
+sil @test_effect_attribute_barrier : $@convention(thin) () -> () {
+  [global: deinit_barrier]
+}
+// CHECK-LABEL: sil [readnone] @test_effect_attribute_call_no_barrier
+// CHECK-NEXT:  [global: ]
+// CHECK-NEXT:  {{^[^[]}}
+// CHECK-LABEL: } // end sil function 'test_effect_attribute_call_no_barrier'
+sil [readnone] @test_effect_attribute_call_no_barrier : $@convention(thin) () -> @out Any {
+bb0(%0 : $*Any):
+  %2 = function_ref @test_effect_attribute_no_barrier : $@convention(thin) () -> ()
+  apply %2() : $@convention(thin) () -> ()
+  %4 = tuple ()
+  return %4
+}
+
+// CHECK-LABEL: sil [readnone] @test_effect_attribute_call_barrier
+// CHECK-NEXT:  [global: deinit_barrier]
+// CHECK-NEXT:  {{^[^[]}}
+// CHECK-LABEL: } // end sil function 'test_effect_attribute_call_barrier'
+sil [readnone] @test_effect_attribute_call_barrier : $@convention(thin) () -> @out Any {
+bb0(%0 : $*Any):
+  %2 = function_ref @test_effect_attribute_barrier : $@convention(thin) () -> ()
+  apply %2() : $@convention(thin) () -> ()
+  %4 = tuple ()
+  return %4
+}
+
+// The readnone attribute can not rule out reads from indirect arguments.
+// CHECK-LABEL: sil [readnone] @test_effect_attribute_readnone_direct_argument
+// CHECK-NEXT:  [global: ]
+// CHECK-NEXT:  {{^[^[]}}
+// CHECK-LABEL: } // end sil function 'test_effect_attribute_readnone_direct_argument'
+sil [readnone] @test_effect_attribute_readnone_direct_argument : $@convention(thin) (Int) -> Int {
+bb0(%0 : $Int):
+  return %0
+}
+
+// CHECK-LABEL: sil [readnone] @test_effect_attribute_readnone_indirect_argument
+// CHECK-NEXT:  [%0: read v**]
+// CHECK-NEXT:  [global: ]
+// CHECK-NEXT:  {{^[^[]}}
+// CHECK-LABEL: } // end sil function 'test_effect_attribute_readnone_indirect_argument'
+sil [readnone] @test_effect_attribute_readnone_indirect_argument : $@convention(thin) (@in Int) -> Int {
+bb0(%0 : $*Int):
+  %1 = load %0
+  return %1
+}
+
+// Base case: @retain_and_store. The indirect result has a write effect, which readnone & readonly can't rule out.
+// CHECK-LABEL: sil [readnone] @test_effect_attribute_readnone_indirect_result
+// CHECK-NEXT:  [%0: write v**]
+// CHECK-NEXT:  [global: ]
+// CHECK-NEXT:  {{^[^[]}}
+// CHECK-LABEL: } // end sil function 'test_effect_attribute_readnone_indirect_result'
+sil [readnone] @test_effect_attribute_readnone_indirect_result : $@convention(thin) (@guaranteed X) -> @out X {
+bb0(%0 : $*X, %1 : $X):
+  strong_retain %1 : $X
+  store %1 to %0  : $*X
+
+  %r = tuple ()
+  return %r : $()
+}
+
+// CHECK-LABEL: sil [readonly] @test_effect_attribute_readonly_indirect_result
+// CHECK-NEXT:  [%0: write v**]
+// CHECK-NEXT:  [%1: copy v**]
+// CHECK-NEXT:  [global: ]
+// CHECK-NEXT:  {{^[^[]}}
+// CHECK-LABEL: } // end sil function 'test_effect_attribute_readonly_indirect_result'
+sil [readonly] @test_effect_attribute_readonly_indirect_result : $@convention(thin) (@guaranteed X) -> @out X {
+bb0(%0 : $*X, %1 : $X):
+  strong_retain %1 : $X
+  store %1 to %0  : $*X
+
+  %r = tuple ()
+  return %r : $()
+}

test/SILOptimizer/templvalueopt_ossa.sil

@@ -336,6 +336,39 @@ bb0(%0 : @guaranteed $Any, %1 : $*Any):
   return %11 : $()
 }
 
+sil [readnone] @createAny_effects : $@convention(thin) () -> @out Any
+sil [readnone] @createAny_no_barrier_effects : $@convention(thin) () -> @out Any {
+ [global:]
+}
+
+// CHECK-LABEL: sil [ossa] @test_deinit_barrier_effects :
+// CHECK:         copy_addr [take]
+// CHECK-LABEL: } // end sil function 'test_deinit_barrier_effects'
+sil [ossa] @test_deinit_barrier_effects : $@convention(thin) (@guaranteed Any, @inout Any) -> () {
+bb0(%0 : @guaranteed $Any, %1 : $*Any):
+  %2 = alloc_stack $Any
+  %4 = function_ref @createAny_effects : $@convention(thin) () -> @out Any
+  %5 = apply %4(%2) : $@convention(thin) () -> @out Any
+  copy_addr [take] %2 to %1 : $*Any
+  dealloc_stack %2 : $*Any
+  %11 = tuple ()
+  return %11 : $()
+}
+
+// CHECK-LABEL: sil [ossa] @test_no_deinit_barrier_effects :
+// CHECK-NOT:     copy_addr
+// CHECK-LABEL: } // end sil function 'test_no_deinit_barrier_effects'
+sil [ossa] @test_no_deinit_barrier_effects : $@convention(thin) (@guaranteed Any, @inout Any) -> () {
+bb0(%0 : @guaranteed $Any, %1 : $*Any):
+  %2 = alloc_stack $Any
+  %4 = function_ref @createAny_no_barrier_effects : $@convention(thin) () -> @out Any
+  %5 = apply %4(%2) : $@convention(thin) () -> @out Any
+  copy_addr [take] %2 to %1 : $*Any
+  dealloc_stack %2 : $*Any
+  %11 = tuple ()
+  return %11 : $()
+}
+
 struct TwoFields {
   var a: Child
   var b: Child