Skip to content

ARCCodeMotion: fix two problems in release hoisting: #10586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 26, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 38 additions & 7 deletions lib/SILOptimizer/Transforms/ARCCodeMotion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -618,13 +618,16 @@ class ReleaseBlockState : public BlockState {
}

/// constructor.
ReleaseBlockState(bool IsExit, unsigned size, bool MultiIteration) {
///
/// If \p InitOptimistic is true, the block in-bits are initialized to 1
/// which enables optimistic data flow evaluation.
ReleaseBlockState(bool InitOptimistic, unsigned size) {
// backward data flow.
// Initialize to true if we are running optimistic data flow, i.e.
// MultiIteration is true.
BBSetIn.resize(size, MultiIteration);
BBSetIn.resize(size, InitOptimistic);
BBSetOut.resize(size, false);
BBMaxSet.resize(size, !IsExit && MultiIteration);
BBMaxSet.resize(size, InitOptimistic);

// Genset and Killset are initially empty.
BBGenSet.resize(size, false);
Expand Down Expand Up @@ -735,6 +738,17 @@ bool ReleaseCodeMotionContext::requireIteration() {
}

void ReleaseCodeMotionContext::initializeCodeMotionDataFlow() {
// All blocks which are initialized with 1-bits. These are all blocks which
// eventually reach the function exit (return, throw), excluding the
// function exit blocks themselves.
// Optimistic initialization enables moving releases across loops. On the
// other hand, blocks, which never reach the function exit, e.g. infinite
// loop blocks, must be excluded. Otherwise we would end up inserting
// completely unrelated release instructions in such blocks.
llvm::SmallPtrSet<SILBasicBlock *, 32> BlocksInitOptimistically;

llvm::SmallVector<SILBasicBlock *, 32> Worklist;

// Find all the RC roots in the function.
for (auto &BB : *F) {
for (auto &II : BB) {
Expand All @@ -750,13 +764,25 @@ void ReleaseCodeMotionContext::initializeCodeMotionDataFlow() {
RCRootIndex[Root] = RCRootVault.size();
RCRootVault.insert(Root);
}
if (MultiIteration && BB.getTerminator()->isFunctionExiting())
Worklist.push_back(&BB);
}

// Find all blocks from which there is a path to the function exit.
// Note: the Worklist is empty if we are not in MultiIteration mode.
while (!Worklist.empty()) {
SILBasicBlock *BB = Worklist.pop_back_val();
for (SILBasicBlock *Pred : BB->getPredecessorBlocks()) {
if (BlocksInitOptimistically.insert(Pred).second)
Worklist.push_back(Pred);
}
}

// Initialize all the data flow bit vector for all basic blocks.
for (auto &BB : *F) {
BlockStates[&BB] = new (BPA.Allocate())
ReleaseBlockState(BB.getTerminator()->isFunctionExiting(),
RCRootVault.size(), MultiIteration);
ReleaseBlockState(BlocksInitOptimistically.count(&BB) != 0,
RCRootVault.size());
}
}

Expand Down Expand Up @@ -1038,17 +1064,22 @@ class ARCCodeMotion : public SILFunctionTransform {
return;

DEBUG(llvm::dbgs() << "*** ARCCM on function: " << F->getName() << " ***\n");

PostOrderAnalysis *POA = PM->getAnalysis<PostOrderAnalysis>();

// Split all critical edges.
//
// TODO: maybe we can do this lazily or maybe we should disallow SIL passes
// to create critical edges.
bool EdgeChanged = splitAllCriticalEdges(*F, false, nullptr, nullptr);
if (EdgeChanged)
POA->invalidateFunction(F);

llvm::SpecificBumpPtrAllocator<BlockState> BPA;
auto *PO = PM->getAnalysis<PostOrderAnalysis>()->get(F);
auto *PO = POA->get(F);
auto *AA = PM->getAnalysis<AliasAnalysis>();
auto *RCFI = PM->getAnalysis<RCIdentityAnalysis>()->get(F);

llvm::SpecificBumpPtrAllocator<BlockState> BPA;
bool InstChanged = false;
if (Kind == Release) {
// TODO: we should consider Throw block as well, or better we should
Expand Down
88 changes: 88 additions & 0 deletions test/SILOptimizer/retain_release_code_motion.sil
Original file line number Diff line number Diff line change
Expand Up @@ -443,3 +443,91 @@ bb3:
%5 = tuple()
return %5 : $()
}

// CHECK-LABEL: sil @move_retain_over_loop
// CHECK: bb0({{.*}}):
// CHECK-NEXT: br bb1
// CHECK: bb1:
// CHECK-NEXT: cond_br
// CHECK: bb2:
// CHECK: strong_retain
// CHECK: apply
// CHECK: strong_release
// CHECK: return
sil @move_retain_over_loop : $@convention(thin) (Builtin.NativeObject) -> () {
bb0(%0 : $Builtin.NativeObject):
strong_retain %0 : $Builtin.NativeObject
br bb1

bb1:
cond_br undef, bb1, bb2

bb2:
%2 = function_ref @blocker : $@convention(thin) () -> ()
apply %2() : $@convention(thin) () -> ()
strong_release %0 : $Builtin.NativeObject
%1 = tuple()
return %1 : $()
}

// CHECK-LABEL: sil @move_release_over_loop
// CHECK: bb0{{.*}}:
// CHECK: strong_retain
// CHECK: apply
// CHECK: strong_release
// CHECK: br bb1
// CHECK: bb1:
// CHECK-NEXT: cond_br
// CHECK: bb2:
// CHECK-NEXT: br bb1
// CHECK: bb3:
// CHECK-NEXT: tuple
// CHECK-NEXT: return
sil @move_release_over_loop : $@convention(thin) (Builtin.NativeObject) -> () {
bb0(%0 : $Builtin.NativeObject):
strong_retain %0 : $Builtin.NativeObject
%2 = function_ref @blocker : $@convention(thin) () -> ()
apply %2() : $@convention(thin) () -> ()
br bb1

bb1:
cond_br undef, bb1, bb2

bb2:
strong_release %0 : $Builtin.NativeObject
%1 = tuple()
return %1 : $()
}

// CHECK-LABEL: sil @handle_infinite_loop
// CHECK: bb0{{.*}}:
// CHECK-NEXT: cond_br
// CHECK: bb1:
// CHECK-NOT: {{(retain|release)}}
// CHECK: apply
// CHECK-NEXT: br bb2
// CHECK: bb2:
// CHECK-NEXT: br bb2
// CHECK: bb3:
// CHECK: strong_retain
// CHECK: strong_release
// CHECK: return
sil @handle_infinite_loop : $@convention(thin) (@inout Builtin.NativeObject) -> () {
bb0(%a : $*Builtin.NativeObject):
cond_br undef, bb1, bb3

bb1:
%2 = function_ref @blocker : $@convention(thin) () -> ()
apply %2() : $@convention(thin) () -> ()
br bb2

bb2:
br bb2

bb3:
%0 = load %a : $*Builtin.NativeObject
strong_retain %0 : $Builtin.NativeObject
strong_release %0 : $Builtin.NativeObject
%1 = tuple()
return %1 : $()
}