[InstructionDeleter] Delete dead load [take]s.

Previously, `isScopeAffectingInstructionDead` determined that
an otherwise satisfactory `load` was not dead if it was a `load [take]`.
The immediate effect was that dead `load [take]`s were not deleted.  The
downstream effect was that otherwise dead graphs of instructions would
not be deleted.  This was especially a problem for OSLogOptimization
which deletes a great deal of code.

Here, the InstructionDeleter is taught to compensate for the deletion of
such `load [take]` by inserting `destroy_addr`s in their stead.

rdar://117011668

Author: nate-chandler

lib/SILOptimizer/Utils/InstructionDeleter.cpp

@@ -97,11 +97,9 @@ static bool isScopeAffectingInstructionDead(SILInstruction *inst,
         cast<LoadInst>(inst)->getOwnershipQualifier();
     // If the load creates a copy, it is dead, since we know that if at all it
     // is used, it is only in a destroy_value instruction.
-    return (loadOwnershipQual == LoadOwnershipQualifier::Copy ||
+    return (loadOwnershipQual == LoadOwnershipQualifier::Take ||
+            loadOwnershipQual == LoadOwnershipQualifier::Copy ||
             loadOwnershipQual == LoadOwnershipQualifier::Trivial);
-    // TODO: we can handle load [take] but we would have to know that the
-    // operand has been consumed. Note that OperandOwnershipKind map does not
-    // say this for load.
   }
   case SILInstructionKind::PartialApplyInst: {
     bool onlyTrivialArgs = true;
@@ -250,10 +248,19 @@ void InstructionDeleter::deleteWithUses(SILInstruction *inst, bool fixLifetimes,
       if (!operandValue)
         continue;
 
-      if (fixLifetimes && operand.isConsuming()) {
-        SILBuilderWithScope builder(inst);
-        auto *dvi = builder.createDestroyValue(inst->getLoc(), operandValue);
-        getCallbacks().createdNewInst(dvi);
+      if (fixLifetimes) {
+        LoadInst *li = nullptr;
+        if (operand.isConsuming()) {
+          SILBuilderWithScope builder(inst);
+          auto *dvi = builder.createDestroyValue(inst->getLoc(), operandValue);
+          getCallbacks().createdNewInst(dvi);
+        } else if ((li = dyn_cast<LoadInst>(inst)) &&
+                   li->getOwnershipQualifier() ==
+                       LoadOwnershipQualifier::Take) {
+          SILBuilderWithScope builder(inst);
+          auto *dai = builder.createDestroyAddr(inst->getLoc(), operandValue);
+          getCallbacks().createdNewInst(dai);
+        }
       }
       auto *operDef = operandValue->getDefiningInstruction();
       operand.drop();
@@ -419,4 +426,3 @@ void swift::recursivelyDeleteTriviallyDeadInstructions(
     nextInsts.clear();
   }
 }
-

test/SILOptimizer/instruction_deleter.sil

@@ -5,6 +5,10 @@ struct MOS : ~Copyable {}
 sil @getMOS : $() -> (@owned MOS)
 sil @barrier : $() -> ()
 
+class C {}
+
+sil @get : $<T> () -> (@out T)
+
 // CHECK-LABEL: begin running test {{.*}} on dontDeleteDeadMoveOnlyValue
 // CHECK:       Deleting-if-dead {{.*}} move_value
 // CHECK:       deleteIfDead returned 0
@@ -28,3 +32,26 @@ sil [ossa] @dontDeleteDeadMoveOnlyValue : $() -> () {
   %retval = tuple ()
   return %retval : $()
 }
+
+// CHECK-LABEL: begin running test {{.*}} on doDeleteLoadTake
+// CHECK:       Deleting-if-dead   {{%[^,]+}} = load [take]
+// CHECK:       deleteIfDead returned 1
+// CHECK-LABEL: sil [ossa] @doDeleteLoadTake : {{.*}} {
+// CHECK:         [[STACK:%[^,]+]] = alloc_stack $C
+// CHECK:         [[GET:%[^,]+]] = function_ref @get
+// CHECK:         apply [[GET]]<C>([[STACK]])
+// CHECK:         destroy_addr [[STACK]]
+// CHECK:         dealloc_stack [[STACK]]
+// CHECK-LABEL: } // end sil function 'doDeleteLoadTake'
+// CHECK-LABEL: end running test {{.*}} on doDeleteLoadTake
+sil [ossa] @doDeleteLoadTake : $() -> () {
+  %stack = alloc_stack $C
+  %get = function_ref @get : $@convention(thin) <T> () -> (@out T)
+  apply %get<C>(%stack) : $@convention(thin) <T> () -> (@out T)
+  specify_test "deleter-delete-if-dead @instruction"
+  %c = load [take] %stack : $*C
+  destroy_value %c : $C
+  dealloc_stack %stack : $*C
+  %retval = tuple ()
+  return %retval : $()
+}

test/SILOptimizer/predictable_deadalloc_elim_ownership.sil

@@ -156,8 +156,7 @@ bb0(%0 : @owned $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
 // CHECK: bb0([[ARG0:%.*]] : @owned $Builtin.NativeObject, [[ARG1:%.*]] : $*Builtin.NativeObject):
 // CHECK-NOT: alloc_stack
 // CHECK:  destroy_value [[ARG0]]
-// CHECK:  [[ARG1_LOADED:%.*]] = load [take] [[ARG1]]
-// CHECK:  destroy_value [[ARG1_LOADED]]
+// CHECK:  destroy_addr [[ARG1]]
 // CHECK: } // end sil function 'simple_init_take'
 sil [ossa] @simple_init_take : $@convention(thin) (@owned Builtin.NativeObject, @in Builtin.NativeObject) -> () {
 bb0(%0 : @owned $Builtin.NativeObject, %1 : $*Builtin.NativeObject):

validation-test/SILOptimizer/rdar117011668.swift

@@ -0,0 +1,20 @@
+// RUN: %target-swift-frontend -disable-availability-checking -emit-sil -verify %s | %FileCheck %s
+
+// REQUIRES: objc_interop
+
+import Foundation
+import os.log
+
+struct Log {
+  static let attachmentLedgerTopic = Logger(
+    subsystem: "com.xxx.yyy.zzz",
+    category: "ccc")
+}
+
+// CHECK-LABEL: sil @logWriter : {{.*}} {
+// CHECK-NOT: $s2os18OSLogInterpolationV13appendLiteralyySSF
+// CHECK-LABEL: } // end sil function 'logWriter'
+@_silgen_name("logWriter")
+public func logWriter(_ attachmentID: UUID) {
+  Log.attachmentLedgerTopic.info("aaa: \(attachmentID)")
+}