Consistenly use RCIdentity to get the RCRoot of the operand.

trentxintong · trentxintong · commit 8bb673895a21 · 2016-02-23T16:47:19.000-08:00
We were using a stripCast in some places and getRCIdentityRoot in others.
stripCasts is not identical to getRCIdentityRoot.

In particular, it does not look through struct_extract, tuple_extract,
unchecked_enum_data.

Created a struct and tuple test cases for make sure things are optimized
as they should be.

We have test case for unchecked_enum_data before.
diff --git a/lib/SILOptimizer/ARC/RCStateTransitionVisitors.cpp b/lib/SILOptimizer/ARC/RCStateTransitionVisitors.cpp
@@ -68,7 +68,7 @@ BottomUpDataflowRCStateVisitor<ARCState>::visitStrongDecrement(ValueBase *V) {
     return DataflowResult(Op);
 
   BottomUpRefCountState &State = DataflowState.getBottomUpRefCountState(Op);
-  bool NestingDetected = State.initWithMutatorInst(SetFactory.get(I));
+  bool NestingDetected = State.initWithMutatorInst(SetFactory.get(I), RCFI);
 
   // If we are running with 'frozen' owned arg releases, check if we have a
   // frozen use in the side table. If so, this release must be known safe.
@@ -203,7 +203,7 @@ TopDownDataflowRCStateVisitor<ARCState>::visitStrongIncrement(ValueBase *V) {
   // count state and continue...
   SILValue Op = RCFI->getRCIdentityRoot(I->getOperand(0));
   auto &State = DataflowState.getTopDownRefCountState(Op);
-  bool NestingDetected = State.initWithMutatorInst(SetFactory.get(I));
+  bool NestingDetected = State.initWithMutatorInst(SetFactory.get(I), RCFI);
 
   DEBUG(llvm::dbgs() << "    REF COUNT INCREMENT! Known Safe: "
                      << (State.isKnownSafe() ? "yes" : "no") << "\n");
diff --git a/lib/SILOptimizer/ARC/RefCountState.cpp b/lib/SILOptimizer/ARC/RefCountState.cpp
@@ -81,14 +81,15 @@ MergeTopDownLatticeStates(TopDownRefCountState::LatticeState L1,
 /// Initializes/reinitialized the state for I. If we reinitialize we return
 /// true.
 bool BottomUpRefCountState::initWithMutatorInst(
-    ImmutablePointerSet<SILInstruction> *I) {
+    ImmutablePointerSet<SILInstruction> *I,
+    RCIdentityFunctionInfo *RCFI) {
   assert(I->size() == 1);
   SILInstruction *Inst = *I->begin();
   assert((isa<StrongReleaseInst>(Inst) || isa<ReleaseValueInst>(Inst)) &&
          "strong_release and release_value are only supported.");
   (void) Inst;
 
-  bool NestingDetected = SuperTy::initWithMutatorInst(I);
+  bool NestingDetected = SuperTy::initWithMutatorInst(I, RCFI);
 
   // If we know that there is another decrement on the same pointer that has
   // not been matched up to an increment, then the pointer must have a
@@ -545,14 +546,15 @@ void BottomUpRefCountState::updateForPredTerminators(
 /// Initializes/reinitialized the state for I. If we reinitialize we return
 /// true.
 bool TopDownRefCountState::initWithMutatorInst(
-    ImmutablePointerSet<SILInstruction> *I) {
+    ImmutablePointerSet<SILInstruction> *I,
+    RCIdentityFunctionInfo *RCFI) {
   assert(I->size() == 1);
   SILInstruction *Inst = *I->begin();
   (void)Inst;
   assert((isa<StrongRetainInst>(Inst) || isa<RetainValueInst>(Inst)) &&
          "strong_retain and retain_value are only supported.");
 
-  bool NestingDetected = SuperTy::initWithMutatorInst(I);
+  bool NestingDetected = SuperTy::initWithMutatorInst(I, RCFI);
 
   // This retain is known safe if the operand we are tracking was already
   // known incremented previously. This occurs when you have nested
diff --git a/lib/SILOptimizer/ARC/RefCountState.h b/lib/SILOptimizer/ARC/RefCountState.h
@@ -22,6 +22,7 @@
 #include "swift/SIL/SILBasicBlock.h"
 #include "swift/SIL/InstructionUtils.h"
 #include "swift/SILOptimizer/Analysis/ARCAnalysis.h"
+#include "swift/SILOptimizer/Analysis/RCIdentityAnalysis.h"
 #include <algorithm>
 
 namespace swift {
@@ -73,7 +74,8 @@ class RefCountState {
 
   /// Initializes/reinitialized the state for I. If we reinitialize we return
   /// true.
-  bool initWithMutatorInst(ImmutablePointerSet<SILInstruction> *I) {
+  bool initWithMutatorInst(ImmutablePointerSet<SILInstruction> *I,
+                           RCIdentityFunctionInfo *RCFI) {
     assert(I->size() == 1);
 
     // Are we already tracking a ref count modification?
@@ -86,7 +88,7 @@ class RefCountState {
     KnownSafe = false;
 
     // Initialize value.
-    RCRoot = stripCasts((*I->begin())->getOperand(0));
+    RCRoot = RCFI->getRCIdentityRoot((*I->begin())->getOperand(0));
 
     // Clear our insertion point list.
     InsertPts = ImmutablePointerSetFactory<SILInstruction>::getEmptySet();
@@ -204,7 +206,8 @@ class BottomUpRefCountState : public RefCountState {
 
   /// Initializes/reinitialized the state for I. If we reinitialize we return
   /// true.
-  bool initWithMutatorInst(ImmutablePointerSet<SILInstruction> *I);
+  bool initWithMutatorInst(ImmutablePointerSet<SILInstruction> *I,
+                           RCIdentityFunctionInfo *RCFI);
 
   /// Update this reference count's state given the instruction \p I. \p
   /// InsertPt is the point furthest up the CFG where we can move the currently
@@ -349,7 +352,8 @@ class TopDownRefCountState : public RefCountState {
 
   /// Initializes/reinitialized the state for I. If we reinitialize we return
   /// true.
-  bool initWithMutatorInst(ImmutablePointerSet<SILInstruction> *I);
+  bool initWithMutatorInst(ImmutablePointerSet<SILInstruction> *I,
+                           RCIdentityFunctionInfo *RCFI);
 
   /// Initialize the state given the consumed argument Arg.
   void initWithArg(SILArgument *Arg);
diff --git a/test/SILOptimizer/arcsequenceopts.sil b/test/SILOptimizer/arcsequenceopts.sil
@@ -12,6 +12,10 @@ sil @user : $@convention(thin) (@box Builtin.Int32) -> ()
 
 sil @owned_user : $@convention(thin) (@owned @box Builtin.Int32) -> ()
 
+struct Int {
+  var value : Builtin.Int64
+}
+
 struct S {
   var x : Builtin.NativeObject
 }
@@ -46,11 +50,23 @@ class C {
   var w : FakeOptional<Builtin.NativeObject>
 }
 
+sil @cls_use : $@convention(thin) (@owned Cls) -> ()
+
 class Container {
   @sil_stored var c : Cls
   init()
 }
 
+struct SContainer {
+  @sil_stored var c : Cls
+  init()
+}
+
+struct SContainer2 {
+  @sil_stored var b : Cls
+  @sil_stored var c : Cls
+  init()
+}
 
 class RetainUser { }
 
@@ -2067,3 +2083,76 @@ bb2(%4 : $ErrorType):
   strong_release %0 : $Builtin.NativeObject
   throw %4 : $ErrorType
 }
+
+// CHECK-LABEL: sil @retain_release_struct_with_single_nontrivial
+// CHECK-NOT: strong_retain
+// CHECK-NOT: strong_release
+// CHECK: return
+sil @retain_release_struct_with_single_nontrivial : $@convention(thin) (SContainer) -> () {
+bb0(%0 : $SContainer):
+  retain_value %0 : $SContainer
+  %1 = struct_extract %0 : $SContainer, #SContainer.c
+  strong_release %1 : $Cls
+  %r = tuple()
+  return %r : $()
+}
+
+// CHECK-LABEL: sil @retain_release_struct_with_multiple_nontrivials
+// CHECK: retain_value 
+// CHECK: strong_release 
+// CHECK: return
+sil @retain_release_struct_with_multiple_nontrivials : $@convention(thin) (SContainer2) -> () {
+bb0(%0 : $SContainer2):
+  retain_value %0 : $SContainer2
+  %1 = struct_extract %0 : $SContainer2, #SContainer2.c
+  strong_release %1 : $Cls
+  %r = tuple()
+  return %r : $()
+}
+
+// Make sure the RR pair is not removed here as a result of the decrement and use.
+//
+// CHECK-LABEL: sil @retain_release_struct_with_single_nontrivial_with_use_and_decrement
+// CHECK: retain_value
+// CHECK: release_value
+sil @retain_release_struct_with_single_nontrivial_with_use_and_decrement : $@convention(thin) (SContainer) -> () {
+bb0(%0 : $SContainer):
+  retain_value %0 : $SContainer
+  %1 = struct_extract %0 : $SContainer, #SContainer.c
+  %2 = function_ref @cls_use : $@convention(thin) (@owned Cls) -> ()
+  apply %2(%1) : $@convention(thin) (@owned Cls) -> ()
+  apply %2(%1) : $@convention(thin) (@owned Cls) -> ()
+  release_value %1 : $Cls
+  %r = tuple()
+  return %r : $()
+}
+
+// Make sure %0 and %1 are identified as rc-identical and the RR can be removed.
+//
+// CHECK-LABEL: sil @retain_release_tuple_with_single_nontrivial
+// CHECK-NOT: strong_retain
+// CHECK-NOT: strong_release
+sil @retain_release_tuple_with_single_nontrivial : $@convention(thin) ((Cls, Int)) -> () {
+bb0(%0 : $(Cls, Int)):
+  %1 = tuple_extract %0 : $(Cls, Int), 0 
+  retain_value  %0 : $(Cls, Int)
+  strong_release %1 : $Cls
+  %r = tuple()
+  return %r : $()
+}
+
+// Make sure %0 and %1 are not rc-identical and RR are not removed.
+//
+// CHECK-LABEL: sil @retain_release_tuple_with_multiple_nontrivials
+// CHECK: retain_value 
+// CHECK: strong_release
+sil @retain_release_tuple_with_multiple_nontrivials : $@convention(thin) ((Cls, Cls)) -> () {
+bb0(%0 : $(Cls, Cls)):
+  %1 = tuple_extract %0 : $(Cls, Cls), 0 
+  retain_value  %0 : $(Cls, Cls)
+  strong_release %1 : $Cls
+  %r = tuple()
+  return %r : $()
+}
+
+
