From cea7da8d26dc5f423491c7ab4eb2821fd88fcbbd Mon Sep 17 00:00:00 2001
From: Erik Eckstein <eeckstein@apple.com>
Date: Thu, 14 Nov 2024 18:25:11 +0100
Subject: [PATCH] RedundantLoadElimination: correctly handle end_borrow
 instructions

Scope-ending instructions, like `end_borrow` are only irrelevant for RLE if the preceding load is not changed.
If it is changed from `load [copy]` -> `load [take]` the memory effects of those scope-ending instructions prevent that the `load [take]` will illegally mutate memory which is protected from mutation by the scope.

Fixes a memory verifier crash
rdar://139824805
---
 .../RedundantLoadElimination.swift            | 11 ++++--
 .../SILOptimizer/redundant_load_elim_ossa.sil | 39 +++++++++++++++++++
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/RedundantLoadElimination.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/RedundantLoadElimination.swift
index 7bcb77af12773..ea72541f30eb5 100644
--- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/RedundantLoadElimination.swift
+++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/RedundantLoadElimination.swift
@@ -480,9 +480,14 @@ private struct InstructionScanner {
 
   private mutating func visit(instruction: Instruction) -> ScanResult {
     switch instruction {
-    case is FixLifetimeInst, is EndAccessInst, is BeginBorrowInst, is EndBorrowInst:
-      return .transparent
-
+    case is FixLifetimeInst, is EndAccessInst, is EndBorrowInst:
+      // Those scope-ending instructions are only irrelevant if the preceding load is not changed.
+      // If it is changed from `load [copy]` -> `load [take]` the memory effects of those scope-ending
+      // instructions prevent that the `load [take]` will illegally mutate memory which is protected
+      // from mutation by the scope.
+      if load.loadOwnership != .take {
+        return .transparent
+      }
     case let precedingLoad as LoadInst:
       if precedingLoad == load {
         // We need to stop the data flow analysis when we visit the original load again.
diff --git a/test/SILOptimizer/redundant_load_elim_ossa.sil b/test/SILOptimizer/redundant_load_elim_ossa.sil
index 7e4df04a2d4c9..351d53a42b4c1 100644
--- a/test/SILOptimizer/redundant_load_elim_ossa.sil
+++ b/test/SILOptimizer/redundant_load_elim_ossa.sil
@@ -139,6 +139,7 @@ sil @use_2_64 : $@convention(thin) (Builtin.Int64, Builtin.Int64) -> ()
 sil @use_a : $@convention(thin) (A) -> ()
 sil @use_twofield : $@convention(thin) (TwoField) -> ()
 sil @init_twofield : $@convention(thin) (@thin TwoField.Type) -> TwoField
+sil @consumeB : $@convention(thin) (@owned B) -> ()
 
 // We have a bug in the old projection code which this test case exposes.
 // Make sure its handled properly in the new projection.
@@ -1492,3 +1493,41 @@ bb0:
   dealloc_stack %1 : $*Optional<B>
   return %3 : $Optional<B>
 }
+
+// CHECK-LABEL: sil [ossa] @dont_take_in_borrow_scope :
+// CHECK:         load [copy]
+// CHECK:         end_borrow
+// CHECK:         load [take]
+// CHECK-LABEL: } // end sil function 'dont_take_in_borrow_scope'
+sil [ossa] @dont_take_in_borrow_scope : $@convention(thin) (@in B) -> () {
+bb0(%0 : $*B):
+  %1 = load_borrow %0 : $*B
+  %2 = load [copy] %0 : $*B
+  %3 = function_ref @consumeB : $@convention(thin) (@owned B) -> ()
+  %4 = apply %3(%2) : $@convention(thin) (@owned B) -> ()
+  end_borrow %1 : $B
+  %6 = load [take] %0 : $*B
+  %7 = apply %3(%6) : $@convention(thin) (@owned B) -> ()
+  %8 = tuple ()
+  return %8 : $()
+}
+
+// CHECK-LABEL: sil [ossa] @copy_in_borrow_scope :
+// CHECK:         load [copy]
+// CHECK:         copy_value
+// CHECK:         end_borrow
+// CHECK-NOT:     load
+// CHECK-LABEL: } // end sil function 'copy_in_borrow_scope'
+sil [ossa] @copy_in_borrow_scope : $@convention(thin) (@inout B) -> () {
+bb0(%0 : $*B):
+  %1 = load_borrow %0 : $*B
+  %2 = load [copy] %0 : $*B
+  %3 = function_ref @consumeB : $@convention(thin) (@owned B) -> ()
+  %4 = apply %3(%2) : $@convention(thin) (@owned B) -> ()
+  end_borrow %1 : $B
+  %6 = load [copy] %0 : $*B
+  %7 = apply %3(%6) : $@convention(thin) (@owned B) -> ()
+  %8 = tuple ()
+  return %8 : $()
+}
+