Fix SideEffectsAnalysis for a function with defined @effects

We weren't looking at the parameter conventions that resulted in
different side-effects than the defined @effects.

Author: meg-gupta

include/swift/SILOptimizer/Analysis/SideEffectAnalysis.h

@@ -418,6 +418,9 @@ class FunctionSideEffects {
   /// Returns true if \a F has an @_effects attribute which could be handled.
   bool setDefinedEffects(SILFunction *F);
 
+  /// Set the parameter effects of a function by looking at their conventions
+  bool setParamEffects(SILFunction *F);
+
   /// Set the side-effects of a semantic call.
   /// Return true if \p ASC could be handled.
   bool setSemanticEffects(ArraySemanticsCall ASC);

lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp

@@ -341,32 +341,45 @@ FunctionSideEffectFlags *FunctionSideEffects::getEffectsOn(SILValue Addr) {
   return &GlobalEffects;
 }
 
+bool FunctionSideEffects::setParamEffects(SILFunction *F) {
+  for (auto i :
+       range(F->getLoweredFunctionType()->getNumIndirectFormalResults())) {
+    ParamEffects[i].Writes = true;
+  }
+  for (auto &paramAndIdx :
+       enumerate(F->getLoweredFunctionType()->getParameters())) {
+    auto &param = paramAndIdx.value();
+    if (param.isIndirectMutating() || param.isConsumed()) {
+      return false;
+    }
+  }
+  return true;
+}
+
 // Return true if the given function has defined effects that were successfully
 // recorded in this FunctionSideEffects object.
 bool FunctionSideEffects::setDefinedEffects(SILFunction *F) {
   if (F->hasSemanticsAttr(SEMANTICS_PROGRAMTERMINATION_POINT)) {
     Traps = true;
     return true;
   }
+
   switch (F->getEffectsKind()) {
-    case EffectsKind::ReleaseNone:
-      GlobalEffects.Reads = true;
-      GlobalEffects.Writes = true;
-      GlobalEffects.Releases = false;
-      return true;
-    case EffectsKind::ReadNone:
-      return true;
-    case EffectsKind::ReadOnly:
-      // @_effects(readonly) is worthless if we have owned parameters, because
-      // the release inside the callee may call a deinit, which itself can do
-      // anything.
-      if (!F->hasOwnedParameters()) {
-        GlobalEffects.Reads = true;
-        return true;
-      }
-      break;
-    default:
-      break;
+  case EffectsKind::ReleaseNone: {
+    GlobalEffects.Reads = true;
+    GlobalEffects.Writes = true;
+    GlobalEffects.Releases = false;
+    return setParamEffects(F);
+  }
+  case EffectsKind::ReadNone: {
+    return setParamEffects(F);
+  }
+  case EffectsKind::ReadOnly: {
+    GlobalEffects.Reads = true;
+    return setParamEffects(F);
+  }
+  default:
+    break;
   }
 
   return false;
@@ -523,6 +536,7 @@ void FunctionSideEffects::analyzeInstruction(SILInstruction *I) {
 #include "swift/AST/ReferenceStorage.def"
   case SILInstructionKind::StrongRetainInst:
   case SILInstructionKind::RetainValueInst:
+  case SILInstructionKind::CopyValueInst:
     getEffectsOn(I->getOperand(0))->Retains = true;
     return;
 #define UNCHECKED_REF_STORAGE(Name, ...)                                       \
@@ -532,6 +546,8 @@ void FunctionSideEffects::analyzeInstruction(SILInstruction *I) {
 #include "swift/AST/ReferenceStorage.def"
   case SILInstructionKind::StrongReleaseInst:
   case SILInstructionKind::ReleaseValueInst:
+  case SILInstructionKind::DestroyValueInst:
+  case SILInstructionKind::DestroyAddrInst:
     getEffectsOn(I->getOperand(0))->Releases = true;
     return;
   case SILInstructionKind::UnconditionalCheckedCastInst:

test/SILOptimizer/side-effect.sil

@@ -32,12 +32,12 @@ class X {
 
   init()
 }
-
 sil [_semantics "programtermination_point"] @exitfunc : $@convention(thin) () -> Never
 sil [readnone] @pure_func : $@convention(thin) () -> ()
 sil [releasenone] @releasenone_func : $@convention(thin) () -> ()
 sil [readonly] @readonly_owned : $@convention(thin) (@owned X) -> ()
 sil [readonly] @readonly_guaranteed : $@convention(thin) (@guaranteed X) -> ()
+sil [readnone] [ossa] @get_object : $@convention(thin) () -> (@owned X)
 
 sil @unknown_func : $@convention(thin) () -> ()
 
@@ -551,3 +551,47 @@ bb0:
   return %r : $()
 }
 
+// CHECK-LABEL: sil @readonly_and_store
+// CHECK: <func=r,param0=w>
+sil [readonly] [ossa] @readonly_and_store : $@convention(thin) () -> @out X {
+bb0(%0 : $*X):
+  %f = function_ref @get_object : $@convention(thin) () -> (@owned X)
+  %a = apply %f() : $@convention(thin) () -> (@owned X)
+  store %a to [init] %0  : $*X
+  %r = tuple ()
+  return %r : $()
+}
+
+sil [readonly] [ossa] @readonly_ownedparam : $@convention(thin) (@owned X) -> () {
+bb0(%0 : @owned $X):
+  destroy_value %0 : $X
+  %r = tuple ()
+  return %r : $()
+}
+
+sil [readonly] [ossa] @readonly_inoutparam : $@convention(thin) (@inout X) -> () {
+bb0(%0 : $*X):
+  %f = function_ref @get_object : $@convention(thin) () -> (@owned X)
+  %a = apply %f() : $@convention(thin) () -> (@owned X)
+  destroy_addr %0 : $*X
+  store %a to [init] %0  : $*X
+  %r = tuple ()
+  return %r : $()
+}
+
+sil [readnone] [ossa] @readnone_ownedparam : $@convention(thin) (@owned X) -> () {
+bb0(%0 : @owned $X):
+  destroy_value %0 : $X
+  %r = tuple ()
+  return %r : $()
+}
+
+sil [readnone] [ossa] @readnone_inoutparam : $@convention(thin) (@inout X) -> () {
+bb0(%0 : $*X):
+  %f = function_ref @get_object : $@convention(thin) () -> (@owned X)
+  %a = apply %f() : $@convention(thin) () -> (@owned X)
+  destroy_addr %0 : $*X
+  store %a to [init] %0  : $*X
+  %r = tuple ()
+  return %r : $()
+}