Skip to content

Commit 4d097da

Browse files
slavapestovslavapestov
committed
RequirementMachine: Re-use requirement machines constructed by minimization for queries
Fixes rdar://problem/88135641.
1 parent 3d2559f commit 4d097da

28 files changed

+211
-65
lines changed

‎include/swift/AST/GenericSignature.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -535,13 +535,16 @@ GenericSignature buildGenericSignature(
535535

536536
/// Summary of error conditions detected by the Requirement Machine.
537537
enum class GenericSignatureErrorFlags {
538-
/// The original requirements referenced a non-existent type parameter.
539-
HasUnresolvedType = (1<<0),
540-
541-
/// The original requirements were in conflict with each other, meaning
538+
/// The original requirements referenced a non-existent type parameter,
539+
/// or the original requirements were in conflict with each other, meaning
542540
/// there are no possible concrete substitutions which statisfy the
543541
/// generic signature.
544-
HasConflict = (1<<1),
542+
HasInvalidRequirements = (1<<0),
543+
544+
/// The generic signature had non-redundant concrete conformance
545+
/// requirements, which means the rewrite system used for minimization
546+
/// must be discarded and a new one built for queries.
547+
HasConcreteConformances = (1<<1),
545548

546549
/// The Knuth-Bendix completion procedure failed to construct a confluent
547550
/// rewrite system.

‎lib/AST/GenericSignature.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1067,7 +1067,9 @@ void swift::validateGenericSignature(ASTContext &context,
10671067
GenericSignatureWithError());
10681068

10691069
// If there were any errors, the signature was invalid.
1070-
if (newSigWithError.getInt()) {
1070+
auto errorFlags = newSigWithError.getInt();
1071+
if (errorFlags.contains(GenericSignatureErrorFlags::HasInvalidRequirements) ||
1072+
errorFlags.contains(GenericSignatureErrorFlags::CompletionFailed)) {
10711073
context.Diags.diagnose(SourceLoc(), diag::generic_signature_not_valid,
10721074
sig->getAsString());
10731075
}

‎lib/AST/GenericSignatureBuilder.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8275,7 +8275,7 @@ AbstractGenericSignatureRequest::evaluate(
82758275
if (!rqmResult.getPointer() && !gsbResult.getPointer())
82768276
return rqmResult;
82778277

8278-
if (!rqmResult.getInt().contains(GenericSignatureErrorFlags::HasConflict) &&
8278+
if (!rqmResult.getInt().contains(GenericSignatureErrorFlags::HasInvalidRequirements) &&
82798279
!rqmResult.getInt().contains(GenericSignatureErrorFlags::CompletionFailed) &&
82808280
!rqmResult.getPointer()->isEqual(gsbResult.getPointer())) {
82818281
PrintOptions opts;
@@ -8427,7 +8427,7 @@ AbstractGenericSignatureRequestGSB::evaluate(
84278427

84288428
GenericSignatureErrors errorFlags;
84298429
if (builder.hadAnyError())
8430-
errorFlags |= GenericSignatureErrorFlags::HasUnresolvedType;
8430+
errorFlags |= GenericSignatureErrorFlags::HasInvalidRequirements;
84318431
auto result = std::move(builder).computeGenericSignature(
84328432
/*allowConcreteGenericParams=*/true);
84338433
return GenericSignatureWithError(result, errorFlags);
@@ -8489,7 +8489,7 @@ InferredGenericSignatureRequest::evaluate(
84898489
if (!rqmResult.getPointer() && !gsbResult.getPointer())
84908490
return rqmResult;
84918491

8492-
if (!rqmResult.getInt().contains(GenericSignatureErrorFlags::HasConflict) &&
8492+
if (!rqmResult.getInt().contains(GenericSignatureErrorFlags::HasInvalidRequirements) &&
84938493
!rqmResult.getInt().contains(GenericSignatureErrorFlags::CompletionFailed) &&
84948494
!rqmResult.getPointer()->isEqual(gsbResult.getPointer())) {
84958495
PrintOptions opts;
@@ -8635,7 +8635,7 @@ InferredGenericSignatureRequestGSB::evaluate(
86358635

86368636
GenericSignatureErrors errorFlags;
86378637
if (builder.hadAnyError())
8638-
errorFlags |= GenericSignatureErrorFlags::HasUnresolvedType;
8638+
errorFlags |= GenericSignatureErrorFlags::HasInvalidRequirements;
86398639
auto result = std::move(builder).computeGenericSignature(
86408640
allowConcreteGenericParams);
86418641
return GenericSignatureWithError(result, errorFlags);
@@ -8705,7 +8705,7 @@ RequirementSignatureRequest::evaluate(Evaluator &evaluator,
87058705
auto rqmResult = buildViaRQM();
87068706
auto gsbResult = buildViaGSB();
87078707

8708-
if (!rqmResult.getErrors().contains(GenericSignatureErrorFlags::HasConflict) &&
8708+
if (!rqmResult.getErrors().contains(GenericSignatureErrorFlags::HasInvalidRequirements) &&
87098709
!rqmResult.getErrors().contains(GenericSignatureErrorFlags::CompletionFailed) &&
87108710
!compare(rqmResult.getRequirements(),
87118711
gsbResult.getRequirements())) {

‎lib/AST/RequirementMachine/HomotopyReduction.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,7 @@ void RewriteSystem::minimizeRewriteSystem() {
471471

472472
assert(Complete);
473473
assert(!Minimized);
474+
assert(!Frozen);
474475
Minimized = 1;
475476

476477
propagateExplicitBits();
@@ -626,7 +627,7 @@ GenericSignatureErrors RewriteSystem::getErrors() const {
626627

627628
GenericSignatureErrors result;
628629

629-
for (const auto &rule : Rules) {
630+
for (const auto &rule : getLocalRules()) {
630631
if (rule.isPermanent())
631632
continue;
632633

@@ -636,10 +637,15 @@ GenericSignatureErrors RewriteSystem::getErrors() const {
636637
if (!rule.isRedundant() &&
637638
!rule.isProtocolTypeAliasRule() &&
638639
rule.containsUnresolvedSymbols())
639-
result |= GenericSignatureErrorFlags::HasUnresolvedType;
640+
result |= GenericSignatureErrorFlags::HasInvalidRequirements;
640641

641642
if (rule.isConflicting())
642-
result |= GenericSignatureErrorFlags::HasConflict;
643+
result |= GenericSignatureErrorFlags::HasInvalidRequirements;
644+
645+
if (!rule.isRedundant())
646+
if (auto property = rule.isPropertyRule())
647+
if (property->getKind() == Symbol::Kind::ConcreteConformance)
648+
result |= GenericSignatureErrorFlags::HasConcreteConformances;
643649
}
644650

645651
return result;

‎lib/AST/RequirementMachine/KnuthBendix.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ RewriteSystem::computeConfluentCompletion(unsigned maxRuleCount,
287287
unsigned maxRuleLength) {
288288
assert(Initialized);
289289
assert(!Minimized);
290+
assert(!Frozen);
290291

291292
// Complete might already be set, if we're re-running completion after
292293
// adding new rules in the property map's concrete type unification procedure.

‎lib/AST/RequirementMachine/RequirementMachine.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,10 @@ RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy) {
332332
return std::make_pair(CompletionResult::Success, 0);
333333
}
334334

335+
void RequirementMachine::freeze() {
336+
System.freeze();
337+
}
338+
335339
std::string RequirementMachine::getRuleAsStringForDiagnostics(
336340
unsigned ruleID) const {
337341
const auto &rule = System.getRule(ruleID);

‎lib/AST/RequirementMachine/RequirementMachine.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ class RequirementMachine final {
113113
std::pair<CompletionResult, unsigned>
114114
computeCompletion(RewriteSystem::ValidityPolicy policy);
115115

116+
void freeze();
117+
116118
MutableTerm getLongestValidPrefix(const MutableTerm &term) const;
117119

118120
void buildRequirementsFromRules(
@@ -159,8 +161,8 @@ class RequirementMachine final {
159161
llvm::DenseMap<const ProtocolDecl *, RequirementSignature>
160162
computeMinimalProtocolRequirements();
161163

162-
std::vector<Requirement>
163-
computeMinimalGenericSignatureRequirements(bool reconstituteSugar);
164+
GenericSignature
165+
computeMinimalGenericSignature(bool reconstituteSugar);
164166

165167
ArrayRef<Rule> getLocalRules() const;
166168

‎lib/AST/RequirementMachine/RequirementMachineRequests.cpp

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,8 @@ RequirementSignatureRequestRQM::evaluate(Evaluator &evaluator,
288288

289289
auto minimalRequirements = machine->computeMinimalProtocolRequirements();
290290

291-
if (!machine->getErrors()) {
291+
if (!machine->getErrors().contains(
292+
GenericSignatureErrorFlags::HasInvalidRequirements)) {
292293
if (shouldSplitConcreteEquivalenceClasses(minimalRequirements, machine.get())) {
293294
++attempt;
294295
splitConcreteEquivalenceClasses(ctx, minimalRequirements,
@@ -345,15 +346,20 @@ RequirementSignatureRequestRQM::evaluate(Evaluator &evaluator,
345346
/*allowConcreteGenericParams=*/false);
346347
}
347348

349+
if (!machine->getErrors())
350+
rewriteCtx.installRequirementMachine(proto, std::move(machine));
351+
348352
// Return the result for the specific protocol this request was kicked off on.
349353
return *result;
350354
}
351355
}
352356

353357
/// Builds the top-level generic signature requirements for this rewrite system.
354-
std::vector<Requirement>
355-
RequirementMachine::computeMinimalGenericSignatureRequirements(
358+
GenericSignature
359+
RequirementMachine::computeMinimalGenericSignature(
356360
bool reconstituteSugar) {
361+
assert(!Sig &&
362+
"Already computed minimal generic signature");
357363
assert(System.getProtocols().empty() &&
358364
"Not a top-level generic signature rewrite system");
359365
assert(!Params.empty() &&
@@ -375,7 +381,14 @@ RequirementMachine::computeMinimalGenericSignatureRequirements(
375381
reconstituteSugar, reqs, aliases);
376382
assert(aliases.empty());
377383

378-
return reqs;
384+
auto sig = GenericSignature::get(getGenericParams(), reqs);
385+
386+
// Remember the signature for generic signature queries. In particular,
387+
// getConformanceAccessPath() needs the current requirement machine's
388+
// generic signature.
389+
Sig = sig.getCanonicalSignature();
390+
391+
return sig;
379392
}
380393

381394
/// Check whether the inputs to the \c AbstractGenericSignatureRequest are
@@ -549,14 +562,11 @@ AbstractGenericSignatureRequestRQM::evaluate(
549562

550563
// We pass reconstituteSugar=false to ensure that if the original
551564
// requirements were canonical, the final signature remains canonical.
552-
auto minimalRequirements =
553-
machine->computeMinimalGenericSignatureRequirements(
565+
auto result = machine->computeMinimalGenericSignature(
554566
/*reconstituteSugar=*/false);
555-
556-
auto result = GenericSignature::get(genericParams, minimalRequirements);
557567
auto errorFlags = machine->getErrors();
558568

559-
if (!errorFlags) {
569+
if (!errorFlags.contains(GenericSignatureErrorFlags::HasInvalidRequirements)) {
560570
if (shouldSplitConcreteEquivalenceClasses(result.getRequirements(),
561571
/*proto=*/nullptr,
562572
machine.get())) {
@@ -567,7 +577,19 @@ AbstractGenericSignatureRequestRQM::evaluate(
567577
requirements, attempt);
568578
continue;
569579
}
580+
}
570581

582+
if (!errorFlags) {
583+
// If this signature was minimized without errors or non-redundant
584+
// concrete conformances, we can re-use the requirement machine for
585+
// subsequent queries, instead of building a new requirement machine
586+
// from the minimized signature. Do this before verify(), which
587+
// performs queries.
588+
rewriteCtx.installRequirementMachine(result.getCanonicalSignature(),
589+
std::move(machine));
590+
}
591+
592+
if (!errorFlags.contains(GenericSignatureErrorFlags::HasInvalidRequirements)) {
571593
// Check invariants.
572594
result.verify();
573595
}
@@ -731,11 +753,8 @@ InferredGenericSignatureRequestRQM::evaluate(
731753
result, GenericSignatureErrorFlags::CompletionFailed);
732754
}
733755

734-
auto minimalRequirements =
735-
machine->computeMinimalGenericSignatureRequirements(
756+
auto result = machine->computeMinimalGenericSignature(
736757
/*reconstituteSugar=*/true);
737-
738-
auto result = GenericSignature::get(genericParams, minimalRequirements);
739758
auto errorFlags = machine->getErrors();
740759

741760
if (attempt == 0 &&
@@ -747,7 +766,7 @@ InferredGenericSignatureRequestRQM::evaluate(
747766

748767
// FIXME: Handle allowConcreteGenericParams
749768

750-
if (!errorFlags) {
769+
if (!errorFlags.contains(GenericSignatureErrorFlags::HasInvalidRequirements)) {
751770
// Check if we need to rebuild the signature.
752771
if (shouldSplitConcreteEquivalenceClasses(result.getRequirements(),
753772
/*proto=*/nullptr,
@@ -759,7 +778,19 @@ InferredGenericSignatureRequestRQM::evaluate(
759778
requirements, attempt);
760779
continue;
761780
}
781+
}
782+
783+
if (!errorFlags) {
784+
// If this signature was minimized without errors or non-redundant
785+
// concrete conformances, we can re-use the requirement machine for
786+
// subsequent queries, instead of building a new requirement machine
787+
// from the minimized signature. Do this before verify(), which
788+
// performs queries.
789+
rewriteCtx.installRequirementMachine(result.getCanonicalSignature(),
790+
std::move(machine));
791+
}
762792

793+
if (!errorFlags.contains(GenericSignatureErrorFlags::HasInvalidRequirements)) {
763794
// Check invariants.
764795
result.verify();
765796
}

‎lib/AST/RequirementMachine/RewriteContext.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,21 @@ bool RewriteContext::isRecursivelyConstructingRequirementMachine(
195195
return !found->second->isComplete();
196196
}
197197

198+
/// Given a requirement machine that built a minimized signature, attempt to
199+
/// re-use it for subsequent queries against the minimized signature, instead
200+
/// of building a new one later.
201+
void RewriteContext::installRequirementMachine(
202+
CanGenericSignature sig,
203+
std::unique_ptr<RequirementMachine> machine) {
204+
auto *machinePtr = machine.release();
205+
206+
machinePtr->freeze();
207+
208+
auto inserted = Machines.insert(std::make_pair(sig, machinePtr)).second;
209+
if (!inserted)
210+
delete machinePtr;
211+
}
212+
198213
/// Implement Tarjan's algorithm to compute strongly-connected components in
199214
/// the protocol dependency graph.
200215
void RewriteContext::getProtocolComponentRec(
@@ -370,6 +385,11 @@ void RewriteContext::finishComputingRequirementSignatures(
370385
/// for being built with the same component.
371386
RequirementMachine *RewriteContext::getRequirementMachine(
372387
const ProtocolDecl *proto) {
388+
// First, get the requirement signature. If this protocol was written in
389+
// source, we'll minimize it and install the machine below, saving us the
390+
// effort of recomputing it.
391+
(void) proto->getRequirementSignature();
392+
373393
auto &component = getProtocolComponentImpl(proto);
374394

375395
if (component.Machine) {
@@ -435,6 +455,24 @@ bool RewriteContext::isRecursivelyConstructingRequirementMachine(
435455
!component->second.ComputedRequirementSignatures);
436456
}
437457

458+
/// Given a reuirement machine that built the requirement signatures for a
459+
/// protocol connected component, attempt to re-use it for subsequent
460+
/// queries against the connected component, instead of building a new one
461+
/// later.
462+
void RewriteContext::installRequirementMachine(
463+
const ProtocolDecl *proto,
464+
std::unique_ptr<RequirementMachine> machine) {
465+
auto *machinePtr = machine.release();
466+
467+
machinePtr->freeze();
468+
469+
auto &component = getProtocolComponentImpl(proto);
470+
if (component.Machine == nullptr)
471+
component.Machine = machinePtr;
472+
else
473+
delete machinePtr;
474+
}
475+
438476
/// We print stats in the destructor, which should get executed at the end of
439477
/// a compilation job.
440478
RewriteContext::~RewriteContext() {

‎lib/AST/RequirementMachine/RewriteContext.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,9 @@ class RewriteContext final {
206206
RequirementMachine *getRequirementMachine(CanGenericSignature sig);
207207
bool isRecursivelyConstructingRequirementMachine(CanGenericSignature sig);
208208

209+
void installRequirementMachine(CanGenericSignature sig,
210+
std::unique_ptr<RequirementMachine> machine);
211+
209212
ArrayRef<const ProtocolDecl *>
210213
startComputingRequirementSignatures(const ProtocolDecl *proto);
211214

@@ -214,6 +217,9 @@ class RewriteContext final {
214217
RequirementMachine *getRequirementMachine(const ProtocolDecl *proto);
215218
bool isRecursivelyConstructingRequirementMachine(const ProtocolDecl *proto);
216219

220+
void installRequirementMachine(const ProtocolDecl *proto,
221+
std::unique_ptr<RequirementMachine> machine);
222+
217223
~RewriteContext();
218224
};
219225

0 commit comments

Comments
 (0)