LoopInvariantCodeMotion: don't hoist loads and stores if the memory location is not initialized at loop exits.

If the memory is not initialized at all exits, it would be wrong to insert stores at exit blocks.

Author: eeckstein

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift

@@ -693,6 +693,12 @@ private extension MovableInstructions {
     accessPath: AccessPath,
     context: FunctionPassContext
   ) -> Bool {
+
+    // If the memory is not initialized at all exits, it would be wrong to insert stores at exit blocks.
+    guard memoryIsInitializedAtAllExits(of: loop, accessPath: accessPath, context) else {
+      return false
+    }
+
     // Initially load the value in the loop pre header.
     let builder = Builder(before: loop.preheader!.terminator, context)
     var firstStore: StoreInst?
@@ -831,6 +837,40 @@ private extension MovableInstructions {
 
     return changed
   }
+
+  func memoryIsInitializedAtAllExits(of loop: Loop, accessPath: AccessPath, _ context: FunctionPassContext) -> Bool {
+
+    // Perform a simple dataflow analysis which checks if there is a path from a `load [take]`
+    // (= the only kind of instruction which can de-initialize the memory) to a loop exit without
+    // a `store` in between.
+
+    var stores = InstructionSet(context)
+    defer { stores.deinitialize() }
+    for case let store as StoreInst in loadsAndStores where store.storesTo(accessPath) {
+      stores.insert(store)
+    }
+
+    var exitInsts = InstructionSet(context)
+    defer { exitInsts.deinitialize() }
+    exitInsts.insert(contentsOf: loop.exitBlocks.lazy.map { $0.instructions.first! })
+
+    var worklist = InstructionWorklist(context)
+    defer { worklist.deinitialize() }
+    for case let load as LoadInst in loadsAndStores where load.loadOwnership == .take && load.loadsFrom(accessPath) {
+      worklist.pushIfNotVisited(load)
+    }
+
+    while let inst = worklist.pop() {
+      if stores.contains(inst) {
+        continue
+      }
+      if exitInsts.contains(inst) {
+        return false
+      }
+      worklist.pushSuccessors(of: inst)
+    }
+    return true
+  }
 }
 
 private extension Instruction {

SwiftCompilerSources/Sources/SIL/DataStructures/Worklist.swift

@@ -77,7 +77,7 @@ public typealias ValueWorklist = Worklist<ValueSet>
 public typealias OperandWorklist = Worklist<OperandSet>
 
 extension InstructionWorklist {
-  public mutating func pushPredecessors(of inst: Instruction, ignoring ignoreInst: Instruction) {
+  public mutating func pushPredecessors(of inst: Instruction, ignoring ignoreInst: Instruction? = nil) {
     if let prev = inst.previous {
       if prev != ignoreInst {
         pushIfNotVisited(prev)
@@ -92,7 +92,7 @@ extension InstructionWorklist {
     }
   }
 
-  public mutating func pushSuccessors(of inst: Instruction, ignoring ignoreInst: Instruction) {
+  public mutating func pushSuccessors(of inst: Instruction, ignoring ignoreInst: Instruction? = nil) {
     if let succ = inst.next {
       if succ != ignoreInst {
         pushIfNotVisited(succ)

test/SILOptimizer/licm.sil

@@ -1927,3 +1927,81 @@ bb3:
   %13 = tuple ()
   return %13
 }
+
+// Just check that LICM doesn't produce invalid SIL.
+// CHECK-LABEL: sil [ossa] @uninitialized_at_entry_and_exit :
+// CHECK-LABEL: } // end sil function 'uninitialized_at_entry_and_exit'
+sil [ossa] @uninitialized_at_entry_and_exit : $@convention(thin) (@owned S) -> @out S {
+bb0(%0 : $*S, %1 : @owned $S):
+  br bb1
+
+bb1:
+  %5 = copy_value %1
+  store %5 to [init] %0
+  %7 = load [take] %0
+  destroy_value %7
+  cond_br undef, bb2, bb3
+
+bb2:
+  br bb1
+
+bb3:
+  store %1 to [init] %0
+  %r = tuple ()
+  return %r
+}
+
+// CHECK-LABEL: sil [ossa] @uninitialized_at_entry :
+// CHECK:       bb1:
+// CHECK-NEXT:    [[C:%.*]] = copy_value %1
+// CHECK-NEXT:    cond_br
+// CHECK:       bb2:
+// CHECK-NEXT:    destroy_value [[C]]
+// CHECK:       bb3:
+// CHECK-NEXT:    store [[C]] to [init] %0
+// CHECK-LABEL: } // end sil function 'uninitialized_at_entry'
+sil [ossa] @uninitialized_at_entry : $@convention(thin) (@owned S) -> @out S {
+bb0(%0 : $*S, %1 : @owned $S):
+  br bb1
+
+bb1:
+  %5 = copy_value %1
+  store %5 to [init] %0
+  cond_br undef, bb2, bb3
+
+bb2:
+  %7 = load [take] %0
+  destroy_value %7
+  br bb1
+
+bb3:
+  destroy_value %1
+  %r = tuple ()
+  return %r
+}
+
+// Just check that LICM doesn't produce invalid SIL.
+// CHECK-LABEL: sil [ossa] @uninitialized_at_exit :
+// CHECK-LABEL: } // end sil function 'uninitialized_at_exit'
+sil [ossa] @uninitialized_at_exit : $@convention(thin) (@in S) -> () {
+bb0(%0 : $*S):
+  br bb1
+
+bb1:
+  %4 = load [take] %0
+  %5 = move_value %4
+  store %5 to [init] %0
+  %6 = load [take] %0
+  cond_br undef, bb2, bb3
+
+bb2:
+  %8 = move_value %6
+  store %8 to [init] %0
+  br bb1
+
+bb3:
+  destroy_value %6
+  %r = tuple ()
+  return %r
+}
+