Skip to content

[SILOptimizer] Don't diagnose infinite recursion if a branch terminates the program #19724

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
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
4 changes: 2 additions & 2 deletions docs/ARCOptimization.rst
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ Semantic Tags
ARC takes advantage of certain semantic tags. This section documents these
semantics and their meanings.

arc.programtermination_point
programtermination_point
----------------------------

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

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

2. If we are able to infer that an object's lifetime scope would never end due
to the unreachable/no-return function, then we do not need to end the lifetime
Expand Down
5 changes: 4 additions & 1 deletion include/swift/SIL/SILBasicBlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,12 @@ public llvm::ilist_node<SILBasicBlock>, public SILAllocated<SILBasicBlock> {
/// no-return apply or builtin.
bool isNoReturn() const;

/// Returns true if this instruction only contains a branch instruction.
/// Returns true if this block only contains a branch instruction.
bool isTrampoline() const;

/// Returns true if this block traps without any side effects.
bool isProgramTerminationPoint() const;

/// Returns true if it is legal to hoist instructions into this block.
///
/// Used by llvm::LoopInfo.
Expand Down
3 changes: 3 additions & 0 deletions include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -6645,6 +6645,9 @@ class TermInst : public NonValueInstruction {
/// Returns true if this terminator exits the function.
bool isFunctionExiting() const;

/// Returns true if this terminator terminates the program.
bool isProgramTerminating() const;

TermKind getTermKind() const { return TermKind(getKind()); }
};

Expand Down
3 changes: 0 additions & 3 deletions include/swift/SILOptimizer/Analysis/ARCAnalysis.h
Original file line number Diff line number Diff line change
Expand Up @@ -424,9 +424,6 @@ class ReleaseTracker {
/// FinalRelease.
bool getFinalReleasesForValue(SILValue Value, ReleaseTracker &Tracker);

/// Match a call to a trap BB with no ARC relevant side effects.
bool isARCInertTrapBB(const SILBasicBlock *BB);

/// Get the two result values of the builtin "unsafeGuaranteed" instruction.
///
/// Gets the (GuaranteedValue, Token) tuple from a call to "unsafeGuaranteed"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class ProgramTerminationFunctionInfo {
public:
ProgramTerminationFunctionInfo(const SILFunction *F) {
for (const auto &BB : *F) {
if (!isARCInertTrapBB(&BB))
if (!BB.isProgramTerminationPoint())
continue;
ProgramTerminatingBlocks.insert(&BB);
}
Expand Down
4 changes: 2 additions & 2 deletions include/swift/Strings.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ constexpr static const char BUILTIN_TYPE_NAME_VEC[] = "Builtin.Vec";
constexpr static const char BUILTIN_TYPE_NAME_SILTOKEN[] = "Builtin.SILToken";
/// The name of the Builtin type for Word
constexpr static const char BUILTIN_TYPE_NAME_WORD[] = "Builtin.Word";
constexpr static StringLiteral SEMANTICS_ARC_PROGRAMTERMINATION_POINT =
"arc.programtermination_point";
constexpr static StringLiteral SEMANTICS_PROGRAMTERMINATION_POINT =
"programtermination_point";
} // end namespace swift

#endif // SWIFT_STRINGS_H
75 changes: 75 additions & 0 deletions lib/SIL/SILBasicBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "swift/SIL/SILFunction.h"
#include "swift/SIL/SILInstruction.h"
#include "swift/SIL/SILModule.h"
#include "swift/Strings.h"
using namespace swift;

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -336,6 +337,80 @@ bool SILBasicBlock::isEntry() const {
return this == &*getParent()->begin();
}

//===----------------------------------------------------------------------===//
// Program Termination Point Analysis
//===----------------------------------------------------------------------===//

static bool ignorableApplyInstInUnreachableBlock(const ApplyInst *AI) {
const auto *Fn = AI->getReferencedFunction();
if (!Fn)
return false;

return Fn->hasSemanticsAttr(SEMANTICS_PROGRAMTERMINATION_POINT);
}

static bool ignorableBuiltinInstInUnreachableBlock(const BuiltinInst *BI) {
const BuiltinInfo &BInfo = BI->getBuiltinInfo();
if (BInfo.ID == BuiltinValueKind::CondUnreachable)
return true;

const IntrinsicInfo &IInfo = BI->getIntrinsicInfo();
if (IInfo.ID == llvm::Intrinsic::trap)
return true;

return false;
}

// Match a call to a basic block that's going to terminate
// with no side effects.
bool SILBasicBlock::isProgramTerminationPoint() const {
// Do a quick check at the beginning to make sure that our terminator is
// actually an unreachable. This ensures that in many cases this function will
// exit early and quickly.
auto II = rbegin();
if (!isa<UnreachableInst>(*II))
return false;

auto IE = rend();
while (II != IE) {
// Ignore any instructions without side effects.
if (!II->mayHaveSideEffects()) {
++II;
continue;
}

// Ignore cond fail.
if (isa<CondFailInst>(*II)) {
++II;
continue;
}

// Check for apply insts that we can ignore.
if (auto *AI = dyn_cast<ApplyInst>(&*II)) {
if (ignorableApplyInstInUnreachableBlock(AI)) {
++II;
continue;
}
}

// Check for builtins that we can ignore.
if (auto *BI = dyn_cast<BuiltinInst>(&*II)) {
if (ignorableBuiltinInstInUnreachableBlock(BI)) {
++II;
continue;
}
}

// If we can't ignore the instruction, return false.
return false;
}

// Otherwise, we have an unreachable and every instruction is inert in an
// unreachable BB.
return true;
}


/// Declared out of line so we can have a declaration of SILArgument.
PhiArgumentArrayRef SILBasicBlock::getPhiArguments() const {
return PhiArgumentArrayRef(getArguments(), [](SILArgument *arg) {
Expand Down
24 changes: 24 additions & 0 deletions lib/SIL/SILInstructions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,30 @@ bool TermInst::isFunctionExiting() const {
llvm_unreachable("Unhandled TermKind in switch.");
}

bool TermInst::isProgramTerminating() const {
switch (getTermKind()) {
case TermKind::BranchInst:
case TermKind::CondBranchInst:
case TermKind::SwitchValueInst:
case TermKind::SwitchEnumInst:
case TermKind::SwitchEnumAddrInst:
case TermKind::DynamicMethodBranchInst:
case TermKind::CheckedCastBranchInst:
case TermKind::CheckedCastValueBranchInst:
case TermKind::CheckedCastAddrBranchInst:
case TermKind::TryApplyInst:
case TermKind::YieldInst:
case TermKind::ReturnInst:
case TermKind::ThrowInst:
case TermKind::UnwindInst:
return false;
case TermKind::UnreachableInst:
return true;
}

llvm_unreachable("Unhandled TermKind in switch.");
}

TermInst::SuccessorBlockArgumentsListTy
TermInst::getSuccessorBlockArguments() const {
function_ref<PhiArgumentArrayRef(const SILSuccessor &)> op;
Expand Down
72 changes: 0 additions & 72 deletions lib/SILOptimizer/Analysis/ARCAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1087,78 +1087,6 @@ bool swift::getFinalReleasesForValue(SILValue V, ReleaseTracker &Tracker) {
return true;
}

//===----------------------------------------------------------------------===//
// Leaking BB Analysis
//===----------------------------------------------------------------------===//

static bool ignorableApplyInstInUnreachableBlock(const ApplyInst *AI) {
const auto *Fn = AI->getReferencedFunction();
if (!Fn)
return false;

return Fn->hasSemanticsAttr("arc.programtermination_point");
}

static bool ignorableBuiltinInstInUnreachableBlock(const BuiltinInst *BI) {
const BuiltinInfo &BInfo = BI->getBuiltinInfo();
if (BInfo.ID == BuiltinValueKind::CondUnreachable)
return true;

const IntrinsicInfo &IInfo = BI->getIntrinsicInfo();
if (IInfo.ID == llvm::Intrinsic::trap)
return true;

return false;
}

/// Match a call to a trap BB with no ARC relevant side effects.
bool swift::isARCInertTrapBB(const SILBasicBlock *BB) {
// Do a quick check at the beginning to make sure that our terminator is
// actually an unreachable. This ensures that in many cases this function will
// exit early and quickly.
auto II = BB->rbegin();
if (!isa<UnreachableInst>(*II))
return false;

auto IE = BB->rend();
while (II != IE) {
// Ignore any instructions without side effects.
if (!II->mayHaveSideEffects()) {
++II;
continue;
}

// Ignore cond fail.
if (isa<CondFailInst>(*II)) {
++II;
continue;
}

// Check for apply insts that we can ignore.
if (auto *AI = dyn_cast<ApplyInst>(&*II)) {
if (ignorableApplyInstInUnreachableBlock(AI)) {
++II;
continue;
}
}

// Check for builtins that we can ignore.
if (auto *BI = dyn_cast<BuiltinInst>(&*II)) {
if (ignorableBuiltinInstInUnreachableBlock(BI)) {
++II;
continue;
}
}

// If we can't ignore the instruction, return false.
return false;
}

// Otherwise, we have an unreachable and every instruction is inert from an
// ARC perspective in an unreachable BB.
return true;
}

//===----------------------------------------------------------------------===//
// Analysis of builtin "unsafeGuaranteed" instructions
//===----------------------------------------------------------------------===//
Expand Down
3 changes: 2 additions & 1 deletion lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
#include "swift/SILOptimizer/Analysis/FunctionOrder.h"
#include "swift/SILOptimizer/PassManager/PassManager.h"
#include "swift/Strings.h"

using namespace swift;

Expand Down Expand Up @@ -321,7 +322,7 @@ FunctionSideEffectFlags *FunctionSideEffects::getEffectsOn(SILValue Addr) {
// Return true if the given function has defined effects that were successfully
// recorded in this FunctionSideEffects object.
bool FunctionSideEffects::setDefinedEffects(SILFunction *F) {
if (F->hasSemanticsAttr("arc.programtermination_point")) {
if (F->hasSemanticsAttr(SEMANTICS_PROGRAMTERMINATION_POINT)) {
Traps = true;
return true;
}
Expand Down
13 changes: 12 additions & 1 deletion lib/SILOptimizer/Mandatory/DiagnoseInfiniteRecursion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
#include "swift/SIL/CFG.h"
#include "swift/SIL/SILArgument.h"
#include "swift/SIL/SILInstruction.h"
#include "swift/SILOptimizer/Analysis/ARCAnalysis.h"
#include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
#include "swift/SILOptimizer/PassManager/Passes.h"
#include "swift/SILOptimizer/PassManager/Transforms.h"
#include "swift/SILOptimizer/Utils/Devirtualize.h"
#include "swift/Strings.h"
#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/Support/Debug.h"
Expand Down Expand Up @@ -109,6 +111,14 @@ static bool hasInfinitelyRecursiveApply(SILFunction *targetFn) {
while (!workList.empty()) {
SILBasicBlock *curBlock = workList.pop_back_val();

// If the block is "program terminating", meaning it traps without relevant
// side effects, then skip it.
// Note that this check happens _before_ the recursion check. This is
// deliberate, as we want to ignore recursive calls inside a program
// termination point.
if (curBlock->isProgramTerminationPoint())
continue;

// We're looking for functions that are recursive on _all_ paths. If this
// block is recursive, mark that we found recursion and check the next
// block in the work list.
Expand All @@ -119,7 +129,8 @@ static bool hasInfinitelyRecursiveApply(SILFunction *targetFn) {

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

// Otherwise, push the successors onto the stack if we haven't visited them.
Expand Down
2 changes: 1 addition & 1 deletion lib/SILOptimizer/Transforms/ARCCodeMotion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,7 @@ static void eliminateRetainsPrecedingProgramTerminationPoints(SILFunction *f) {
if (auto apply = FullApplySite::isa(&*iter)) {
SILFunction *callee = apply.getCalleeFunction();
if (!callee ||
!callee->hasSemanticsAttr(SEMANTICS_ARC_PROGRAMTERMINATION_POINT)) {
!callee->hasSemanticsAttr(SEMANTICS_PROGRAMTERMINATION_POINT)) {
continue;
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion lib/SILOptimizer/Transforms/SimplifyCFG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3515,7 +3515,7 @@ bool SimplifyCFG::simplifyProgramTerminationBlock(SILBasicBlock *BB) {
// use the analysis is because the CFG is likely to be invalidated right
// after this pass, o we do not really get the benefit of reusing the
// computation for the next iteration of the pass.
if (!isARCInertTrapBB(BB))
if (!BB->isProgramTerminationPoint())
return false;

// This is going to be the last basic block this program is going to execute
Expand Down
Loading