Fix wrong combination of MemoryBehavior.

Instead of taking the maximum we need to handle the special case MayRead + MayWrite = MayReadWrite

Author: eeckstein

include/swift/SIL/SILInstruction.h

@@ -282,6 +282,21 @@ class SILInstruction : public ValueBase,public llvm::ilist_node<SILInstruction>{
   bool isTriviallyDuplicatable() const;
 };
 
+/// Returns the combined behavior of \p B1 and \p B2.
+inline SILInstruction::MemoryBehavior
+combineMemoryBehavior(SILInstruction::MemoryBehavior B1,
+                  SILInstruction::MemoryBehavior B2) {
+  // Basically the combined behavior is the maximum of both operands.
+  auto Result = std::max(B1, B2);
+
+  // With one exception: MayRead, MayWrite -> MayReadWrite.
+  if (Result == SILInstruction::MemoryBehavior::MayWrite &&
+        (B1 == SILInstruction::MemoryBehavior::MayRead ||
+         B2 == SILInstruction::MemoryBehavior::MayRead))
+    return SILInstruction::MemoryBehavior::MayReadWrite;
+  return Result;
+}
+
 #ifndef NDEBUG
 /// Pretty-print the MemoryBehavior.
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,

lib/SILOptimizer/Analysis/MemoryBehavior.cpp

@@ -253,12 +253,13 @@ MemBehavior MemoryBehaviorVisitor::visitApplyInst(ApplyInst *AI) {
          Idx < End && Behavior < MemBehavior::MayHaveSideEffects; ++Idx) {
       auto &ArgEffect = ApplyEffects.getParameterEffects()[Idx];
       auto ArgBehavior = ArgEffect.getMemBehavior(InspectionMode);
-      if (ArgBehavior > Behavior) {
+      auto NewBehavior = combineMemoryBehavior(Behavior, ArgBehavior);
+      if (NewBehavior != Behavior) {
         SILValue Arg = AI->getArgument(Idx);
         // We only consider the argument effects if the argument aliases V.
         if (!Arg.getType().isAddress() ||
             !AA->isNoAlias(Arg, V, computeTBAAType(Arg), getValueTBAAType())) {
-          Behavior = ArgBehavior;
+          Behavior = NewBehavior;
         }
       }
     }

lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp

@@ -37,8 +37,7 @@ FunctionEffects::getMemBehavior(RetainObserveKind ScanKind) const {
   for (auto &ParamEffect : ParamEffects) {
     MemoryBehavior ArgBehavior = ParamEffect.getMemBehavior(ScanKind);
 
-    if (ArgBehavior > Behavior)
-      Behavior = ArgBehavior;
+    Behavior = combineMemoryBehavior(Behavior, ArgBehavior);
 
     // Stop the scan if we've reached the highest level of side effect.
     if (Behavior == MemoryBehavior::MayHaveSideEffects)

test/SILOptimizer/mem-behavior.sil

@@ -167,3 +167,32 @@ bb0(%0 : $*X):
   return %2 : $()
 }
 
+struct TwoInts {
+  var i1 : Int32
+  var i2 : Int32
+}
+
+// CHECK-LABEL: @combination_of_read_and_write_effects
+// CHECK:     PAIR #1.
+// CHECK-NEXT: (0):   %4 = apply %3(%1, %2) : $@convention(thin) (@inout Int32, @inout Int32) -> ()
+// CHECK-NEXT: (1):   %0 = alloc_stack $TwoInts                       // users: %1, %2, %5
+// CHECK-NEXT: r=1,w=1,se=1
+sil @combination_of_read_and_write_effects : $@convention(thin) () -> () {
+bb0:
+  %0 = alloc_stack $TwoInts
+  %1 = struct_element_addr %0#1 : $*TwoInts, #TwoInts.i1
+  %2 = struct_element_addr %0#1 : $*TwoInts, #TwoInts.i2
+  %3 = function_ref @copy_ints : $@convention(thin) (@inout Int32, @inout Int32) -> ()
+  apply %3 (%1, %2) : $@convention(thin) (@inout Int32, @inout Int32) -> ()
+  dealloc_stack %0#0 : $*@local_storage TwoInts
+  %r = tuple()
+  return %r : $()
+}
+
+sil @copy_ints : $@convention(thin) (@inout Int32, @inout Int32) -> () {
+bb0(%0 : $*Int32, %1 : $*Int32):
+  %2 = load %0 : $*Int32
+  store %2 to %1 : $*Int32
+  %r = tuple()
+  return %r : $()
+}