Loosen a CopyForwarding assert: store instead of deinit. (#32641)

CopyForwarding attempts to enforce "normal" SIL address patterns using
asserts. This isn't a good strategy because it results in strange
crash diagnostics in release builds. Eventually, we should replace
this logic with a SIL address lifetime utility based on OSSA form and
enforced in the verifier.

Loosen one of these restrictions where we assume that a value
initialized with "copy_addr [initialization]" will be properly
destroyed. This assumption is violated when lowering
.int_fma_FPIEEE32, which knows that the type is trivial, so avoids
deinitialization.

The original SIL looks like this:

  copy_addr %src to [initialization] %dest : $*Float
  %fma = builtin "int_fma_FPIEEE32"(% : $Builtin.FPIEEE32, % : $Builtin.FPIEEE32, % : $Builtin.FPIEEE32) : $Builtin.FPIEEE32
  %result = struct $Float (%fma : $Builtin.FPIEEE32)
  store %result to %dest : $*Float

Fixes rdar://64671864.

Author: atrick

lib/SILOptimizer/Transforms/CopyForwarding.cpp

@@ -322,6 +322,9 @@ namespace {
 /// This returns false and sets Oper to a valid operand if the instruction is a
 /// projection of the value at the given address. The assumption is that we
 /// cannot deinitialize memory via projections.
+///
+/// This returns true with Oper == nullptr for trivial stores (without a proper
+/// deinit).
 class AnalyzeForwardUse
     : public SILInstructionVisitor<AnalyzeForwardUse, bool> {
 public:
@@ -352,7 +355,10 @@ class AnalyzeForwardUse
     return true;
   }
   bool visitStoreInst(StoreInst *Store) {
-    llvm_unreachable("illegal reinitialization or store of an address");
+    // Trivial values may be stored prior to the next deinit. A store is an
+    // implicit "deinit" with no operand to replace.
+    assert(Store->getOperand(0)->getType().isTrivial(*Store->getFunction()));
+    return true;
   }
   bool visitDestroyAddrInst(DestroyAddrInst *UserInst) {
     Oper = &UserInst->getOperandRef();
@@ -1011,15 +1017,17 @@ bool CopyForwarding::forwardPropagateCopy() {
       continue;
 
     AnalyzeForwardUse AnalyzeUse(CopyDest);
-    bool seenDeinit = AnalyzeUse.visit(UserInst);
-    // If this use cannot be analyzed, then abort.
+    bool seenDeinitOrStore = AnalyzeUse.visit(UserInst);
+    if (AnalyzeUse.Oper)
+      ValueUses.push_back(AnalyzeUse.Oper);
+
+    // If this is a deinit or store, we're done searching.
+    if (seenDeinitOrStore)
+      break;
+
+    // If this non-deinit instruction wasn't fully analyzed, bail-out.
     if (!AnalyzeUse.Oper)
       return false;
-    // Otherwise record the operand.
-    ValueUses.push_back(AnalyzeUse.Oper);
-    // If this is a deinit, we're done searching.
-    if (seenDeinit)
-      break;
   }
   if (SI == SE)
     return false;

test/SILOptimizer/copyforward.sil

@@ -915,3 +915,43 @@ bb1:
   %v = tuple ()
   return %v : $()
 }
+
+// Test an "illegal" reinitialization of a stack location. This
+// happens because lowering .int_fma_FPIEEE32 knows that the type is
+// trivial, so avoids deinitialization. rdar://64671864.
+//
+// Note: we have given up on enforcing known SIL-patterns in
+// CopyForwarding. Instead, we just remove restructions whenever we see
+// unexpected patterns. Eventually, we will replace it with an OSSA
+// pass and enforce all assumptions about SIL patterns in the verifier.
+
+// CHECK-LABEL: sil @testCPF : $@convention(thin) (Float) -> @out Float {
+// CHECK: bb0(%0 : $*Float, %1 : $Float):
+// CHECK:   [[TMP1:%.*]] = alloc_stack $Float
+// CHECK:   store %1 to [[TMP1]] : $*Float
+// CHECK:   [[TMP2:%.*]] = alloc_stack $Float
+// CHECK-NOT: copy_addr
+// CHECK:   [[FMA:%.*]] = builtin "int_fma_FPIEEE32"
+// CHECK:   [[RESULT:%.*]] = struct $Float ([[FMA]] : $Builtin.FPIEEE32)
+// CHECK:   store [[RESULT]] to %0 : $*Float
+// CHECK-NOT: copy_addr
+// CHECK-NOT: destroy_addr
+// CHECK-LABEL: } // end sil function 'testCPF'
+sil @testCPF : $@convention(thin) (Float) -> @out Float {
+bb0(%0 : $*Float, %1 : $Float):
+  %2 = alloc_stack $Float
+  store %1 to %2 : $*Float
+  %4 = alloc_stack $Float
+  copy_addr %2 to [initialization] %4 : $*Float
+  %6 = struct_extract %1 : $Float, #Float._value
+  %7 = builtin "int_fma_FPIEEE32"(%6 : $Builtin.FPIEEE32, %6 : $Builtin.FPIEEE32, %6 : $Builtin.FPIEEE32) : $Builtin.FPIEEE32
+  %8 = struct $Float (%7 : $Builtin.FPIEEE32)
+  store %8 to %4 : $*Float
+  copy_addr %4 to [initialization] %0 : $*Float
+  destroy_addr %4 : $*Float
+  dealloc_stack %4 : $*Float
+  destroy_addr %2 : $*Float
+  dealloc_stack %2 : $*Float
+  %15 = tuple ()
+  return %15 : $()
+}