[objc-arc] KnownSafe does not imply that it is safe to perform code motion across...

rdar://13949644

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@182670 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/ObjCARC/ObjCARCOpts.cpp b/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
index 91490d1b2d06f0d7418a70fff6c710eb89664f1c..e6e1ff95aabf5c7a064b05ab606edabf0b36ac63 100644 (file)
--- a/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
+++ b/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
@@ -455,8 +455,13 @@ namespace {
     /// sequence.
     SmallPtrSet<Instruction *, 2> ReverseInsertPts;
 
+    /// If this is true, we cannot perform code motion but can still remove
+    /// retain/release pairs.
+    bool CFGHazardAfflicted;
+
     RRInfo() :
-      KnownSafe(false), IsTailCallRelease(false), ReleaseMetadata(0) {}
+      KnownSafe(false), IsTailCallRelease(false), ReleaseMetadata(0),
+      CFGHazardAfflicted(false) {}
 
     void clear();
 
@@ -472,6 +477,7 @@ void RRInfo::clear() {
   ReleaseMetadata = 0;
   Calls.clear();
   ReverseInsertPts.clear();
+  CFGHazardAfflicted = false;
 }
 
 namespace {
@@ -559,6 +565,7 @@ PtrState::Merge(const PtrState &Other, bool TopDown) {
     RRI.IsTailCallRelease = RRI.IsTailCallRelease &&
                             Other.RRI.IsTailCallRelease;
     RRI.Calls.insert(Other.RRI.Calls.begin(), Other.RRI.Calls.end());
+    RRI.CFGHazardAfflicted |= Other.RRI.CFGHazardAfflicted;
 
     // Merge the insert point sets. If there are any differences,
     // that makes this a partial merge.
@@ -1690,6 +1697,7 @@ static void CheckForUseCFGHazard(const Sequence SuccSSeq,
                                  PtrState &S,
                                  bool &SomeSuccHasSame,
                                  bool &AllSuccsHaveSame,
+                                 bool &NotAllSeqEqualButKnownSafe,
                                  bool &ShouldContinue) {
   switch (SuccSSeq) {
   case S_CanRelease: {
@@ -1697,6 +1705,7 @@ static void CheckForUseCFGHazard(const Sequence SuccSSeq,
       S.ClearSequenceProgress();
       break;
     }
+    S.RRI.CFGHazardAfflicted = true;
     ShouldContinue = true;
     break;
   }
@@ -1708,6 +1717,8 @@ static void CheckForUseCFGHazard(const Sequence SuccSSeq,
   case S_MovableRelease:
     if (!S.RRI.KnownSafe && !SuccSRRIKnownSafe)
       AllSuccsHaveSame = false;
+    else
+      NotAllSeqEqualButKnownSafe = true;
     break;
   case S_Retain:
     llvm_unreachable("bottom-up pointer in retain state!");
@@ -1723,7 +1734,8 @@ static void CheckForCanReleaseCFGHazard(const Sequence SuccSSeq,
                                         const bool SuccSRRIKnownSafe,
                                         PtrState &S,
                                         bool &SomeSuccHasSame,
-                                        bool &AllSuccsHaveSame) {
+                                        bool &AllSuccsHaveSame,
+                                        bool &NotAllSeqEqualButKnownSafe) {
   switch (SuccSSeq) {
   case S_CanRelease:
     SomeSuccHasSame = true;
@@ -1734,6 +1746,8 @@ static void CheckForCanReleaseCFGHazard(const Sequence SuccSSeq,
   case S_Use:
     if (!S.RRI.KnownSafe && !SuccSRRIKnownSafe)
       AllSuccsHaveSame = false;
+    else
+      NotAllSeqEqualButKnownSafe = true;
     break;
   case S_Retain:
     llvm_unreachable("bottom-up pointer in retain state!");
@@ -1769,6 +1783,7 @@ ObjCARCOpt::CheckForCFGHazards(const BasicBlock *BB,
     const TerminatorInst *TI = cast<TerminatorInst>(&BB->back());
     bool SomeSuccHasSame = false;
     bool AllSuccsHaveSame = true;
+    bool NotAllSeqEqualButKnownSafe = false;
 
     succ_const_iterator SI(TI), SE(TI, false);
 
@@ -1800,17 +1815,17 @@ ObjCARCOpt::CheckForCFGHazards(const BasicBlock *BB,
       switch(S.GetSeq()) {
       case S_Use: {
         bool ShouldContinue = false;
-        CheckForUseCFGHazard(SuccSSeq, SuccSRRIKnownSafe, S,
-                             SomeSuccHasSame, AllSuccsHaveSame,
+        CheckForUseCFGHazard(SuccSSeq, SuccSRRIKnownSafe, S, SomeSuccHasSame,
+                             AllSuccsHaveSame, NotAllSeqEqualButKnownSafe,
                              ShouldContinue);
         if (ShouldContinue)
           continue;
         break;
       }
       case S_CanRelease: {
-        CheckForCanReleaseCFGHazard(SuccSSeq, SuccSRRIKnownSafe,
-                                    S, SomeSuccHasSame,
-                                    AllSuccsHaveSame);
+        CheckForCanReleaseCFGHazard(SuccSSeq, SuccSRRIKnownSafe, S,
+                                    SomeSuccHasSame, AllSuccsHaveSame,
+                                    NotAllSeqEqualButKnownSafe);
         break;
       }
       case S_Retain:
@@ -1825,8 +1840,15 @@ ObjCARCOpt::CheckForCFGHazards(const BasicBlock *BB,
     // If the state at the other end of any of the successor edges
     // matches the current state, require all edges to match. This
     // guards against loops in the middle of a sequence.
-    if (SomeSuccHasSame && !AllSuccsHaveSame)
+    if (SomeSuccHasSame && !AllSuccsHaveSame) {
       S.ClearSequenceProgress();
+    } else if (NotAllSeqEqualButKnownSafe) {
+      // If we would have cleared the state foregoing the fact that we are known
+      // safe, stop code motion. This is because whether or not it is safe to
+      // remove RR pairs via KnownSafe is an orthogonal concept to whether we
+      // are allowed to perform code motion.
+      S.RRI.CFGHazardAfflicted = true;
+    }
   }
 }
 
@@ -2489,6 +2511,7 @@ ObjCARCOpt::ConnectTDBUTraversals(DenseMap<const BasicBlock *, BBState>
   // we are dealing with a retainable object with multiple provenance sources.
   bool KnownSafeTD = true, KnownSafeBU = true;
   bool MultipleOwners = false;
+  bool CFGHazardAfflicted = false;
 
   // Connect the dots between the top-down-collected RetainsToMove and
   // bottom-up-collected ReleasesToMove to form sets of related calls.
@@ -2565,6 +2588,7 @@ ObjCARCOpt::ConnectTDBUTraversals(DenseMap<const BasicBlock *, BBState>
       assert(It != Releases.end());
       const RRInfo &NewReleaseRRI = It->second;
       KnownSafeBU &= NewReleaseRRI.KnownSafe;
+      CFGHazardAfflicted |= NewReleaseRRI.CFGHazardAfflicted;
       for (SmallPtrSet<Instruction *, 2>::const_iterator
              LI = NewReleaseRRI.Calls.begin(),
              LE = NewReleaseRRI.Calls.end(); LI != LE; ++LI) {
@@ -2618,6 +2642,14 @@ ObjCARCOpt::ConnectTDBUTraversals(DenseMap<const BasicBlock *, BBState>
     // less aggressive solution which is.
     if (NewDelta != 0)
       return false;
+
+    // At this point, we are not going to remove any RR pairs, but we still are
+    // able to move RR pairs. If one of our pointers is afflicted with
+    // CFGHazards, we cannot perform such code motion so exit early.
+    const bool WillPerformCodeMotion = RetainsToMove.ReverseInsertPts.size() ||
+      ReleasesToMove.ReverseInsertPts.size();
+    if (CFGHazardAfflicted && WillPerformCodeMotion)
+      return false;
   }
 
   // Determine whether the original call points are balanced in the retain and

diff --git a/test/Transforms/ObjCARC/allocas.ll b/test/Transforms/ObjCARC/allocas.ll
index 58740d2d802fbd83951eb93cd67c7327088511e4..50656739ae7134209605152874a9d5cdc5706956 100644 (file)
--- a/test/Transforms/ObjCARC/allocas.ll
+++ b/test/Transforms/ObjCARC/allocas.ll
@@ -18,6 +18,8 @@ declare void @callee()
 declare void @callee_fnptr(void ()*)
 declare void @invokee()
 declare i8* @returner()
+declare i8* @returner1()
+declare i8* @returner2()
 declare void @bar(i32 ()*)
 declare void @use_alloca(i8**)
 
@@ -295,6 +297,204 @@ bb3:
   ret void
 }
 
+; CHECK: define void @test2d(i8* %x)
+; CHECK: @objc_retain(i8* %x)
+; CHECK: @objc_retain(i8* %x)
+; CHECK: @objc_release(i8* %y)
+; CHECK: @objc_release(i8* %x)
+; CHECK: ret void
+; CHECK: }
+define void @test2d(i8* %x) {
+entry:
+  tail call i8* @objc_retain(i8* %x)
+  br label %bb1
+
+bb1:
+  %Abb1 = alloca i8*, i32 3
+  %gepbb11 = getelementptr i8** %Abb1, i32 2
+  store i8* %x, i8** %gepbb11, align 8
+  %gepbb12 = getelementptr i8** %Abb1, i32 2
+  %ybb1 = load i8** %gepbb12
+  br label %bb3
+
+bb2:
+  %Abb2 = alloca i8*, i32 4
+  %gepbb21 = getelementptr i8** %Abb2, i32 2
+  store i8* %x, i8** %gepbb21, align 8
+  %gepbb22 = getelementptr i8** %Abb2, i32 2
+  %ybb2 = load i8** %gepbb22
+  br label %bb3
+
+bb3:
+  %A = phi i8** [ %Abb1, %bb1 ], [ %Abb2, %bb2 ]
+  %y = phi i8* [ %ybb1, %bb1 ], [ %ybb2, %bb2 ]
+  tail call i8* @objc_retain(i8* %x)
+  call void @use_alloca(i8** %A)
+  call void @objc_release(i8* %y), !clang.imprecise_release !0
+  call void @use_pointer(i8* %x)
+  call void @objc_release(i8* %x), !clang.imprecise_release !0
+  ret void
+}
+
+; Make sure in the presense of allocas, if we find a cfghazard we do not perform
+; code motion even if we are known safe. These two concepts are separate and
+; should be treated as such.
+;
+; rdar://13949644
+
+; CHECK: define void @test3a() {
+; CHECK: entry:
+; CHECK:   @objc_retainAutoreleasedReturnValue
+; CHECK:   @objc_retain
+; CHECK:   @objc_retain
+; CHECK:   @objc_retain
+; CHECK:   @objc_retain
+; CHECK: arraydestroy.body:
+; CHECK:   @objc_release
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.done:
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.body1:
+; CHECK:   @objc_release
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.done1:
+; CHECK: @objc_release
+; CHECK: ret void
+; CHECK: }
+define void @test3a() {
+entry:
+  %keys = alloca [2 x i8*], align 16
+  %objs = alloca [2 x i8*], align 16
+  
+  %call1 = call i8* @returner()
+  %tmp0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %call1)
+
+  %objs.begin = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 0
+  tail call i8* @objc_retain(i8* %call1)
+  store i8* %call1, i8** %objs.begin, align 8
+  %objs.elt = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 1
+  tail call i8* @objc_retain(i8* %call1)
+  store i8* %call1, i8** %objs.elt
+
+  %call2 = call i8* @returner1()
+  %call3 = call i8* @returner2()
+  %keys.begin = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 0
+  tail call i8* @objc_retain(i8* %call2)
+  store i8* %call2, i8** %keys.begin, align 8
+  %keys.elt = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 1
+  tail call i8* @objc_retain(i8* %call3)
+  store i8* %call3, i8** %keys.elt  
+  
+  %gep = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 2
+  br label %arraydestroy.body
+
+arraydestroy.body:
+  %arraydestroy.elementPast = phi i8** [ %gep, %entry ], [ %arraydestroy.element, %arraydestroy.body ]
+  %arraydestroy.element = getelementptr inbounds i8** %arraydestroy.elementPast, i64 -1
+  %destroy_tmp = load i8** %arraydestroy.element, align 8
+  call void @objc_release(i8* %destroy_tmp), !clang.imprecise_release !0
+  %objs_ptr = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 0
+  %arraydestroy.cmp = icmp eq i8** %arraydestroy.element, %objs_ptr
+  br i1 %arraydestroy.cmp, label %arraydestroy.done, label %arraydestroy.body
+
+arraydestroy.done:
+  %gep1 = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 2
+  br label %arraydestroy.body1
+
+arraydestroy.body1:
+  %arraydestroy.elementPast1 = phi i8** [ %gep1, %arraydestroy.done ], [ %arraydestroy.element1, %arraydestroy.body1 ]
+  %arraydestroy.element1 = getelementptr inbounds i8** %arraydestroy.elementPast1, i64 -1
+  %destroy_tmp1 = load i8** %arraydestroy.element1, align 8
+  call void @objc_release(i8* %destroy_tmp1), !clang.imprecise_release !0
+  %keys_ptr = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 0
+  %arraydestroy.cmp1 = icmp eq i8** %arraydestroy.element1, %keys_ptr
+  br i1 %arraydestroy.cmp1, label %arraydestroy.done1, label %arraydestroy.body1
+
+arraydestroy.done1:
+  call void @objc_release(i8* %call1), !clang.imprecise_release !0
+  ret void
+}
+
+; Make sure that even though we stop said code motion we still allow for
+; pointers to be removed if we are known safe in both directions.
+;
+; rdar://13949644
+
+; CHECK: define void @test3b() {
+; CHECK: entry:
+; CHECK:   @objc_retainAutoreleasedReturnValue
+; CHECK:   @objc_retain
+; CHECK:   @objc_retain
+; CHECK:   @objc_retain
+; CHECK:   @objc_retain
+; CHECK: arraydestroy.body:
+; CHECK:   @objc_release
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.done:
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.body1:
+; CHECK:   @objc_release
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.done1:
+; CHECK: @objc_release
+; CHECK: ret void
+; CHECK: }
+define void @test3b() {
+entry:
+  %keys = alloca [2 x i8*], align 16
+  %objs = alloca [2 x i8*], align 16
+  
+  %call1 = call i8* @returner()
+  %tmp0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %call1)
+  %tmp1 = tail call i8* @objc_retain(i8* %call1)
+
+  %objs.begin = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 0
+  tail call i8* @objc_retain(i8* %call1)
+  store i8* %call1, i8** %objs.begin, align 8
+  %objs.elt = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 1
+  tail call i8* @objc_retain(i8* %call1)
+  store i8* %call1, i8** %objs.elt
+
+  %call2 = call i8* @returner1()
+  %call3 = call i8* @returner2()
+  %keys.begin = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 0
+  tail call i8* @objc_retain(i8* %call2)
+  store i8* %call2, i8** %keys.begin, align 8
+  %keys.elt = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 1
+  tail call i8* @objc_retain(i8* %call3)
+  store i8* %call3, i8** %keys.elt  
+  
+  %gep = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 2
+  br label %arraydestroy.body
+
+arraydestroy.body:
+  %arraydestroy.elementPast = phi i8** [ %gep, %entry ], [ %arraydestroy.element, %arraydestroy.body ]
+  %arraydestroy.element = getelementptr inbounds i8** %arraydestroy.elementPast, i64 -1
+  %destroy_tmp = load i8** %arraydestroy.element, align 8
+  call void @objc_release(i8* %destroy_tmp), !clang.imprecise_release !0
+  %objs_ptr = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 0
+  %arraydestroy.cmp = icmp eq i8** %arraydestroy.element, %objs_ptr
+  br i1 %arraydestroy.cmp, label %arraydestroy.done, label %arraydestroy.body
+
+arraydestroy.done:
+  %gep1 = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 2
+  br label %arraydestroy.body1
+
+arraydestroy.body1:
+  %arraydestroy.elementPast1 = phi i8** [ %gep1, %arraydestroy.done ], [ %arraydestroy.element1, %arraydestroy.body1 ]
+  %arraydestroy.element1 = getelementptr inbounds i8** %arraydestroy.elementPast1, i64 -1
+  %destroy_tmp1 = load i8** %arraydestroy.element1, align 8
+  call void @objc_release(i8* %destroy_tmp1), !clang.imprecise_release !0
+  %keys_ptr = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 0
+  %arraydestroy.cmp1 = icmp eq i8** %arraydestroy.element1, %keys_ptr
+  br i1 %arraydestroy.cmp1, label %arraydestroy.done1, label %arraydestroy.body1
+
+arraydestroy.done1:
+  call void @objc_release(i8* %call1), !clang.imprecise_release !0
+  call void @objc_release(i8* %call1), !clang.imprecise_release !0
+  ret void
+}
+
 !0 = metadata !{}
 
 declare i32 @__gxx_personality_v0(...)