PredictableMemOpt: be more conservative about address_to_pointer

Handling address_to_pointer as a plain inout missed some mutations and lead to miscompiles.
We now treat address_to_pointer as escaping address.

Fixes SR-3554

Author: eeckstein

lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp

@@ -408,6 +408,15 @@ namespace {
     /// top level of a 'self' variable in a non-delegating init method.
     bool IsSelfOfNonDelegatingInitializer;
 
+    /// How should address_to_pointer be handled?
+    ///
+    /// In DefinitInitialization it is considered as an inout parameter to get
+    /// diagnostics about passing a let variable to an inout mutable-pointer
+    /// argument.
+    /// In PredictableMemOpt it is considered as an escape point to be
+    /// conservative.
+    bool TreatAddressToPointerAsInout;
+
     /// When walking the use list, if we index into a struct element, keep track
     /// of this, so that any indexes into tuple subelements don't affect the
     /// element we attribute an access to.
@@ -421,11 +430,13 @@ namespace {
                         SmallVectorImpl<DIMemoryUse> &Uses,
                         SmallVectorImpl<TermInst*> &FailableInits,
                         SmallVectorImpl<SILInstruction*> &Releases,
-                        bool isDefiniteInitFinished)
+                        bool isDefiniteInitFinished,
+                        bool TreatAddressToPointerAsInout)
       : Module(TheMemory.MemoryInst->getModule()),
         TheMemory(TheMemory), Uses(Uses),
         FailableInits(FailableInits), Releases(Releases),
-        isDefiniteInitFinished(isDefiniteInitFinished) {
+        isDefiniteInitFinished(isDefiniteInitFinished),
+        TreatAddressToPointerAsInout(TreatAddressToPointerAsInout) {
     }
 
     /// This is the main entry point for the use walker.  It collects uses from
@@ -765,7 +776,7 @@ void ElementUseCollector::collectUses(SILValue Pointer, unsigned BaseEltNo) {
       llvm_unreachable("bad parameter convention");
     }
 
-    if (isa<AddressToPointerInst>(User)) {
+    if (isa<AddressToPointerInst>(User) && TreatAddressToPointerAsInout) {
       // address_to_pointer is a mutable escape, which we model as an inout use.
       addElementUses(BaseEltNo, PointeeType, User, DIUseKind::InOutUse);
       continue;
@@ -1496,7 +1507,9 @@ void swift::collectDIElementUsesFrom(const DIMemoryObjectInfo &MemoryInfo,
                                      SmallVectorImpl<DIMemoryUse> &Uses,
                                      SmallVectorImpl<TermInst*> &FailableInits,
                                      SmallVectorImpl<SILInstruction*> &Releases,
-                                     bool isDIFinished) {
-  ElementUseCollector(MemoryInfo, Uses, FailableInits, Releases, isDIFinished)
+                                     bool isDIFinished,
+                                     bool TreatAddressToPointerAsInout) {
+  ElementUseCollector(MemoryInfo, Uses, FailableInits, Releases, isDIFinished,
+                      TreatAddressToPointerAsInout)
       .collectFrom();
 }

lib/SILOptimizer/Mandatory/DIMemoryUseCollector.h

@@ -288,7 +288,8 @@ void collectDIElementUsesFrom(const DIMemoryObjectInfo &MemoryInfo,
                               SmallVectorImpl<DIMemoryUse> &Uses,
                               SmallVectorImpl<TermInst*> &FailableInits,
                               SmallVectorImpl<SILInstruction*> &Releases,
-                              bool isDefiniteInitFinished);
+                              bool isDefiniteInitFinished,
+                              bool TreatAddressToPointerAsInout);
 
 } // end namespace swift
 

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

@@ -2615,7 +2615,8 @@ static bool processMemoryObject(SILInstruction *I) {
   SmallVector<SILInstruction*, 4> Releases;
 
   // Walk the use list of the pointer, collecting them into the Uses array.
-  collectDIElementUsesFrom(MemInfo, Uses, FailableInits, Releases, false);
+  collectDIElementUsesFrom(MemInfo, Uses, FailableInits, Releases, false,
+                           /*TreatAddressToPointerAsInout*/ true);
 
   LifetimeChecker(MemInfo, Uses, FailableInits, Releases).doIt();
   return true;

lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp

@@ -982,7 +982,8 @@ static bool optimizeMemoryAllocations(SILFunction &Fn) {
       SmallVector<SILInstruction*, 4> Releases;
 
       // Walk the use list of the pointer, collecting them.
-      collectDIElementUsesFrom(MemInfo, Uses, FailableInits, Releases, true);
+      collectDIElementUsesFrom(MemInfo, Uses, FailableInits, Releases, true,
+                               /*TreatAddressToPointerAsInout*/ false);
 
       Changed |= AllocOptimize(Inst, Uses, Releases).doIt();
 

test/SILOptimizer/predictable_memopt.sil

@@ -387,3 +387,32 @@ entry(%x : $Int):
   return %e : $IndirectCase
 }
 
+sil @write_to_bool : $@convention(c) (UnsafeMutablePointer<Bool>) -> Int32
+
+// CHECK-LABEL: sil @escaping_address
+sil @escaping_address : $@convention(thin) () -> Bool {
+bb0:
+  // CHECK: [[A:%[0-9]+]] = alloc_stack
+  %a = alloc_stack $Bool
+  %f = function_ref @write_to_bool : $@convention(c) (UnsafeMutablePointer<Bool>) -> Int32
+  %a2p = address_to_pointer %a : $*Bool to $Builtin.RawPointer
+  %ump = struct $UnsafeMutablePointer<Bool> (%a2p : $Builtin.RawPointer)
+
+  %0 = integer_literal $Builtin.Int1, 0
+  %b0 = struct $Bool (%0 : $Builtin.Int1)
+  // CHECK: [[BV:%[0-9]+]] = struct_element_addr [[A]]
+  %bv = struct_element_addr %a : $*Bool, #Bool._value
+  store %b0 to %a : $*Bool
+
+  // CHECK: apply
+  %ap = apply %f(%ump) : $@convention(c) (UnsafeMutablePointer<Bool>) -> Int32
+
+  // CHECK: [[L:%[0-9]+]] = load [[BV]]
+  %l = load %bv : $*Builtin.Int1
+  // CHECK: [[R:%[0-9]+]] = struct $Bool ([[L]]
+  %r = struct $Bool (%l : $Builtin.Int1)
+  dealloc_stack %a : $*Bool
+  // CHECK: return [[R]]
+  return %r : $Bool
+}
+