Improve handling of copy_value and destroy_value in (swiftlang#35011)

MemoryBehaviorVisitor

- Also, compute use points for destroy_value
- Cleanup explicit checks for refcount instructions in RLE

Author: meg-gupta

include/swift/SILOptimizer/Analysis/EscapeAnalysis.h

@@ -1158,6 +1158,10 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
   /// Note that if \p RI is a retain-instruction always false is returned.
   bool canEscapeTo(SILValue V, RefCountingInst *RI);
 
+  /// Returns true if the value \p V can escape to the destroy_value instruction
+  /// \p DVI.
+  bool canEscapeTo(SILValue V, DestroyValueInst *DVI);
+
   /// Return true if \p releasedReference deinitialization may release memory
   /// pointed to by \p accessedAddress.
   bool mayReleaseContent(SILValue releasedReference, SILValue accessedAddress);

lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

@@ -870,6 +870,7 @@ void EscapeAnalysis::ConnectionGraph::computeUsePoints() {
 #include "swift/AST/ReferenceStorage.def"
         case SILInstructionKind::StrongReleaseInst:
         case SILInstructionKind::ReleaseValueInst:
+        case SILInstructionKind::DestroyValueInst:
         case SILInstructionKind::ApplyInst:
         case SILInstructionKind::TryApplyInst: {
           /// Actually we only add instructions which may release a reference.
@@ -2589,8 +2590,9 @@ bool EscapeAnalysis::canEscapeToUsePoint(SILValue value,
                                          SILInstruction *usePoint,
                                          ConnectionGraph *conGraph) {
 
-  assert((FullApplySite::isa(usePoint) || isa<RefCountingInst>(usePoint))
-         && "use points are only created for calls and refcount instructions");
+  assert((FullApplySite::isa(usePoint) || isa<RefCountingInst>(usePoint) ||
+          isa<DestroyValueInst>(usePoint)) &&
+         "use points are only created for calls and refcount instructions");
 
   CGNode *node = conGraph->getValueContent(value);
   if (!node)
@@ -2649,6 +2651,14 @@ bool EscapeAnalysis::canEscapeTo(SILValue V, RefCountingInst *RI) {
   return canEscapeToUsePoint(V, RI, ConGraph);
 }
 
+bool EscapeAnalysis::canEscapeTo(SILValue V, DestroyValueInst *DVI) {
+  // If it's not uniquely identified we don't know anything about the value.
+  if (!isUniquelyIdentified(V))
+    return true;
+  auto *ConGraph = getConnectionGraph(DVI->getFunction());
+  return canEscapeToUsePoint(V, DVI, ConGraph);
+}
+
 /// Utility to get the function which contains both values \p V1 and \p V2.
 static SILFunction *getCommonFunction(SILValue V1, SILValue V2) {
   SILBasicBlock *BB1 = V1->getParentBlock();

lib/SILOptimizer/Analysis/MemoryBehavior.cpp

@@ -182,6 +182,7 @@ class MemoryBehaviorVisitor
   MemBehavior visitBuiltinInst(BuiltinInst *BI);
   MemBehavior visitStrongReleaseInst(StrongReleaseInst *BI);
   MemBehavior visitReleaseValueInst(ReleaseValueInst *BI);
+  MemBehavior visitDestroyValueInst(DestroyValueInst *DVI);
   MemBehavior visitSetDeallocatingInst(SetDeallocatingInst *BI);
   MemBehavior visitBeginCOWMutationInst(BeginCOWMutationInst *BCMI);
 #define ALWAYS_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE(Name, ...) \
@@ -225,6 +226,7 @@ class MemoryBehaviorVisitor
   }
   REFCOUNTINC_MEMBEHAVIOR_INST(StrongRetainInst)
   REFCOUNTINC_MEMBEHAVIOR_INST(RetainValueInst)
+  REFCOUNTINC_MEMBEHAVIOR_INST(CopyValueInst)
 #define UNCHECKED_REF_STORAGE(Name, ...)                                       \
   REFCOUNTINC_MEMBEHAVIOR_INST(Name##RetainValueInst)                          \
   REFCOUNTINC_MEMBEHAVIOR_INST(StrongCopy##Name##ValueInst)
@@ -490,6 +492,13 @@ MemBehavior MemoryBehaviorVisitor::visitReleaseValueInst(ReleaseValueInst *SI) {
   return MemBehavior::MayHaveSideEffects;
 }
 
+MemBehavior
+MemoryBehaviorVisitor::visitDestroyValueInst(DestroyValueInst *DVI) {
+  if (!EA->canEscapeTo(V, DVI))
+    return MemBehavior::None;
+  return MemBehavior::MayHaveSideEffects;
+}
+
 MemBehavior MemoryBehaviorVisitor::visitSetDeallocatingInst(SetDeallocatingInst *SDI) {
   return MemBehavior::None;
 }

lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp

@@ -146,15 +146,6 @@ static bool inline isPerformingRLE(RLEKind Kind) {
 /// general sense but are inert from a load store perspective.
 static bool isRLEInertInstruction(SILInstruction *Inst) {
   switch (Inst->getKind()) {
-#define UNCHECKED_REF_STORAGE(Name, ...)                                       \
-  case SILInstructionKind::StrongCopy##Name##ValueInst:
-#define ALWAYS_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE(Name, ...)            \
-  case SILInstructionKind::Name##RetainInst:                                   \
-  case SILInstructionKind::StrongRetain##Name##Inst:                           \
-  case SILInstructionKind::StrongCopy##Name##ValueInst:
-#include "swift/AST/ReferenceStorage.def"
-  case SILInstructionKind::StrongRetainInst:
-  case SILInstructionKind::RetainValueInst:
   case SILInstructionKind::DeallocStackInst:
   case SILInstructionKind::CondFailInst:
   case SILInstructionKind::IsEscapingClosureInst:

lib/SILOptimizer/UtilityPasses/EscapeAnalysisDumper.cpp

@@ -85,6 +85,14 @@ class EscapeAnalysisDumper : public SILModuleTransform {
                              << " to " << ii;
               }
             }
+            if (DestroyValueInst *dvi = dyn_cast<DestroyValueInst>(&ii)) {
+              for (unsigned i = 0, e = Values.size(); i != e; ++i) {
+                SILValue val = Values[i];
+                bool escape = EA->canEscapeTo(val, dvi);
+                llvm::outs() << (escape ? "May" : "No") << "Escape: " << val
+                             << " to " << ii;
+              }
+            }
           }
         }
       }

test/SILOptimizer/escape_analysis.sil

@@ -1962,3 +1962,22 @@ bb0(%0 : $Z):
   unreachable
 }
 
+// CHECK-LABEL: CG of test_destroy_value
+// CHECK:   Arg %0 Esc: A, Succ: (%1)
+// CHECK:   Con [ref] %1 Esc: A, Succ: (%1.1)
+// CHECK:   Con [int] %1.1 Esc: G, Succ: (%1.2)
+// CHECK:   Con [ref] %1.2 Esc: G, Succ:
+// CHECK-LABEL: End
+// CHECK: NoEscape: %0 = argument of bb0 : $*X
+// CHECK:  to   destroy_value %1 : $X
+// CHECK: MayEscape:   %1 = load [copy] %0 : $*X
+// CHECK:  to   destroy_value %1 : $X
+sil [ossa] @test_destroy_value : $@convention(thin) (@in X) -> () {
+bb0 (%0 : $*X):
+  %1 = load [copy] %0 : $*X
+  destroy_value %1 : $X
+  destroy_addr %0 : $*X
+  %2 = tuple()
+  return %2 : $()
+}
+

test/SILOptimizer/mem-behavior-all.sil

@@ -244,3 +244,45 @@ bb0(%0 : $*C, %1 : $*C, %2: $*C):
   return %6 : $()
 }
 
+// CHECK-LABEL: @test_destroy_value
+// CHECK: PAIR #0.
+// CHECK-NEXT:     %1 = load [copy] %0 : $*C
+// CHECK-NEXT:   %0 = argument of bb0 : $*C
+// CHECK-NEXT:   r=1,w=0
+// CHECK: PAIR #2.
+// CHECK-NEXT:     destroy_value %1 : $C
+// CHECK-NEXT:   %0 = argument of bb0 : $*C
+// CHECK-NEXT:   r=0,w=0
+// CHECK: PAIR #3.
+// CHECK-NEXT:     destroy_value %1 : $C
+// CHECK-NEXT:     %1 = load [copy] %0 : $*C
+// CHECK-NEXT:   r=1,w=1
+sil [ossa] @test_destroy_value : $@convention(thin) (@in C) -> () {
+bb0 (%0 : $*C):
+  %1 = load [copy] %0 : $*C
+  destroy_value %1 : $C
+  destroy_addr %0 : $*C
+  %2 = tuple()
+  return %2 : $()
+}
+
+// CHECK-LABEL: @test_copy_value
+// CHECK: PAIR #3.
+// CHECK-NEXT:     %2 = copy_value %1 : $C
+// CHECK-NEXT:   %0 = argument of bb0 : $*C
+// CHECK-NEXT:   r=0,w=0
+// CHECK: PAIR #4.
+// CHECK-NEXT:     %2 = copy_value %1 : $C
+// CHECK-NEXT:     %1 = load [copy] %0 : $*C
+// CHECK-NEXT:   r=0,w=0
+sil [ossa] @test_copy_value : $@convention(thin) (@in C) -> () {
+bb0 (%0 : $*C):
+  %1 = load [copy] %0 : $*C
+  %2 = copy_value %1 : $C
+  destroy_value %1 : $C
+  destroy_value %2 : $C
+  destroy_addr %0 : $*C
+  %3 = tuple()
+  return %3 : $()
+}
+