SideEffectAnalysis: don't assume that arguments with trivial type cannot be pointers.

Even values of trivial type can contain a Builtin.RawPointer, which can be used to read/write from/to.

To compensate for the removed check, enable the escape-analysis check in MemBehavior (as it was before).

This fixes a recently introduced miscompile.
rdar://problem/70220876

Author: eeckstein

lib/SILOptimizer/Analysis/MemoryBehavior.cpp

@@ -215,11 +215,7 @@ class MemoryBehaviorVisitor
   SIMPLE_MEMBEHAVIOR_INST(CondFailInst, None)
 #undef SIMPLE_MEMBEHAVIOR_INST
 
-  // If we are asked to treat ref count increments as being inert, return None
-  // for these.
-  //
-  // FIXME: Once we separate the notion of ref counts from reading/writing
-  // memory this will be unnecessary.
+  // Incrementing reference counts doesn't have an observable memory effect.
 #define REFCOUNTINC_MEMBEHAVIOR_INST(Name)                                     \
   MemBehavior visit##Name(Name *I) {                                           \
     return MemBehavior::None;                                                \
@@ -455,7 +451,7 @@ MemBehavior MemoryBehaviorVisitor::getApplyBehavior(FullApplySite AS) {
       behavior = MemBehavior::MayRead;
 
     // Ask escape analysis.
-    if (!nonEscapingAddress && !EA->canEscapeTo(V, AS))
+    if (!EA->canEscapeTo(V, AS))
       behavior = MemBehavior::None;
   }
   LLVM_DEBUG(llvm::dbgs() << "  Found apply, returning " << behavior << '\n');

lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp

@@ -222,11 +222,6 @@ FunctionSideEffects::getMemBehavior(RetainObserveKind ScanKind) const {
 MemoryBehavior
 FunctionSideEffects::getArgumentBehavior(FullApplySite applySite,
                                          unsigned argIdx) {
-  // Rule out trivial non-address argument types.
-  SILType argType = applySite.getArgument(argIdx)->getType();
-  if (!argType.isAddress() && argType.isTrivial(*applySite.getFunction()))
-    return MemoryBehavior::None;
-
   // The overall argument effect is the combination of the argument and the
   // global effects.
   MemoryBehavior behavior =

test/SILOptimizer/dead_store_elim.sil

@@ -469,6 +469,28 @@ bb3:
   return %9999 : $()
 }
 
+sil @read_from_raw_pointer : $@convention(thin) (Builtin.RawPointer) -> UInt8 {
+bb0(%0 : $Builtin.RawPointer):
+  %1 = pointer_to_address %0 : $Builtin.RawPointer to [strict] $*UInt8
+  %2 = load %1 : $*UInt8
+  return %2 : $UInt8
+}
+
+// CHECK-LABEL: sil @dont_remove_store_to_escaping_allocstack_to_known_function : $@convention(thin) (UInt8) -> UInt8
+// CHECK:        alloc_stack
+// CHECK-NEXT:   store
+// CHECK:      } // end sil function 'dont_remove_store_to_escaping_allocstack_to_known_function'
+sil @dont_remove_store_to_escaping_allocstack_to_known_function : $@convention(thin) (UInt8) -> UInt8 {
+bb0(%0 : $UInt8):
+  %1 = alloc_stack $UInt8
+  store %0 to %1 : $*UInt8
+  %3 = address_to_pointer %1 : $*UInt8 to $Builtin.RawPointer
+  %4 = function_ref @read_from_raw_pointer : $@convention(thin) (Builtin.RawPointer) -> UInt8
+  %5 = apply %4(%3) : $@convention(thin) (Builtin.RawPointer) -> UInt8
+  dealloc_stack %1 : $*UInt8
+  return %5 : $UInt8
+}
+
 sil @unknown : $@convention(thin) () -> ()
 
 // CHECK-LABEL: sil @inout_is_not_aliasing : $@convention(thin) (@inout Builtin.Int32) -> () {

test/SILOptimizer/mem-behavior.sil

@@ -35,6 +35,13 @@ bb0(%0 : $X):
   return %r : $()
 }
 
+sil @read_from_raw_pointer : $@convention(thin) (Builtin.RawPointer) -> UInt8 {
+bb0(%0 : $Builtin.RawPointer):
+  %1 = pointer_to_address %0 : $Builtin.RawPointer to [strict] $*UInt8
+  %2 = load %1 : $*UInt8
+  return %2 : $UInt8
+}
+
 // CHECK-LABEL: @call_unknown_func
 // CHECK:     PAIR #1.
 // CHECK-NEXT:   %4 = apply %3(%0, %1) : $@convention(thin) (Int32, @in Int32) -> ()
@@ -224,6 +231,21 @@ bb0(%0 : $*Int32):
   return %5 : $Int32
 }
 
+// CHECK-LABEL:  @escaping_allocstack_to_known_function
+// CHECK:        PAIR #1.
+// CHECK-NEXT:       %5 = apply %4(%3) : $@convention(thin) (Builtin.RawPointer) -> UInt8 // user: %7
+// CHECK-NEXT:       %1 = alloc_stack $UInt8                         // users: %6, %3, %2
+// CHECK-NEXT:     r=1,w=0
+sil @escaping_allocstack_to_known_function : $@convention(thin) (UInt8) -> UInt8 {
+bb0(%0 : $UInt8):
+  %1 = alloc_stack $UInt8
+  store %0 to %1 : $*UInt8
+  %3 = address_to_pointer %1 : $*UInt8 to $Builtin.RawPointer
+  %4 = function_ref @read_from_raw_pointer : $@convention(thin) (Builtin.RawPointer) -> UInt8
+  %5 = apply %4(%3) : $@convention(thin) (Builtin.RawPointer) -> UInt8
+  dealloc_stack %1 : $*UInt8
+  return %5 : $UInt8
+}
 
 // CHECK-LABEL:  @tryapply_allocstack_and_copyaddr
 // CHECK:        PAIR #0.