Skip to content

Commit 628bf7b

Browse files
authored
Merge pull request swiftlang#30957 from eeckstein/fix-large-string-arrays
Several compile time fixes related to large string arrays.
2 parents 365f0c7 + e117378 commit 628bf7b

25 files changed

+323
-138
lines changed

include/swift/AST/SILOptions.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ class SILOptions {
7373
/// Controls whether or not paranoid verification checks are run.
7474
bool VerifyAll = false;
7575

76+
/// If true, no SIL verification is done at all.
77+
bool VerifyNone = false;
78+
7679
/// Are we debugging sil serialization.
7780
bool DebugSerialization = false;
7881

include/swift/Option/FrontendOptions.td

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,9 @@ def sil_merge_partial_modules : Flag<["-"], "sil-merge-partial-modules">,
550550
def sil_verify_all : Flag<["-"], "sil-verify-all">,
551551
HelpText<"Verify SIL after each transform">;
552552

553+
def sil_verify_none : Flag<["-"], "sil-verify-none">,
554+
HelpText<"Completely disable SIL verification">;
555+
553556
def verify_all_substitution_maps : Flag<["-"], "verify-all-substitution-maps">,
554557
HelpText<"Verify all SubstitutionMaps on construction">;
555558

include/swift/SIL/SILBasicBlock.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ public llvm::ilist_node<SILBasicBlock>, public SILAllocated<SILBasicBlock> {
110110
InstList.splice(end(), Other->InstList);
111111
}
112112

113+
void spliceAtBegin(SILBasicBlock *Other) {
114+
InstList.splice(begin(), Other->InstList);
115+
}
116+
113117
bool empty() const { return InstList.empty(); }
114118
iterator begin() { return InstList.begin(); }
115119
iterator end() { return InstList.end(); }
@@ -202,6 +206,8 @@ public llvm::ilist_node<SILBasicBlock>, public SILAllocated<SILBasicBlock> {
202206

203207
void cloneArgumentList(SILBasicBlock *Other);
204208

209+
void moveArgumentList(SILBasicBlock *from);
210+
205211
/// Erase a specific argument from the arg list.
206212
void eraseArgument(int Index);
207213

include/swift/SIL/SILInstructionWorklist.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ class SILInstructionWorklist : SILInstructionWorklistBase {
7272
/// Returns true if the worklist is empty.
7373
bool isEmpty() const { return worklist.empty(); }
7474

75+
/// Returns the number of elements in the worklist.
76+
unsigned size() const { return worklist.size(); }
77+
7578
/// Add the specified instruction to the worklist if it isn't already in it.
7679
void add(SILInstruction *instruction) {
7780
if (worklist.insert(instruction).second) {

include/swift/SILOptimizer/Analysis/EscapeAnalysis.h

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,9 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
652652
/// True if this is a summary graph.
653653
bool isSummaryGraph;
654654

655+
/// True if the graph could be computed.
656+
bool valid = true;
657+
655658
/// Track the currently active intrusive worklist -- one at a time.
656659
CGNodeWorklist *activeWorklist = nullptr;
657660

@@ -859,6 +862,22 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
859862
bool forwardTraverseDefer(CGNode *startNode, CGNodeVisitor &&visitor);
860863

861864
public:
865+
866+
/// Returns true if the graph could be computed.
867+
///
868+
/// For very large functions (> 10000 nodes), graphs are not cumputed to
869+
/// avoid quadratic complexity of the node merging algorithm.
870+
bool isValid() const {
871+
assert((valid || isEmpty()) && "invalid graph must not contain nodes");
872+
return valid;
873+
}
874+
875+
/// Invalides the graph in case it's getting too large.
876+
void invalidate() {
877+
clear();
878+
valid = false;
879+
}
880+
862881
/// Get the content node pointed to by \p ptrVal.
863882
///
864883
/// If \p ptrVal cannot be mapped to a node, return nullptr.
@@ -1045,8 +1064,7 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
10451064
/// If \p ai is an optimizable @_semantics("array.uninitialized") call, return
10461065
/// valid call information.
10471066
ArrayUninitCall
1048-
canOptimizeArrayUninitializedCall(ApplyInst *ai,
1049-
const ConnectionGraph *conGraph);
1067+
canOptimizeArrayUninitializedCall(ApplyInst *ai);
10501068

10511069
/// Return true of this tuple_extract is the result of an optimizable
10521070
/// @_semantics("array.uninitialized") call.

lib/Frontend/CompilerInvocation.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -984,6 +984,7 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
984984
Opts.DisableSILPerfOptimizations |= Args.hasArg(OPT_disable_sil_perf_optzns);
985985
Opts.CrossModuleOptimization |= Args.hasArg(OPT_CrossModuleOptimization);
986986
Opts.VerifyAll |= Args.hasArg(OPT_sil_verify_all);
987+
Opts.VerifyNone |= Args.hasArg(OPT_sil_verify_none);
987988
Opts.DebugSerialization |= Args.hasArg(OPT_sil_debug_serialization);
988989
Opts.EmitVerboseSIL |= Args.hasArg(OPT_emit_verbose_sil);
989990
Opts.EmitSortedSIL |= Args.hasArg(OPT_emit_sorted_sil);

lib/SIL/IR/SILBasicBlock.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,13 @@ void SILBasicBlock::cloneArgumentList(SILBasicBlock *Other) {
142142
}
143143
}
144144

145+
void SILBasicBlock::moveArgumentList(SILBasicBlock *from) {
146+
ArgumentList = std::move(from->ArgumentList);
147+
for (SILArgument *arg : getArguments()) {
148+
arg->parentBlock = this;
149+
}
150+
}
151+
145152
SILFunctionArgument *
146153
SILBasicBlock::createFunctionArgument(SILType Ty, const ValueDecl *D,
147154
bool disableEntryBlockVerification) {

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5232,33 +5232,37 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
52325232
// Out of Line Verifier Run Functions
52335233
//===----------------------------------------------------------------------===//
52345234

5235+
static bool verificationEnabled(const SILModule &M) {
5236+
#ifdef NDEBUG
5237+
if (!M.getOptions().VerifyAll)
5238+
return false;
5239+
#endif
5240+
return !M.getOptions().VerifyNone;
5241+
}
5242+
52355243
/// verify - Run the SIL verifier to make sure that the SILFunction follows
52365244
/// invariants.
52375245
void SILFunction::verify(bool SingleFunction) const {
5238-
#ifdef NDEBUG
5239-
if (!getModule().getOptions().VerifyAll)
5246+
if (!verificationEnabled(getModule()))
52405247
return;
5241-
#endif
5248+
52425249
// Please put all checks in visitSILFunction in SILVerifier, not here. This
52435250
// ensures that the pretty stack trace in the verifier is included with the
52445251
// back trace when the verifier crashes.
52455252
SILVerifier(*this, SingleFunction).verify();
52465253
}
52475254

52485255
void SILFunction::verifyCriticalEdges() const {
5249-
#ifdef NDEBUG
5250-
if (!getModule().getOptions().VerifyAll)
5256+
if (!verificationEnabled(getModule()))
52515257
return;
5252-
#endif
5258+
52535259
SILVerifier(*this, /*SingleFunction=*/true).verifyBranches(this);
52545260
}
52555261

52565262
/// Verify that a property descriptor follows invariants.
52575263
void SILProperty::verify(const SILModule &M) const {
5258-
#ifdef NDEBUG
5259-
if (!M.getOptions().VerifyAll)
5264+
if (!verificationEnabled(M))
52605265
return;
5261-
#endif
52625266

52635267
auto *decl = getDecl();
52645268
auto *dc = decl->getInnermostDeclContext();
@@ -5310,10 +5314,9 @@ void SILProperty::verify(const SILModule &M) const {
53105314

53115315
/// Verify that a vtable follows invariants.
53125316
void SILVTable::verify(const SILModule &M) const {
5313-
#ifdef NDEBUG
5314-
if (!M.getOptions().VerifyAll)
5317+
if (!verificationEnabled(M))
53155318
return;
5316-
#endif
5319+
53175320
for (auto &entry : getEntries()) {
53185321
// All vtable entries must be decls in a class context.
53195322
assert(entry.Method.hasDecl() && "vtable entry is not a decl");
@@ -5361,10 +5364,9 @@ void SILVTable::verify(const SILModule &M) const {
53615364

53625365
/// Verify that a witness table follows invariants.
53635366
void SILWitnessTable::verify(const SILModule &M) const {
5364-
#ifdef NDEBUG
5365-
if (!M.getOptions().VerifyAll)
5367+
if (!verificationEnabled(M))
53665368
return;
5367-
#endif
5369+
53685370
if (isDeclaration())
53695371
assert(getEntries().empty() &&
53705372
"A witness table declaration should not have any entries.");
@@ -5414,10 +5416,9 @@ void SILDefaultWitnessTable::verify(const SILModule &M) const {
54145416

54155417
/// Verify that a global variable follows invariants.
54165418
void SILGlobalVariable::verify() const {
5417-
#ifdef NDEBUG
5418-
if (!getModule().getOptions().VerifyAll)
5419+
if (!verificationEnabled(getModule()))
54195420
return;
5420-
#endif
5421+
54215422
assert(getLoweredType().isObject()
54225423
&& "global variable cannot have address type");
54235424

@@ -5439,10 +5440,9 @@ void SILGlobalVariable::verify() const {
54395440

54405441
/// Verify the module.
54415442
void SILModule::verify() const {
5442-
#ifdef NDEBUG
5443-
if (!getOptions().VerifyAll)
5443+
if (!verificationEnabled(*this))
54445444
return;
5445-
#endif
5445+
54465446
// Uniquing set to catch symbol name collisions.
54475447
llvm::DenseSet<StringRef> symbolNames;
54485448

lib/SILOptimizer/Analysis/AliasAnalysis.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,16 @@ AliasResult AliasAnalysis::aliasInner(SILValue V1, SILValue V2,
661661
}
662662

663663
bool AliasAnalysis::canApplyDecrementRefCount(FullApplySite FAS, SILValue Ptr) {
664+
// If the connection graph is invalid due to a very large function, we also
665+
// skip all other tests, which might take significant time for a very large
666+
// function.
667+
// This is a workaround for some quadratic complexity in ARCSequenceOpt.
668+
// TODO: remove this check once ARCSequenceOpt is retired or the quadratic
669+
// behavior is fixed.
670+
auto *conGraph = EA->getConnectionGraph(FAS.getFunction());
671+
if (!conGraph->isValid())
672+
return true;
673+
664674
// Treat applications of no-return functions as decrementing ref counts. This
665675
// causes the apply to become a sink barrier for ref count increments.
666676
if (FAS.isCalleeNoReturn())

lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,7 @@ void EscapeAnalysis::ConnectionGraph::clear() {
403403
UsePoints.clear();
404404
UsePointTable.clear();
405405
NodeAllocator.DestroyAll();
406+
valid = true;
406407
assert(ToMerge.empty());
407408
}
408409

@@ -416,6 +417,9 @@ void EscapeAnalysis::ConnectionGraph::clear() {
416417
// it's interior property.
417418
EscapeAnalysis::CGNode *
418419
EscapeAnalysis::ConnectionGraph::getNode(SILValue V) {
420+
if (!isValid())
421+
return nullptr;
422+
419423
// Early filter obvious non-pointer opcodes.
420424
if (isa<FunctionRefInst>(V) || isa<DynamicFunctionRefInst>(V) ||
421425
isa<PreviousDynamicFunctionRefInst>(V))
@@ -1028,6 +1032,9 @@ CGNode *EscapeAnalysis::ConnectionGraph::getReturnNode() {
10281032

10291033
bool EscapeAnalysis::ConnectionGraph::mergeFrom(ConnectionGraph *SourceGraph,
10301034
CGNodeMap &Mapping) {
1035+
assert(isValid());
1036+
assert(SourceGraph->isValid());
1037+
10311038
// The main point of the merging algorithm is to map each content node in the
10321039
// source graph to a content node in this (destination) graph. This may
10331040
// require creating new nodes or merging existing nodes in this graph.
@@ -1492,6 +1499,11 @@ void EscapeAnalysis::ConnectionGraph::print(llvm::raw_ostream &OS) const {
14921499
#ifndef NDEBUG
14931500
OS << "CG of " << F->getName() << '\n';
14941501

1502+
if (!isValid()) {
1503+
OS << " invalid\n";
1504+
return;
1505+
}
1506+
14951507
// Assign the same IDs to SILValues as the SILPrinter does.
14961508
llvm::DenseMap<const SILNode *, unsigned> InstToIDMap;
14971509
InstToIDMap[nullptr] = (unsigned)-1;
@@ -1595,6 +1607,7 @@ void EscapeAnalysis::ConnectionGraph::verify() const {
15951607
// Invalidating EscapeAnalysis clears the connection graph.
15961608
if (isEmpty())
15971609
return;
1610+
assert(isValid());
15981611

15991612
verifyStructure();
16001613

@@ -1608,7 +1621,7 @@ void EscapeAnalysis::ConnectionGraph::verify() const {
16081621
continue;
16091622

16101623
if (auto ai = dyn_cast<ApplyInst>(&i)) {
1611-
if (EA->canOptimizeArrayUninitializedCall(ai, this).isValid())
1624+
if (EA->canOptimizeArrayUninitializedCall(ai).isValid())
16121625
continue;
16131626
}
16141627
for (auto result : i.getResults()) {
@@ -1728,10 +1741,21 @@ void EscapeAnalysis::buildConnectionGraph(FunctionInfo *FInfo,
17281741
// Create edges for the instructions.
17291742
for (auto &i : *bb) {
17301743
analyzeInstruction(&i, FInfo, BottomUpOrder, RecursionDepth);
1744+
1745+
// Bail if the graph gets too big. The node merging algorithm has
1746+
// quadratic complexity and we want to avoid this.
1747+
// TODO: fix the quadratic complexity (if possible) and remove this limit.
1748+
if (ConGraph->Nodes.size() > 10000) {
1749+
ConGraph->invalidate();
1750+
return false;
1751+
}
17311752
}
17321753
return true;
17331754
});
17341755

1756+
if (!ConGraph->isValid())
1757+
return;
1758+
17351759
// Second step: create defer-edges for block arguments.
17361760
for (SILBasicBlock &BB : *ConGraph->F) {
17371761
if (!reachable.isVisited(&BB))
@@ -1826,8 +1850,7 @@ bool EscapeAnalysis::buildConnectionGraphForDestructor(
18261850
}
18271851

18281852
EscapeAnalysis::ArrayUninitCall
1829-
EscapeAnalysis::canOptimizeArrayUninitializedCall(
1830-
ApplyInst *ai, const ConnectionGraph *conGraph) {
1853+
EscapeAnalysis::canOptimizeArrayUninitializedCall(ApplyInst *ai) {
18311854
ArrayUninitCall call;
18321855
// This must be an exact match so we don't accidentally optimize
18331856
// "array.uninitialized_intrinsic".
@@ -1873,8 +1896,7 @@ bool EscapeAnalysis::canOptimizeArrayUninitializedResult(
18731896
if (!ai)
18741897
return false;
18751898

1876-
auto *conGraph = getConnectionGraph(ai->getFunction());
1877-
return canOptimizeArrayUninitializedCall(ai, conGraph).isValid();
1899+
return canOptimizeArrayUninitializedCall(ai).isValid();
18781900
}
18791901

18801902
// Handle @_semantics("array.uninitialized")
@@ -1924,7 +1946,7 @@ void EscapeAnalysis::analyzeInstruction(SILInstruction *I,
19241946
return;
19251947
case ArrayCallKind::kArrayUninitialized: {
19261948
ArrayUninitCall call = canOptimizeArrayUninitializedCall(
1927-
cast<ApplyInst>(FAS.getInstruction()), ConGraph);
1949+
cast<ApplyInst>(FAS.getInstruction()));
19281950
if (call.isValid()) {
19291951
createArrayUninitializedSubgraph(call, ConGraph);
19301952
return;
@@ -2480,6 +2502,16 @@ void EscapeAnalysis::recompute(FunctionInfo *Initial) {
24802502
bool EscapeAnalysis::mergeCalleeGraph(SILInstruction *AS,
24812503
ConnectionGraph *CallerGraph,
24822504
ConnectionGraph *CalleeGraph) {
2505+
if (!CallerGraph->isValid())
2506+
return false;
2507+
2508+
if (!CalleeGraph->isValid()) {
2509+
setAllEscaping(AS, CallerGraph);
2510+
// Conservatively assume that setting that setAllEscaping(AS) did change the
2511+
// graph.
2512+
return true;
2513+
}
2514+
24832515
// This CGNodeMap uses an intrusive worklist to keep track of Mapped nodes
24842516
// from the CalleeGraph. Meanwhile, mergeFrom uses separate intrusive
24852517
// worklists to update nodes in the CallerGraph.
@@ -2538,6 +2570,11 @@ bool EscapeAnalysis::mergeCalleeGraph(SILInstruction *AS,
25382570

25392571
bool EscapeAnalysis::mergeSummaryGraph(ConnectionGraph *SummaryGraph,
25402572
ConnectionGraph *Graph) {
2573+
if (!Graph->isValid()) {
2574+
bool changed = SummaryGraph->isValid();
2575+
SummaryGraph->invalidate();
2576+
return changed;
2577+
}
25412578

25422579
// Make a 1-to-1 mapping of all arguments and the return value. This CGNodeMap
25432580
// node map uses an intrusive worklist to keep track of Mapped nodes from the

lib/SILOptimizer/SILCombiner/SILCombine.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,9 @@ class SILCombineCanonicalize final : CanonicalizeInstruction {
131131
}
132132

133133
void notifyHasNewUsers(SILValue value) override {
134-
Worklist.addUsersToWorklist(value);
134+
if (Worklist.size() < 10000) {
135+
Worklist.addUsersToWorklist(value);
136+
}
135137
changed = true;
136138
}
137139

0 commit comments

Comments
 (0)