Effects: bail if effects are requested for not supported projection paths

 Effects are only defined for operations which don't involve a load.
 In case the argument's path involves a load we need to return the global effects.

Author: eeckstein

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeSideEffects.swift

@@ -299,7 +299,7 @@ private struct CollectedEffects {
           if let calleePath = calleeEffect.destroy { addEffects(.destroy, to: argument, fromInitialPath: calleePath) }
         } else {
           let convention = callee.getArgumentConvention(for: calleeArgIdx)
-          let wholeArgument = argument.at(SmallProjectionPath(.anything))
+          let wholeArgument = argument.at(defaultPath(for: argument))
           let calleeEffects = callee.getSideEffects(forArgument: wholeArgument,
                                                     atIndex: calleeArgIdx,
                                                     withConvention: convention)
@@ -313,8 +313,12 @@ private struct CollectedEffects {
   ///
   /// If the value comes from an argument (or mutliple arguments), then the effects are added
   /// to the corrseponding `argumentEffects`. Otherwise they are added to the `global` effects.
+  private mutating func addEffects(_ effects: SideEffects.GlobalEffects, to value: Value) {
+    addEffects(effects, to: value, fromInitialPath: defaultPath(for: value))
+  }
+
   private mutating func addEffects(_ effects: SideEffects.GlobalEffects, to value: Value,
-                                   fromInitialPath: SmallProjectionPath? = nil) {
+                                   fromInitialPath: SmallProjectionPath) {
 
     /// Collects the (non-address) roots of a value.
     struct GetRootsWalker : ValueUseDefWalker {
@@ -346,10 +350,7 @@ private struct CollectedEffects {
 
     var findRoots = GetRootsWalker(context)
     if value.type.isAddress {
-      // If there is no initial path provided, select all value fields.
-      let path = fromInitialPath ?? SmallProjectionPath(.anyValueFields)
-
-      let accessPath = value.getAccessPath(fromInitialPath: path)
+      let accessPath = value.getAccessPath(fromInitialPath: fromInitialPath)
       switch accessPath.base {
         case .stack:
           // We don't care about read and writes from/to stack locations (because they are
@@ -369,16 +370,7 @@ private struct CollectedEffects {
           }
       }
     } else {
-      // Handle non-address `value`s which are projections from a direct arguments.
-      let path: SmallProjectionPath
-      if let fromInitialPath = fromInitialPath {
-        path = fromInitialPath
-      } else if value.type.isClass {
-        path = SmallProjectionPath(.anyValueFields).push(.anyClassField)
-      } else {
-        path = SmallProjectionPath(.anyValueFields).push(.anyClassField).push(.anyValueFields)
-      }
-      _ = findRoots.walkUp(value: value, path: path)
+      _ = findRoots.walkUp(value: value, path: fromInitialPath)
     }
     // Because of phi-arguments, a single (non-address) `value` can come from multiple arguments.
     while let (arg, path) = findRoots.roots.pop() {
@@ -391,6 +383,16 @@ private struct CollectedEffects {
   }
 }
 
+private func defaultPath(for value: Value) -> SmallProjectionPath {
+  if value.type.isAddress {
+    return SmallProjectionPath(.anyValueFields)
+  }
+  if value.type.isClass {
+    return SmallProjectionPath(.anyValueFields).push(.anyClassField)
+  }
+  return SmallProjectionPath(.anyValueFields).push(.anyClassField).push(.anyValueFields)
+}
+
 /// Checks if an argument escapes to some unknown user.
 private struct ArgumentEscapingWalker : ValueDefUseWalker, AddressDefUseWalker {
   var walkDownCache = WalkerCache<UnusedWalkingPath>()

SwiftCompilerSources/Sources/SIL/Effects.swift

@@ -88,9 +88,41 @@ extension Function {
   public func getSideEffects(forArgument argument: ProjectedValue,
                              atIndex argumentIndex: Int,
                              withConvention convention: ArgumentConvention) -> SideEffects.GlobalEffects {
+    var result = SideEffects.GlobalEffects()
+
+    // Effects are only defined for operations which don't involve a load.
+    // In case the argument's path involves a load we need to return the global effects.
+    if argument.value.type.isAddress {
+      // Indirect arguments:
+      if argument.path.mayHaveClassProjection {
+        // For example:
+        //   bb0(%0: $*C):
+        //     %1 = load %0               // the involved load
+        //     %2 = ref_element_addr %1   // class projection
+        return getSideEffects()
+      }
+    } else {
+      // Direct arguments:
+      if argument.path.mayHaveTwoClassProjections {
+        // For example:
+        //   bb0(%0: $C):
+        //     %1 = ref_element_addr %1   // first class projection
+        //     %2 = load %1               // the involved load
+        //     %3 = ref_element_addr %2   // second class projection
+        return getSideEffects()
+
+      } else if argument.path.mayHaveClassProjection {
+        // For example:
+        //   bb0(%0: $C):
+        //     %1 = ref_element_addr %1   // class projection
+        //     %2 = load [take] %1        // the involved load
+        //     destroy_value %2
+        result.ownership = getSideEffects().ownership
+      }
+    }
+
     if let sideEffects = effects.sideEffects {
       /// There are computed side effects.
-      var result = SideEffects.GlobalEffects()
       let argEffect = sideEffects.getArgumentEffects(for: argumentIndex)
       if let effectPath = argEffect.read, effectPath.mayOverlap(with: argument.path) {
         result.memory.read = true

test/SILOptimizer/mem-behavior.sil

@@ -16,7 +16,7 @@ class X {
 class Derived : X { }
 
 class C {
-  @_hasStorage @_hasInitialValue final let prop: Builtin.Int32 { get }
+  @_hasStorage @_hasInitialValue final var prop: Builtin.Int32 { get }
 }
 
 class Parent {
@@ -950,3 +950,33 @@ bb0 (%0 : $*C):
   return %3 : $()
 }
 
+sil @do_store : $@convention(thin) (@guaranteed Parent) -> () {
+[%0: noescape **, read c0.v**]
+[global: write]
+}
+
+// CHECK-LABEL: @test_double_class_indirection
+// CHECK:       PAIR #4.
+// CHECK-NEXT:      %8 = apply %7(%1) : $@convention(thin) (@guaranteed Parent) -> ()
+// CHECK-NEXT:      %3 = ref_element_addr %1 : $Parent, #Parent.child
+// CHECK-NEXT:    r=1,w=0
+// CHECK-NEXT:  PAIR #5.
+// CHECK-NEXT:      %8 = apply %7(%1) : $@convention(thin) (@guaranteed Parent) -> ()
+// CHECK-NEXT:      %5 = ref_element_addr %2 : $C, #C.prop
+// CHECK-NEXT:    r=1,w=1
+sil @test_double_class_indirection : $@convention(thin) (Builtin.Int32) -> () {
+bb0(%0 : $Builtin.Int32):
+  %1 = alloc_ref $Parent
+  %2 = alloc_ref $C
+  %3 = ref_element_addr %1 : $Parent, #Parent.child
+  store %2 to %3 : $*C
+  %5 = ref_element_addr %2 : $C, #C.prop
+  store %0 to %5 : $*Builtin.Int32
+
+  %f = function_ref @do_store : $@convention(thin) (@guaranteed Parent) -> ()
+  %a = apply %f(%1) : $@convention(thin) (@guaranteed Parent) -> ()
+
+  %r = tuple ()
+  return %r : $()
+}
+