Skip to content

Commit 3844327

Browse files
committed
Merge pull request #2249 from trentxintong/CodeMotion
Fix a non-deterministic retain/release insertion in RRCodeMotion
2 parents 0eae3a6 + 7d26a47 commit 3844327

File tree

4 files changed

+35
-10
lines changed

4 files changed

+35
-10
lines changed

lib/SILOptimizer/ARC/ARCMatchingSet.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333

3434
using namespace swift;
3535

36-
llvm::cl::opt<bool> DisableARCRRMotion("disable-code-motion-arc", llvm::cl::init(false));
36+
llvm::cl::opt<bool> DisableARCRRMotion("disable-code-motion-arc", llvm::cl::init(true));
3737

3838
//===----------------------------------------------------------------------===//
3939
// ARC Matching Set Builder

lib/SILOptimizer/Transforms/RetainReleaseCodeMotion.cpp

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ using namespace swift;
9393
STATISTIC(NumRetainsSunk, "Number of retains sunk");
9494
STATISTIC(NumReleasesHoisted, "Number of releases hoisted");
9595

96-
llvm::cl::opt<bool> DisableRRCodeMotion("disable-rr-cm", llvm::cl::init(true));
96+
llvm::cl::opt<bool> DisableRRCodeMotion("disable-rr-cm", llvm::cl::init(false));
9797

9898
//===----------------------------------------------------------------------===//
9999
// Utility
@@ -503,9 +503,12 @@ void RetainCodeMotionContext::computeCodeMotionGenKillSet() {
503503
bool RetainCodeMotionContext::performCodeMotion() {
504504
bool Changed = false;
505505
// Create the new retain instructions.
506-
for (auto RC : InsertPoints) {
507-
for (auto IP : RC.second) {
508-
createIncrementBefore(RC.first, IP);
506+
for (auto RC : RCRootVault) {
507+
auto Iter = InsertPoints.find(RC);
508+
if (Iter == InsertPoints.end())
509+
continue;
510+
for (auto IP : Iter->second) {
511+
createIncrementBefore(Iter->first, IP);
509512
Changed = true;
510513
}
511514
}
@@ -870,9 +873,12 @@ void ReleaseCodeMotionContext::mergeBBDataFlowStates(SILBasicBlock *BB) {
870873
bool ReleaseCodeMotionContext::performCodeMotion() {
871874
bool Changed = false;
872875
// Create the new releases at each anchor point.
873-
for (auto RC : InsertPoints) {
874-
for (auto IP : RC.second) {
875-
createDecrementBefore(RC.first, IP);
876+
for (auto RC : RCRootVault) {
877+
auto Iter = InsertPoints.find(RC);
878+
if (Iter == InsertPoints.end())
879+
continue;
880+
for (auto IP : Iter->second) {
881+
createDecrementBefore(Iter->first, IP);
876882
Changed = true;
877883
}
878884
}

lib/SILOptimizer/Transforms/SILCodeMotion.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ STATISTIC(NumSunk, "Number of instructions sunk");
3535
STATISTIC(NumRefCountOpsSimplified, "Number of enum ref count ops simplified");
3636
STATISTIC(NumHoisted, "Number of instructions hoisted");
3737

38-
llvm::cl::opt<bool> DisableSILRRCodeMotion("disable-sil-cm-rr-cm", llvm::cl::init(false));
38+
llvm::cl::opt<bool> DisableSILRRCodeMotion("disable-sil-cm-rr-cm", llvm::cl::init(true));
3939

4040
using namespace swift;
4141

test/SILOptimizer/retain_release_code_motion.sil

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-sil-opt -enable-sil-verify-all -retain-sinking -late-release-hoisting %s
1+
// RUN: %target-sil-opt -enable-sil-verify-all -retain-sinking -late-release-hoisting %s | FileCheck %s
22

33
import Builtin
44
import Swift
@@ -215,6 +215,25 @@ bb0(%0 : $*Builtin.NativeObject):
215215
return %9999 : $()
216216
}
217217

218+
// Make sure that is_unique stops code motion.
219+
// CHECK-LABEL: sil @retain_inserted_deterministically : $@convention(thin)
220+
// CHECK: bb0([[IN0:%[0-9]+]] : $Builtin.NativeObject, [[IN1:%[0-9]+]] : $Builtin.NativeObject, [[IN2:%[0-9]+]] : $Builtin.NativeObject, [[IN3:%[0-9]+]] : $Builtin.NativeObject):
221+
// CHECK: strong_retain [[IN1]] : $Builtin.NativeObject
222+
// CHECK: strong_retain [[IN3]] : $Builtin.NativeObject
223+
// CHECK: strong_retain [[IN2]] : $Builtin.NativeObject
224+
// CHECK: strong_retain [[IN0]] : $Builtin.NativeObject
225+
sil @retain_inserted_deterministically : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject, @owned Builtin.NativeObject, @owned Builtin.NativeObject) -> () {
226+
bb0(%0 : $Builtin.NativeObject, %1 : $Builtin.NativeObject, %2 : $Builtin.NativeObject, %3 : $Builtin.NativeObject):
227+
strong_retain %1 : $Builtin.NativeObject
228+
strong_retain %3 : $Builtin.NativeObject
229+
strong_retain %2 : $Builtin.NativeObject
230+
strong_retain %0 : $Builtin.NativeObject
231+
%9999 = tuple()
232+
%9998 = function_ref @blocker : $@convention(thin) () -> ()
233+
apply %9998() : $@convention(thin) () -> ()
234+
return %9999 : $()
235+
}
236+
218237
// CHECK-LABEL: sil @sink_retain_partially_block : $@convention(thin) (Builtin.NativeObject) -> () {
219238
// CHECK: bb3:
220239
// CHECK-NOT: string_retain

0 commit comments

Comments
 (0)