Skip to content

Commit cc16ddf

Browse files
committed
Revert "[SILOptimizer] Don't diagnose infinite recursion if a branch terminates (#19724)"
This reverts commit e94450e. rdar://45080912
1 parent 08acbf3 commit cc16ddf

18 files changed

+104
-183
lines changed

docs/ARCOptimization.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ Semantic Tags
373373
ARC takes advantage of certain semantic tags. This section documents these
374374
semantics and their meanings.
375375

376-
programtermination_point
376+
arc.programtermination_point
377377
----------------------------
378378

379379
If this semantic tag is applied to a function, then we know that:
@@ -396,7 +396,7 @@ scope's lifetime may never end. This means that:
396396

397397
1. While we can not ignore all such unreachable terminated blocks for ARC
398398
purposes for instance, if we sink a retain past a br into a non
399-
programtermination_point block, we must sink the retain into the block.
399+
arc.programtermination_point block, we must sink the retain into the block.
400400

401401
2. If we are able to infer that an object's lifetime scope would never end due
402402
to the unreachable/no-return function, then we do not need to end the lifetime

include/swift/SIL/SILBasicBlock.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,12 +370,9 @@ public llvm::ilist_node<SILBasicBlock>, public SILAllocated<SILBasicBlock> {
370370
/// no-return apply or builtin.
371371
bool isNoReturn() const;
372372

373-
/// Returns true if this block only contains a branch instruction.
373+
/// Returns true if this instruction only contains a branch instruction.
374374
bool isTrampoline() const;
375375

376-
/// Returns true if this block traps without any side effects.
377-
bool isProgramTerminationPoint() const;
378-
379376
/// Returns true if it is legal to hoist instructions into this block.
380377
///
381378
/// Used by llvm::LoopInfo.

include/swift/SIL/SILInstruction.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6645,9 +6645,6 @@ class TermInst : public NonValueInstruction {
66456645
/// Returns true if this terminator exits the function.
66466646
bool isFunctionExiting() const;
66476647

6648-
/// Returns true if this terminator terminates the program.
6649-
bool isProgramTerminating() const;
6650-
66516648
TermKind getTermKind() const { return TermKind(getKind()); }
66526649
};
66536650

include/swift/SILOptimizer/Analysis/ARCAnalysis.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,9 @@ class ReleaseTracker {
424424
/// FinalRelease.
425425
bool getFinalReleasesForValue(SILValue Value, ReleaseTracker &Tracker);
426426

427+
/// Match a call to a trap BB with no ARC relevant side effects.
428+
bool isARCInertTrapBB(const SILBasicBlock *BB);
429+
427430
/// Get the two result values of the builtin "unsafeGuaranteed" instruction.
428431
///
429432
/// Gets the (GuaranteedValue, Token) tuple from a call to "unsafeGuaranteed"

include/swift/SILOptimizer/Analysis/ProgramTerminationAnalysis.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class ProgramTerminationFunctionInfo {
3939
public:
4040
ProgramTerminationFunctionInfo(const SILFunction *F) {
4141
for (const auto &BB : *F) {
42-
if (!BB.isProgramTerminationPoint())
42+
if (!isARCInertTrapBB(&BB))
4343
continue;
4444
ProgramTerminatingBlocks.insert(&BB);
4545
}

include/swift/Strings.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ constexpr static const char BUILTIN_TYPE_NAME_VEC[] = "Builtin.Vec";
7777
constexpr static const char BUILTIN_TYPE_NAME_SILTOKEN[] = "Builtin.SILToken";
7878
/// The name of the Builtin type for Word
7979
constexpr static const char BUILTIN_TYPE_NAME_WORD[] = "Builtin.Word";
80-
constexpr static StringLiteral SEMANTICS_PROGRAMTERMINATION_POINT =
81-
"programtermination_point";
80+
constexpr static StringLiteral SEMANTICS_ARC_PROGRAMTERMINATION_POINT =
81+
"arc.programtermination_point";
8282
} // end namespace swift
8383

8484
#endif // SWIFT_STRINGS_H

lib/SIL/SILBasicBlock.cpp

Lines changed: 0 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
#include "swift/SIL/SILFunction.h"
2525
#include "swift/SIL/SILInstruction.h"
2626
#include "swift/SIL/SILModule.h"
27-
#include "swift/Strings.h"
2827
using namespace swift;
2928

3029
//===----------------------------------------------------------------------===//
@@ -337,80 +336,6 @@ bool SILBasicBlock::isEntry() const {
337336
return this == &*getParent()->begin();
338337
}
339338

340-
//===----------------------------------------------------------------------===//
341-
// Program Termination Point Analysis
342-
//===----------------------------------------------------------------------===//
343-
344-
static bool ignorableApplyInstInUnreachableBlock(const ApplyInst *AI) {
345-
const auto *Fn = AI->getReferencedFunction();
346-
if (!Fn)
347-
return false;
348-
349-
return Fn->hasSemanticsAttr(SEMANTICS_PROGRAMTERMINATION_POINT);
350-
}
351-
352-
static bool ignorableBuiltinInstInUnreachableBlock(const BuiltinInst *BI) {
353-
const BuiltinInfo &BInfo = BI->getBuiltinInfo();
354-
if (BInfo.ID == BuiltinValueKind::CondUnreachable)
355-
return true;
356-
357-
const IntrinsicInfo &IInfo = BI->getIntrinsicInfo();
358-
if (IInfo.ID == llvm::Intrinsic::trap)
359-
return true;
360-
361-
return false;
362-
}
363-
364-
// Match a call to a basic block that's going to terminate
365-
// with no side effects.
366-
bool SILBasicBlock::isProgramTerminationPoint() const {
367-
// Do a quick check at the beginning to make sure that our terminator is
368-
// actually an unreachable. This ensures that in many cases this function will
369-
// exit early and quickly.
370-
auto II = rbegin();
371-
if (!isa<UnreachableInst>(*II))
372-
return false;
373-
374-
auto IE = rend();
375-
while (II != IE) {
376-
// Ignore any instructions without side effects.
377-
if (!II->mayHaveSideEffects()) {
378-
++II;
379-
continue;
380-
}
381-
382-
// Ignore cond fail.
383-
if (isa<CondFailInst>(*II)) {
384-
++II;
385-
continue;
386-
}
387-
388-
// Check for apply insts that we can ignore.
389-
if (auto *AI = dyn_cast<ApplyInst>(&*II)) {
390-
if (ignorableApplyInstInUnreachableBlock(AI)) {
391-
++II;
392-
continue;
393-
}
394-
}
395-
396-
// Check for builtins that we can ignore.
397-
if (auto *BI = dyn_cast<BuiltinInst>(&*II)) {
398-
if (ignorableBuiltinInstInUnreachableBlock(BI)) {
399-
++II;
400-
continue;
401-
}
402-
}
403-
404-
// If we can't ignore the instruction, return false.
405-
return false;
406-
}
407-
408-
// Otherwise, we have an unreachable and every instruction is inert in an
409-
// unreachable BB.
410-
return true;
411-
}
412-
413-
414339
/// Declared out of line so we can have a declaration of SILArgument.
415340
PhiArgumentArrayRef SILBasicBlock::getPhiArguments() const {
416341
return PhiArgumentArrayRef(getArguments(), [](SILArgument *arg) {

lib/SIL/SILInstructions.cpp

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,30 +1061,6 @@ bool TermInst::isFunctionExiting() const {
10611061
llvm_unreachable("Unhandled TermKind in switch.");
10621062
}
10631063

1064-
bool TermInst::isProgramTerminating() const {
1065-
switch (getTermKind()) {
1066-
case TermKind::BranchInst:
1067-
case TermKind::CondBranchInst:
1068-
case TermKind::SwitchValueInst:
1069-
case TermKind::SwitchEnumInst:
1070-
case TermKind::SwitchEnumAddrInst:
1071-
case TermKind::DynamicMethodBranchInst:
1072-
case TermKind::CheckedCastBranchInst:
1073-
case TermKind::CheckedCastValueBranchInst:
1074-
case TermKind::CheckedCastAddrBranchInst:
1075-
case TermKind::TryApplyInst:
1076-
case TermKind::YieldInst:
1077-
case TermKind::ReturnInst:
1078-
case TermKind::ThrowInst:
1079-
case TermKind::UnwindInst:
1080-
return false;
1081-
case TermKind::UnreachableInst:
1082-
return true;
1083-
}
1084-
1085-
llvm_unreachable("Unhandled TermKind in switch.");
1086-
}
1087-
10881064
TermInst::SuccessorBlockArgumentsListTy
10891065
TermInst::getSuccessorBlockArguments() const {
10901066
function_ref<PhiArgumentArrayRef(const SILSuccessor &)> op;

lib/SILOptimizer/Analysis/ARCAnalysis.cpp

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,6 +1087,78 @@ bool swift::getFinalReleasesForValue(SILValue V, ReleaseTracker &Tracker) {
10871087
return true;
10881088
}
10891089

1090+
//===----------------------------------------------------------------------===//
1091+
// Leaking BB Analysis
1092+
//===----------------------------------------------------------------------===//
1093+
1094+
static bool ignorableApplyInstInUnreachableBlock(const ApplyInst *AI) {
1095+
const auto *Fn = AI->getReferencedFunction();
1096+
if (!Fn)
1097+
return false;
1098+
1099+
return Fn->hasSemanticsAttr("arc.programtermination_point");
1100+
}
1101+
1102+
static bool ignorableBuiltinInstInUnreachableBlock(const BuiltinInst *BI) {
1103+
const BuiltinInfo &BInfo = BI->getBuiltinInfo();
1104+
if (BInfo.ID == BuiltinValueKind::CondUnreachable)
1105+
return true;
1106+
1107+
const IntrinsicInfo &IInfo = BI->getIntrinsicInfo();
1108+
if (IInfo.ID == llvm::Intrinsic::trap)
1109+
return true;
1110+
1111+
return false;
1112+
}
1113+
1114+
/// Match a call to a trap BB with no ARC relevant side effects.
1115+
bool swift::isARCInertTrapBB(const SILBasicBlock *BB) {
1116+
// Do a quick check at the beginning to make sure that our terminator is
1117+
// actually an unreachable. This ensures that in many cases this function will
1118+
// exit early and quickly.
1119+
auto II = BB->rbegin();
1120+
if (!isa<UnreachableInst>(*II))
1121+
return false;
1122+
1123+
auto IE = BB->rend();
1124+
while (II != IE) {
1125+
// Ignore any instructions without side effects.
1126+
if (!II->mayHaveSideEffects()) {
1127+
++II;
1128+
continue;
1129+
}
1130+
1131+
// Ignore cond fail.
1132+
if (isa<CondFailInst>(*II)) {
1133+
++II;
1134+
continue;
1135+
}
1136+
1137+
// Check for apply insts that we can ignore.
1138+
if (auto *AI = dyn_cast<ApplyInst>(&*II)) {
1139+
if (ignorableApplyInstInUnreachableBlock(AI)) {
1140+
++II;
1141+
continue;
1142+
}
1143+
}
1144+
1145+
// Check for builtins that we can ignore.
1146+
if (auto *BI = dyn_cast<BuiltinInst>(&*II)) {
1147+
if (ignorableBuiltinInstInUnreachableBlock(BI)) {
1148+
++II;
1149+
continue;
1150+
}
1151+
}
1152+
1153+
// If we can't ignore the instruction, return false.
1154+
return false;
1155+
}
1156+
1157+
// Otherwise, we have an unreachable and every instruction is inert from an
1158+
// ARC perspective in an unreachable BB.
1159+
return true;
1160+
}
1161+
10901162
//===----------------------------------------------------------------------===//
10911163
// Analysis of builtin "unsafeGuaranteed" instructions
10921164
//===----------------------------------------------------------------------===//

lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
#include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
1818
#include "swift/SILOptimizer/Analysis/FunctionOrder.h"
1919
#include "swift/SILOptimizer/PassManager/PassManager.h"
20-
#include "swift/Strings.h"
2120

2221
using namespace swift;
2322

@@ -322,7 +321,7 @@ FunctionSideEffectFlags *FunctionSideEffects::getEffectsOn(SILValue Addr) {
322321
// Return true if the given function has defined effects that were successfully
323322
// recorded in this FunctionSideEffects object.
324323
bool FunctionSideEffects::setDefinedEffects(SILFunction *F) {
325-
if (F->hasSemanticsAttr(SEMANTICS_PROGRAMTERMINATION_POINT)) {
324+
if (F->hasSemanticsAttr("arc.programtermination_point")) {
326325
Traps = true;
327326
return true;
328327
}

lib/SILOptimizer/Mandatory/DiagnoseInfiniteRecursion.cpp

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,10 @@
2222
#include "swift/SIL/CFG.h"
2323
#include "swift/SIL/SILArgument.h"
2424
#include "swift/SIL/SILInstruction.h"
25-
#include "swift/SILOptimizer/Analysis/ARCAnalysis.h"
2625
#include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
2726
#include "swift/SILOptimizer/PassManager/Passes.h"
2827
#include "swift/SILOptimizer/PassManager/Transforms.h"
2928
#include "swift/SILOptimizer/Utils/Devirtualize.h"
30-
#include "swift/Strings.h"
3129
#include "llvm/ADT/PostOrderIterator.h"
3230
#include "llvm/ADT/iterator_range.h"
3331
#include "llvm/Support/Debug.h"
@@ -111,14 +109,6 @@ static bool hasInfinitelyRecursiveApply(SILFunction *targetFn) {
111109
while (!workList.empty()) {
112110
SILBasicBlock *curBlock = workList.pop_back_val();
113111

114-
// If the block is "program terminating", meaning it traps without relevant
115-
// side effects, then skip it.
116-
// Note that this check happens _before_ the recursion check. This is
117-
// deliberate, as we want to ignore recursive calls inside a program
118-
// termination point.
119-
if (curBlock->isProgramTerminationPoint())
120-
continue;
121-
122112
// We're looking for functions that are recursive on _all_ paths. If this
123113
// block is recursive, mark that we found recursion and check the next
124114
// block in the work list.
@@ -129,8 +119,7 @@ static bool hasInfinitelyRecursiveApply(SILFunction *targetFn) {
129119

130120
// If this block doesn't have a recursive call, and it exits the function,
131121
// then we know the function is not infinitely recursive.
132-
TermInst *terminator = curBlock->getTerminator();
133-
if (terminator->isFunctionExiting() || terminator->isProgramTerminating())
122+
if (curBlock->getTerminator()->isFunctionExiting())
134123
return false;
135124

136125
// Otherwise, push the successors onto the stack if we haven't visited them.

lib/SILOptimizer/Transforms/ARCCodeMotion.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1074,7 +1074,7 @@ static void eliminateRetainsPrecedingProgramTerminationPoints(SILFunction *f) {
10741074
if (auto apply = FullApplySite::isa(&*iter)) {
10751075
SILFunction *callee = apply.getCalleeFunction();
10761076
if (!callee ||
1077-
!callee->hasSemanticsAttr(SEMANTICS_PROGRAMTERMINATION_POINT)) {
1077+
!callee->hasSemanticsAttr(SEMANTICS_ARC_PROGRAMTERMINATION_POINT)) {
10781078
continue;
10791079
}
10801080
} else {

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3515,7 +3515,7 @@ bool SimplifyCFG::simplifyProgramTerminationBlock(SILBasicBlock *BB) {
35153515
// use the analysis is because the CFG is likely to be invalidated right
35163516
// after this pass, o we do not really get the benefit of reusing the
35173517
// computation for the next iteration of the pass.
3518-
if (!BB->isProgramTerminationPoint())
3518+
if (!isARCInertTrapBB(BB))
35193519
return false;
35203520

35213521
// This is going to be the last basic block this program is going to execute

0 commit comments

Comments
 (0)