Fix SILCombine miscompile for discard on non-copyable types

Fixes rdar://113214179 (Raw layout types don't adhere to discard self)

// swift-frontend ./rawdeinit.swift -enable-experimental-feature RawLayout \
// -enable-builtin-module -emit-sil -o rawdeinit.sil

import Builtin

@_rawLayout(like: T)
public struct Cell<T>: ~Copyable {
  var ptr: UnsafeMutablePointer<T> {
    .init(Builtin.unprotectedAddressOfBorrow(self))
  }

  init(_ initialValue: T) {
    ptr.initialize(to: initialValue)
  }

  deinit {
    ptr.deinitialize(count: 1)
  }

  public consuming func get() -> T {
    let value = ptr.move()
    discard self // SILCombine inserts a destroy of self here
    return value
  }
}

Author: atrick

lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

@@ -371,6 +371,18 @@ struct AllocStackAnalyzer : SILInstructionVisitor<AllocStackAnalyzer> {
     // anything other than the init_existential_addr/open_existential_addr
     // container.
 
+    // There is no interesting scenario where a non-copyable type should have
+    // its allocation eliminated. A destroy_addr cannot be removed because it
+    // may run the struct-deinit, and the lifetime cannot be shortened. A
+    // copy_addr [take] [init] cannot be replaced by a destroy_addr because the
+    // destination may hold a 'discard'ed value, which is never destroyed. This
+    // analysis assumes memory is deinitialized on all paths, which is not the
+    // case for discarded values. Eventually copyable types may also be
+    // discarded; to support that, we will leave a drop_deinit_addr in place.
+    if (ASI->getType().isPureMoveOnly()) {
+      LegalUsers = false;
+      return;
+    }
     for (auto *Op : getNonDebugUses(ASI)) {
       visit(Op->getUser());
 

test/SILOptimizer/sil_combine_moveonly.sil

@@ -24,6 +24,12 @@ enum MaybeFileDescriptor: ~Copyable {
   deinit
 }
 
+struct Wrapper<T>: ~Copyable {
+  var t: T
+}
+
+sil @getWrappedValue : $@convention(thin) <T> (@in_guaranteed Wrapper<T>) -> @out T
+
 // Test that a release_value is not removed for a struct-with-deinit.
 // Doing so would forget the deinit.
 //
@@ -71,3 +77,20 @@ bb0:
   %9 = tuple ()
   return %9 : $()
 }
+
+// Test that a move into a discarded value does not result in a destroy_addr
+//
+// CHECK-LABEL: sil @testNoDeinit : $@convention(method) <T> (@in Wrapper<T>) -> @out T {
+// CHECK-NOT: destroy_addr
+// CHECK-LABEL: } // end sil function 'testNoDeinit'
+sil @testNoDeinit : $@convention(method) <T> (@in Wrapper<T>) -> @out T {
+bb0(%0 : $*T, %1 : $*Wrapper<T>):
+  %2 = function_ref @getWrappedValue : $@convention(thin) <T> (@in_guaranteed Wrapper<T>) -> @out T
+  %3 = apply %2<T>(%0, %1) : $@convention(thin) <τ_0_0> (@in_guaranteed Wrapper<τ_0_0>) -> @out τ_0_0 
+  %4 = alloc_stack $Wrapper<T>
+  copy_addr [take] %1 to [init] %4 : $*Wrapper<T>
+  // discard
+  dealloc_stack %4 : $*Wrapper<T>
+  %9 = tuple ()
+  return %9 : $()
+}