Merge pull request swiftlang#1824 from trentxintong/RLE

Make sure non-epilogue releases do not kill redundant loads

Author: trentxintong

include/swift/SILOptimizer/Utils/Local.h

@@ -14,6 +14,7 @@
 #define SWIFT_SILOPTIMIZER_UTILS_LOCAL_H
 
 #include "swift/Basic/ArrayRefView.h"
+#include "swift/SILOptimizer/Analysis/ARCAnalysis.h"
 #include "swift/SILOptimizer/Analysis/SimplifyInstruction.h"
 #include "swift/SIL/SILInstruction.h"
 #include "swift/SIL/SILBuilder.h"
@@ -69,9 +70,10 @@ recursivelyDeleteTriviallyDeadInstructions(
 /// This routine only examines the state of the instruction at hand.
 bool isInstructionTriviallyDead(SILInstruction *I);
 
-/// \brief Return true if this is a release instruction and the released value
-/// is a part of a guaranteed parameter, false otherwise.
-bool isGuaranteedParamRelease(SILInstruction *I); 
+/// \brief Return true if this is a release instruction thats not going to
+/// free the object.
+bool isIntermediateRelease(SILInstruction *I,
+                           ConsumedArgToEpilogueReleaseMatcher &ERM); 
 
 /// \brief Recursively erase all of the uses of the instruction (but not the
 /// instruction itself) and delete instructions that will become trivially

lib/SILOptimizer/Transforms/DeadStoreElimination.cpp

@@ -322,6 +322,9 @@ class DSEContext {
   /// Type Expansion Analysis.
   TypeExpansionAnalysis *TE;
 
+  /// The epilogue release matcher we are using.
+  ConsumedArgToEpilogueReleaseMatcher& ERM;
+
   /// Allocator.
   llvm::BumpPtrAllocator BPA;
 
@@ -428,8 +431,9 @@ class DSEContext {
 public:
   /// Constructor.
   DSEContext(SILFunction *F, SILModule *M, SILPassManager *PM,
-             AliasAnalysis *AA, EscapeAnalysis *EA, TypeExpansionAnalysis *TE)
-      : Mod(M), F(F), PM(PM), AA(AA), EA(EA), TE(TE) {}
+             AliasAnalysis *AA, EscapeAnalysis *EA, TypeExpansionAnalysis *TE,
+             ConsumedArgToEpilogueReleaseMatcher &ERM)
+      : Mod(M), F(F), PM(PM), AA(AA), EA(EA), TE(TE), ERM(ERM) {}
 
   /// Entry point for dead store elimination.
   bool run();
@@ -446,6 +450,8 @@ class DSEContext {
   /// Returns the location vault of the current function.
   std::vector<LSLocation> &getLocationVault() { return LocationVault; }
 
+  ConsumedArgToEpilogueReleaseMatcher &getERM() const { return ERM; };
+
   /// Use a set of ad hoc rules to tell whether we should run a pessimistic
   /// one iteration data flow on the function.
   ProcessKind getProcessFunctionKind(unsigned StoreCount);
@@ -1049,7 +1055,7 @@ void DSEContext::processUnknownReadInstForDSE(SILInstruction *I) {
 void DSEContext::processUnknownReadInst(SILInstruction *I, DSEKind Kind) {
   // If this is a release on a guaranteed parameter, it can not call deinit,
   // which might read or write memory.
-  if (isGuaranteedParamRelease(I))
+  if (isIntermediateRelease(I, ERM))
     return;
 
   // Are we building genset and killset.
@@ -1231,13 +1237,17 @@ class DeadStoreElimination : public SILFunctionTransform {
 
   /// The entry point to the transformation.
   void run() override {
+    SILFunction *F = getFunction();
+    DEBUG(llvm::dbgs() << "*** DSE on function: " << F->getName() << " ***\n");
+
     auto *AA = PM->getAnalysis<AliasAnalysis>();
     auto *EA = PM->getAnalysis<EscapeAnalysis>();
     auto *TE = PM->getAnalysis<TypeExpansionAnalysis>();
-    SILFunction *F = getFunction();
-    DEBUG(llvm::dbgs() << "*** DSE on function: " << F->getName() << " ***\n");
+    auto *RCFI = PM->getAnalysis<RCIdentityAnalysis>()->get(F);
+
+    ConsumedArgToEpilogueReleaseMatcher ERM(RCFI, F);
 
-    DSEContext DSE(F, &F->getModule(), PM, AA, EA, TE);
+    DSEContext DSE(F, &F->getModule(), PM, AA, EA, TE, ERM);
     if (DSE.run()) {
       invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
     }

lib/SILOptimizer/Transforms/RedundantLoadElimination.cpp

@@ -77,6 +77,7 @@
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILBuilder.h"
 #include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
+#include "swift/SILOptimizer/Analysis/ARCAnalysis.h"
 #include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
 #include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
 #include "swift/SILOptimizer/Analysis/ValueTracking.h"
@@ -439,6 +440,9 @@ class RLEContext {
   /// order of the given function.
   PostOrderFunctionInfo *PO;
 
+  /// The epilogue release matcher we are using.
+  ConsumedArgToEpilogueReleaseMatcher& ERM;
+
   /// Keeps all the locations for the current function. The BitVector in each
   /// BlockState is then laid on top of it to keep track of which LSLocation
   /// has a downward available value.
@@ -475,7 +479,8 @@ class RLEContext {
 
 public:
   RLEContext(SILFunction *F, SILPassManager *PM, AliasAnalysis *AA,
-             TypeExpansionAnalysis *TE, PostOrderFunctionInfo *PO);
+             TypeExpansionAnalysis *TE, PostOrderFunctionInfo *PO,
+             ConsumedArgToEpilogueReleaseMatcher &ERM);
 
   RLEContext(const RLEContext &) = delete;
   RLEContext(RLEContext &&) = default;
@@ -484,6 +489,8 @@ class RLEContext {
   /// Entry point to redundant load elimination.
   bool run();
 
+  ConsumedArgToEpilogueReleaseMatcher &getERM() const { return ERM; };
+
   /// Use a set of ad hoc rules to tell whether we should run a pessimistic
   /// one iteration data flow on the function.
   ProcessKind getProcessFunctionKind(unsigned LoadCount, unsigned StoreCount);
@@ -961,7 +968,7 @@ void BlockState::processUnknownWriteInst(RLEContext &Ctx, SILInstruction *I,
                                          RLEKind Kind) {
   // If this is a release on a guaranteed parameter, it can not call deinit,
   // which might read or write memory.
-  if (isGuaranteedParamRelease(I))
+  if (isIntermediateRelease(I, Ctx.getERM()))
     return;
 
   // Are we computing the genset and killset ?
@@ -1118,8 +1125,9 @@ getProcessFunctionKind(unsigned LoadCount, unsigned StoreCount) {
 //===----------------------------------------------------------------------===//
 
 RLEContext::RLEContext(SILFunction *F, SILPassManager *PM, AliasAnalysis *AA,
-                       TypeExpansionAnalysis *TE, PostOrderFunctionInfo *PO)
-    : Fn(F), PM(PM), AA(AA), TE(TE), PO(PO) {
+                       TypeExpansionAnalysis *TE, PostOrderFunctionInfo *PO,
+                       ConsumedArgToEpilogueReleaseMatcher &ERM)
+    : Fn(F), PM(PM), AA(AA), TE(TE), PO(PO), ERM(ERM) {
 }
 
 LSLocation &RLEContext::getLocation(const unsigned index) {
@@ -1522,8 +1530,11 @@ class RedundantLoadElimination : public SILFunctionTransform {
     auto *AA = PM->getAnalysis<AliasAnalysis>();
     auto *TE = PM->getAnalysis<TypeExpansionAnalysis>();
     auto *PO = PM->getAnalysis<PostOrderAnalysis>()->get(F);
+    auto *RCFI = PM->getAnalysis<RCIdentityAnalysis>()->get(F);
+
+    ConsumedArgToEpilogueReleaseMatcher ERM(RCFI, F);
 
-    RLEContext RLE(F, PM, AA, TE, PO);
+    RLEContext RLE(F, PM, AA, TE, PO, ERM);
     if (RLE.run()) {
       invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
     }

lib/SILOptimizer/Utils/Local.cpp

@@ -77,17 +77,31 @@ swift::isInstructionTriviallyDead(SILInstruction *I) {
 
 /// \brief Return true if this is a release instruction and the released value
 /// is a part of a guaranteed parameter.
-bool swift::isGuaranteedParamRelease(SILInstruction *I) {
-  if (!isa<StrongReleaseInst>(I) || !isa<ReleaseValueInst>(I))
+bool swift::isIntermediateRelease(SILInstruction *I,
+                                  ConsumedArgToEpilogueReleaseMatcher &ERM) {
+  // Check whether this is a release instruction.
+  if (!isa<StrongReleaseInst>(I) && !isa<ReleaseValueInst>(I))
     return false;
+
   // OK. we have a release instruction.
   // Check whether this is a release on part of a guaranteed function argument.
   SILValue Op = stripValueProjections(I->getOperand(0));
   SILArgument *Arg = dyn_cast<SILArgument>(Op);
-  if (!Arg || !Arg->isFunctionArg() ||
-      !Arg->hasConvention(SILArgumentConvention::Direct_Guaranteed))
+  if (!Arg || !Arg->isFunctionArg())
     return false;
-  return true;
+  
+  // This is a release on a guaranteed parameter. Its not the final release.
+  if (Arg->hasConvention(SILArgumentConvention::Direct_Guaranteed))
+    return true;
+
+  // This is a release on a owned parameter and its not the epilogue relase.
+  // Its not the final release.
+  SILInstruction *Rel = ERM.getSingleReleaseForArgument(Arg);
+  if (Rel && Rel != I)
+    return true;
+
+  // Failed to prove anything.
+  return false;
 }
 
 namespace {

test/SILOptimizer/redundant_load_elim.sil

@@ -1095,3 +1095,36 @@ bb2:
   %9999 = tuple()
   return %9999 : $()
 }
+
+// CHECK-LABEL: sil hidden @redundant_load_over_intermediate_release_with_epilogue_release : $@convention(thin) (@owned AB) -> () {
+// CHECK: [[AD:%.*]] = ref_element_addr
+// CHECK: [[AD2:%.*]] = load [[AD]]
+// CHECK: release_value
+// CHECK-NOT: [[AD3:%.*]] = load [[AD]]
+// CHECK: return
+sil hidden @redundant_load_over_intermediate_release_with_epilogue_release : $@convention(thin) (@owned AB) -> () {
+bb0(%0 : $AB):
+  %1 = ref_element_addr %0 : $AB, #AB.value
+  %2 = load %1 : $*Int
+  release_value %0 : $AB
+  %3 = load %1 : $*Int
+  release_value %0 : $AB
+  %4 = tuple ()
+  return %4 : $()
+}
+
+// CHECK-LABEL: sil hidden @redundant_load_over_intermediate_release_without_epilogue_release : $@convention(thin) (@owned AB) -> () {
+// CHECK: [[AD:%.*]] = ref_element_addr
+// CHECK: [[AD2:%.*]] = load [[AD]]
+// CHECK: [[AD3:%.*]] = load [[AD]]
+// CHECK: return
+sil hidden @redundant_load_over_intermediate_release_without_epilogue_release : $@convention(thin) (@owned AB) -> () {
+bb0(%0 : $AB):
+  %1 = ref_element_addr %0 : $AB, #AB.value
+  %2 = load %1 : $*Int
+  release_value %0 : $AB
+  %3 = load %1 : $*Int
+  retain_value %0 : $AB
+  %4 = tuple ()
+  return %4 : $()
+}