diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp --- a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp +++ b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp @@ -532,12 +532,15 @@ bool VisitBottomUp(BasicBlock *BB, DenseMap &BBStates, BlotMapVector &Retains); - bool VisitInstructionTopDown(Instruction *Inst, - DenseMap &Releases, - BBState &MyStates); - bool VisitTopDown(BasicBlock *BB, - DenseMap &BBStates, - DenseMap &Releases); + bool VisitInstructionTopDown( + Instruction *Inst, DenseMap &Releases, BBState &MyStates, + const DenseMap> + &ReleaseInsertPtToRCIdentityRoots); + bool VisitTopDown( + BasicBlock *BB, DenseMap &BBStates, + DenseMap &Releases, + const DenseMap> + &ReleaseInsertPtToRCIdentityRoots); bool Visit(Function &F, DenseMap &BBStates, BlotMapVector &Retains, DenseMap &Releases); @@ -1495,14 +1498,59 @@ return NestingDetected; } -bool -ObjCARCOpt::VisitInstructionTopDown(Instruction *Inst, - DenseMap &Releases, - BBState &MyStates) { +// Fill ReleaseInsertPtToRCIdentityRoots. +static void collectReleaseInsertPts( + const BlotMapVector &Retains, + DenseMap> + &ReleaseInsertPtToRCIdentityRoots) { + for (auto &P : Retains) { + // Get the RC identity root of the objc_retain call. + Instruction *Retain = cast(P.first); + Value *Root = GetRCIdentityRoot(Retain->getOperand(0)); + // Collect all the insertion points of the objc_release calls corresponding + // to the objc_retain call. + for (const Instruction *InsertPt : P.second.ReverseInsertPts) + ReleaseInsertPtToRCIdentityRoots[InsertPt].insert(Root); + } +} + +// Get the RC identity root from an insertion point of an objc_release call. +// Return nullptr if the passed instruction isn't an insertion point. +static const SmallPtrSet * +getRCIdentityRootsFromReleaseInsertPt( + const Instruction *InsertPt, + const DenseMap> + &ReleaseInsertPtToRCIdentityRoots) { + auto I = ReleaseInsertPtToRCIdentityRoots.find(InsertPt); + if (I == ReleaseInsertPtToRCIdentityRoots.end()) + return nullptr; + return &I->second; +} + +bool ObjCARCOpt::VisitInstructionTopDown( + Instruction *Inst, DenseMap &Releases, BBState &MyStates, + const DenseMap> + &ReleaseInsertPtToRCIdentityRoots) { bool NestingDetected = false; ARCInstKind Class = GetARCInstKind(Inst); const Value *Arg = nullptr; + // Make sure a call to objc_retain isn't moved past insertion points of calls + // to objc_release. + if (const SmallPtrSet *Roots = + getRCIdentityRootsFromReleaseInsertPt( + Inst, ReleaseInsertPtToRCIdentityRoots)) + for (auto *Root : *Roots) { + TopDownPtrState &S = MyStates.getPtrTopDownState(Root); + // Disable code motion if the current position is S_Retain to prevent + // moving the objc_retain call past objc_release calls. If it's + // S_CanRelease or larger, it's not necessary to disable code motion as + // the insertion points that prevent the objc_retain call from moving down + // should have been set already. + if (S.GetSeq() == S_Retain) + S.SetCFGHazardAfflicted(true); + } + LLVM_DEBUG(dbgs() << " Class: " << Class << "\n"); switch (Class) { @@ -1565,10 +1613,11 @@ return NestingDetected; } -bool -ObjCARCOpt::VisitTopDown(BasicBlock *BB, - DenseMap &BBStates, - DenseMap &Releases) { +bool ObjCARCOpt::VisitTopDown( + BasicBlock *BB, DenseMap &BBStates, + DenseMap &Releases, + const DenseMap> + &ReleaseInsertPtToRCIdentityRoots) { LLVM_DEBUG(dbgs() << "\n== ObjCARCOpt::VisitTopDown ==\n"); bool NestingDetected = false; BBState &MyStates = BBStates[BB]; @@ -1608,7 +1657,8 @@ for (Instruction &Inst : *BB) { LLVM_DEBUG(dbgs() << " Visiting " << Inst << "\n"); - NestingDetected |= VisitInstructionTopDown(&Inst, Releases, MyStates); + NestingDetected |= VisitInstructionTopDown( + &Inst, Releases, MyStates, ReleaseInsertPtToRCIdentityRoots); // Bail out if the number of pointers being tracked becomes too large so // that this pass can complete in a reasonable amount of time. @@ -1728,10 +1778,15 @@ return false; } + DenseMap> + ReleaseInsertPtToRCIdentityRoots; + collectReleaseInsertPts(Retains, ReleaseInsertPtToRCIdentityRoots); + // Use reverse-postorder for top-down. bool TopDownNestingDetected = false; for (BasicBlock *BB : llvm::reverse(PostOrder)) { - TopDownNestingDetected |= VisitTopDown(BB, BBStates, Releases); + TopDownNestingDetected |= + VisitTopDown(BB, BBStates, Releases, ReleaseInsertPtToRCIdentityRoots); if (DisableRetainReleasePairing) return false; } diff --git a/llvm/test/Transforms/ObjCARC/code-motion.ll b/llvm/test/Transforms/ObjCARC/code-motion.ll --- a/llvm/test/Transforms/ObjCARC/code-motion.ll +++ b/llvm/test/Transforms/ObjCARC/code-motion.ll @@ -2,6 +2,7 @@ declare void @alterRefCount() declare void @use(i8*) +declare void @readOnlyFunc(i8*, i8*) @g0 = global i8* null, align 8 @@ -38,10 +39,78 @@ ret void } +; Check that code motion is disabled. +; Previously, ARC optimizer would move the release past the retain. + +; if.then: +; call void @readOnlyFunc(i8* %obj, i8* null) +; call void @llvm.objc.release(i8* %obj) #1, !clang.imprecise_release !2 +; %1 = add i32 1, 2 +; %2 = tail call i8* @llvm.objc.retain(i8* %obj) + +; CHECK: define void @test3(i8* %[[OBJ:.*]], i1 +; CHECK-NEXT: call i8* @llvm.objc.retain(i8* %[[OBJ]]) +; CHECK-NOT: call {{.*}} @llvm.objc +; CHECK: call void @llvm.objc.release(i8* %[[OBJ]]) +; CHECK-NEXT: ret void + +define void @test3(i8* %obj, i1 %cond) { + %v0 = call i8* @llvm.objc.retain(i8* %obj) + br i1 %cond, label %if.then, label %if.else + +if.then: + call void @readOnlyFunc(i8* %obj, i8* null) #0 + add i32 1, 2 + call void @alterRefCount() + br label %join + +if.else: + call void @alterRefCount() + call void @use(i8* %obj) + br label %join + +join: + call void @llvm.objc.release(i8* %obj), !clang.imprecise_release !9 + ret void +} + +; CHECK: define void @test4(i8* %[[OBJ0:.*]], i8* %[[OBJ1:.*]], i1 +; CHECK-NEXT: call i8* @llvm.objc.retain(i8* %[[OBJ0]]) +; CHECK-NEXT: call i8* @llvm.objc.retain(i8* %[[OBJ1]]) +; CHECK-NOT: call {{.*}} @llvm.objc +; CHECK: call void @llvm.objc.release(i8* %[[OBJ0]]) +; CHECK-NEXT: call void @llvm.objc.release(i8* %[[OBJ1]]) +; CHECK-NEXT: ret void + +define void @test4(i8* %obj0, i8* %obj1, i1 %cond) { + %v0 = call i8* @llvm.objc.retain(i8* %obj0) + %v1 = call i8* @llvm.objc.retain(i8* %obj1) + br i1 %cond, label %if.then, label %if.else + +if.then: + call void @readOnlyFunc(i8* %obj0, i8* %obj1) #0 + add i32 1, 2 + call void @alterRefCount() + br label %join + +if.else: + call void @alterRefCount() + call void @use(i8* %obj0) + call void @use(i8* %obj1) + br label %join + +join: + call void @llvm.objc.release(i8* %obj0), !clang.imprecise_release !9 + call void @llvm.objc.release(i8* %obj1), !clang.imprecise_release !9 + ret void +} + declare void @llvm.dbg.declare(metadata, metadata, metadata) declare i8* @llvm.objc.retain(i8*) local_unnamed_addr declare void @llvm.objc.release(i8*) local_unnamed_addr +attributes #0 = { readonly } + !llvm.module.flags = !{!0, !1} !0 = !{i32 2, !"Dwarf Version", i32 4}