Skip to content

Commit ae4e106

Browse files
authored
Merge pull request #18281 from xedin/gather-constraints-improvements
[Perf Experiment][ConstraintGraph] Gather constraints improvements
2 parents 4bed2b7 + d9f0134 commit ae4e106

File tree

9 files changed

+107
-98
lines changed

9 files changed

+107
-98
lines changed

lib/Sema/CSBindings.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
//===----------------------------------------------------------------------===//
1616
#include "ConstraintGraph.h"
1717
#include "ConstraintSystem.h"
18+
#include "llvm/ADT/SetVector.h"
1819
#include <tuple>
1920

2021
using namespace swift;
@@ -361,8 +362,7 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) {
361362
assert(!typeVar->getImpl().getFixedType(nullptr) && "has a fixed type");
362363

363364
// Gather the constraints associated with this type variable.
364-
SmallVector<Constraint *, 8> constraints;
365-
llvm::SmallPtrSet<Constraint *, 4> visitedConstraints;
365+
llvm::SetVector<Constraint *> constraints;
366366
getConstraintGraph().gatherConstraints(
367367
typeVar, constraints, ConstraintGraph::GatheringKind::EquivalenceClass);
368368

@@ -377,10 +377,6 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) {
377377
bool hasNonDependentMemberRelationalConstraints = false;
378378
bool hasDependentMemberRelationalConstraints = false;
379379
for (auto constraint : constraints) {
380-
// Only visit each constraint once.
381-
if (!visitedConstraints.insert(constraint).second)
382-
continue;
383-
384380
switch (constraint->getKind()) {
385381
case ConstraintKind::Bind:
386382
case ConstraintKind::Equal:

lib/Sema/CSDiag.cpp

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "swift/Basic/Defer.h"
3131
#include "swift/Basic/StringExtras.h"
3232
#include "llvm/ADT/DenseSet.h"
33+
#include "llvm/ADT/SetVector.h"
3334
#include "llvm/Support/Compiler.h"
3435
#include "llvm/Support/SaveAndRestore.h"
3536

@@ -8728,29 +8729,27 @@ bool FailureDiagnosis::diagnoseArchetypeAmbiguity() {
87288729
// because type B would have no constraints associated with it.
87298730
unsigned numConstraints = 0;
87308731
{
8731-
llvm::SmallVector<Constraint *, 2> constraints;
8732+
llvm::SetVector<Constraint *> constraints;
87328733
CS.getConstraintGraph().gatherConstraints(
8733-
tv, constraints, ConstraintGraph::GatheringKind::EquivalenceClass);
8734-
8735-
for (auto constraint : constraints) {
8736-
// We are not interested in ConformsTo constraints because
8737-
// such constraints specify restrictions on the archetypes themselves.
8738-
if (constraint->getKind() == ConstraintKind::ConformsTo)
8739-
continue;
8734+
tv, constraints, ConstraintGraph::GatheringKind::EquivalenceClass,
8735+
[&](Constraint *constraint) -> bool {
8736+
// We are not interested in ConformsTo constraints because
8737+
// we can't derive any concrete type information from them.
8738+
if (constraint->getKind() == ConstraintKind::ConformsTo)
8739+
return false;
8740+
8741+
if (constraint->getKind() == ConstraintKind::Bind) {
8742+
if (auto locator = constraint->getLocator()) {
8743+
auto anchor = locator->getAnchor();
8744+
if (anchor && isa<UnresolvedDotExpr>(anchor))
8745+
return false;
8746+
}
8747+
}
87408748

8741-
// Some of the bind constraints specify relations between
8742-
// parent type and it's member fields/types, we are not
8743-
// interested in that, since it's not related to archetype resolution.
8744-
if (constraint->getKind() == ConstraintKind::Bind) {
8745-
if (auto locator = constraint->getLocator()) {
8746-
auto anchor = locator->getAnchor();
8747-
if (anchor && isa<UnresolvedDotExpr>(anchor))
8748-
continue;
8749-
}
8750-
}
8749+
return true;
8750+
});
87518751

8752-
numConstraints++;
8753-
}
8752+
numConstraints = constraints.size();
87548753
}
87558754

87568755
auto locator = impl.getLocator();

lib/Sema/CSGen.cpp

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "swift/AST/SubstitutionMap.h"
2424
#include "swift/Sema/IDETypeChecking.h"
2525
#include "llvm/ADT/APInt.h"
26+
#include "llvm/ADT/SetVector.h"
2627
#include "llvm/ADT/StringExtras.h"
2728
#include "llvm/ADT/StringSwitch.h"
2829
#include <utility>
@@ -574,18 +575,19 @@ namespace {
574575
// being applied, and the only constraint attached to it should
575576
// be the disjunction constraint for the overload group.
576577
auto &CG = CS.getConstraintGraph();
577-
SmallVector<Constraint *, 4> constraints;
578-
CG.gatherConstraints(tyvarType, constraints,
579-
ConstraintGraph::GatheringKind::EquivalenceClass);
580-
if (constraints.empty())
578+
llvm::SetVector<Constraint *> disjunctions;
579+
CG.gatherConstraints(tyvarType, disjunctions,
580+
ConstraintGraph::GatheringKind::EquivalenceClass,
581+
[](Constraint *constraint) -> bool {
582+
return constraint->getKind() ==
583+
ConstraintKind::Disjunction;
584+
});
585+
if (disjunctions.empty())
581586
return;
582587

583588
// Look for the disjunction that binds the overload set.
584-
for (auto constraint : constraints) {
585-
if (constraint->getKind() != ConstraintKind::Disjunction)
586-
continue;
587-
588-
auto oldConstraints = constraint->getNestedConstraints();
589+
for (auto *disjunction : disjunctions) {
590+
auto oldConstraints = disjunction->getNestedConstraints();
589591
auto csLoc = CS.getConstraintLocator(expr->getFn());
590592

591593
// Only replace the disjunctive overload constraint.
@@ -633,8 +635,8 @@ namespace {
633635

634636
// Remove the original constraint from the inactive constraint
635637
// list and add the new one.
636-
CS.removeInactiveConstraint(constraint);
637-
638+
CS.removeInactiveConstraint(disjunction);
639+
638640
// Create the disjunction of favored constraints.
639641
auto favoredConstraintsDisjunction =
640642
Constraint::createDisjunction(CS,

lib/Sema/CSSimplify.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "swift/AST/ProtocolConformance.h"
2222
#include "swift/Basic/StringExtras.h"
2323
#include "swift/ClangImporter/ClangModule.h"
24+
#include "llvm/ADT/SetVector.h"
2425
#include "llvm/Support/Compiler.h"
2526

2627
using namespace swift;
@@ -5085,17 +5086,19 @@ ConstraintSystem::addKeyPathApplicationRootConstraint(Type root, ConstraintLocat
50855086
auto typeVar = getType(keyPathExpr)->getAs<TypeVariableType>();
50865087
if (!typeVar)
50875088
return;
5088-
5089-
SmallVector<Constraint *, 4> constraints;
5090-
CG.gatherConstraints(typeVar, constraints,
5091-
ConstraintGraph::GatheringKind::EquivalenceClass);
5092-
5089+
5090+
llvm::SetVector<Constraint *> constraints;
5091+
CG.gatherConstraints(
5092+
typeVar, constraints, ConstraintGraph::GatheringKind::EquivalenceClass,
5093+
[&keyPathExpr](Constraint *constraint) -> bool {
5094+
return constraint->getKind() == ConstraintKind::KeyPath &&
5095+
constraint->getLocator()->getAnchor() == keyPathExpr;
5096+
});
5097+
50935098
for (auto constraint : constraints) {
5094-
if (constraint->getKind() == ConstraintKind::KeyPath &&
5095-
constraint->getLocator()->getAnchor() == keyPathExpr) {
5096-
auto keyPathRootTy = constraint->getSecondType();
5097-
addConstraint(ConstraintKind::Subtype, root->getWithoutSpecifierType(), keyPathRootTy, locator);
5098-
}
5099+
auto keyPathRootTy = constraint->getSecondType();
5100+
addConstraint(ConstraintKind::Subtype, root->getWithoutSpecifierType(),
5101+
keyPathRootTy, locator);
50995102
}
51005103
}
51015104

lib/Sema/CSSolver.cpp

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "ConstraintSystem.h"
1818
#include "swift/AST/ParameterList.h"
1919
#include "swift/AST/TypeWalker.h"
20+
#include "llvm/ADT/SetVector.h"
2021
#include "llvm/ADT/Statistic.h"
2122
#include "llvm/Support/Compiler.h"
2223
#include "llvm/Support/SaveAndRestore.h"
@@ -93,15 +94,18 @@ Optional<Type> ConstraintSystem::checkTypeOfBinding(TypeVariableType *typeVar,
9394
*isNilLiteral = false;
9495

9596
// Look for a literal-conformance constraint on the type variable.
96-
SmallVector<Constraint *, 8> constraints;
97+
llvm::SetVector<Constraint *> constraints;
9798
getConstraintGraph().gatherConstraints(
9899
bindingTypeVar, constraints,
99-
ConstraintGraph::GatheringKind::EquivalenceClass);
100+
ConstraintGraph::GatheringKind::EquivalenceClass,
101+
[](Constraint *constraint) -> bool {
102+
return constraint->getKind() == ConstraintKind::LiteralConformsTo &&
103+
constraint->getProtocol()->isSpecificProtocol(
104+
KnownProtocolKind::ExpressibleByNilLiteral);
105+
});
106+
100107
for (auto constraint : constraints) {
101-
if (constraint->getKind() == ConstraintKind::LiteralConformsTo &&
102-
constraint->getProtocol()->isSpecificProtocol(
103-
KnownProtocolKind::ExpressibleByNilLiteral) &&
104-
simplifyType(constraint->getFirstType())->isEqual(bindingTypeVar)) {
108+
if (simplifyType(constraint->getFirstType())->isEqual(bindingTypeVar)) {
105109
*isNilLiteral = true;
106110
break;
107111
}
@@ -1899,14 +1903,14 @@ static Constraint *selectBestBindingDisjunction(
18991903
->getAs<TypeVariableType>();
19001904
assert(tv);
19011905

1902-
SmallVector<Constraint *, 8> constraints;
1906+
llvm::SetVector<Constraint *> constraints;
19031907
cs.getConstraintGraph().gatherConstraints(
1904-
tv, constraints, ConstraintGraph::GatheringKind::EquivalenceClass);
1908+
tv, constraints, ConstraintGraph::GatheringKind::EquivalenceClass,
1909+
[](Constraint *constraint) {
1910+
return constraint->getKind() == ConstraintKind::Conversion;
1911+
});
19051912

19061913
for (auto *constraint : constraints) {
1907-
if (constraint->getKind() != ConstraintKind::Conversion)
1908-
continue;
1909-
19101914
auto toType =
19111915
cs.simplifyType(constraint->getSecondType())->getRValueType();
19121916
auto *toTV = toType->getAs<TypeVariableType>();
@@ -2224,26 +2228,23 @@ void DisjunctionChoice::propagateConversionInfo() const {
22242228
return;
22252229

22262230
auto conversionType = bindings.Bindings[0].BindingType;
2227-
SmallVector<Constraint *, 4> constraints;
2231+
llvm::SetVector<Constraint *> constraints;
22282232
CS->CG.gatherConstraints(typeVar, constraints,
2229-
ConstraintGraph::GatheringKind::EquivalenceClass);
2230-
2231-
bool viableForBinding = true;
2232-
for (auto adjacent : constraints) {
2233-
switch (adjacent->getKind()) {
2234-
case ConstraintKind::Conversion:
2235-
case ConstraintKind::Defaultable:
2236-
case ConstraintKind::ConformsTo:
2237-
case ConstraintKind::LiteralConformsTo:
2238-
break;
2239-
2240-
default:
2241-
viableForBinding = false;
2242-
break;
2243-
}
2244-
}
2245-
2246-
if (viableForBinding)
2233+
ConstraintGraph::GatheringKind::EquivalenceClass,
2234+
[](Constraint *constraint) -> bool {
2235+
switch (constraint->getKind()) {
2236+
case ConstraintKind::Conversion:
2237+
case ConstraintKind::Defaultable:
2238+
case ConstraintKind::ConformsTo:
2239+
case ConstraintKind::LiteralConformsTo:
2240+
return false;
2241+
2242+
default:
2243+
return true;
2244+
}
2245+
});
2246+
2247+
if (constraints.empty())
22472248
CS->addConstraint(ConstraintKind::Bind, typeVar, conversionType,
22482249
Choice->getLocator());
22492250
}

lib/Sema/ConstraintGraph.cpp

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "ConstraintGraphScope.h"
2020
#include "ConstraintSystem.h"
2121
#include "swift/Basic/Statistic.h"
22+
#include "llvm/ADT/SetVector.h"
2223
#include "llvm/Support/Debug.h"
2324
#include "llvm/Support/SaveAndRestore.h"
2425
#include <algorithm>
@@ -467,18 +468,20 @@ void ConstraintGraph::unbindTypeVariable(TypeVariableType *typeVar, Type fixed){
467468
}
468469

469470
void ConstraintGraph::gatherConstraints(
470-
TypeVariableType *typeVar,
471-
SmallVectorImpl<Constraint *> &constraints,
472-
GatheringKind kind) {
471+
TypeVariableType *typeVar, llvm::SetVector<Constraint *> &constraints,
472+
GatheringKind kind,
473+
llvm::function_ref<bool(Constraint *)> acceptConstraint) {
473474
auto &reprNode = (*this)[CS.getRepresentative(typeVar)];
474475
auto equivClass = reprNode.getEquivalenceClass();
475476
llvm::SmallPtrSet<TypeVariableType *, 4> typeVars;
476477
for (auto typeVar : equivClass) {
477478
if (!typeVars.insert(typeVar).second)
478479
continue;
479480

480-
for (auto constraint : (*this)[typeVar].getConstraints())
481-
constraints.push_back(constraint);
481+
for (auto constraint : (*this)[typeVar].getConstraints()) {
482+
if (acceptConstraint(constraint))
483+
constraints.insert(constraint);
484+
}
482485

483486
auto &node = (*this)[typeVar];
484487

@@ -510,8 +513,10 @@ void ConstraintGraph::gatherConstraints(
510513
if (!typeVars.insert(adjTypeVarEquiv).second)
511514
continue;
512515

513-
for (auto constraint : (*this)[adjTypeVarEquiv].getConstraints())
514-
constraints.push_back(constraint);
516+
for (auto constraint : (*this)[adjTypeVarEquiv].getConstraints()) {
517+
if (acceptConstraint(constraint))
518+
constraints.insert(constraint);
519+
}
515520
}
516521
}
517522
}

lib/Sema/ConstraintGraph.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "llvm/ADT/ArrayRef.h"
2323
#include "llvm/ADT/DenseSet.h"
2424
#include "llvm/ADT/DenseMap.h"
25+
#include "llvm/ADT/SetVector.h"
2526
#include "llvm/ADT/SmallVector.h"
2627
#include "llvm/Support/Compiler.h"
2728
#include <functional>
@@ -221,11 +222,12 @@ class ConstraintGraph {
221222
/// Gather the set of constraints that involve the given type variable,
222223
/// i.e., those constraints that will be affected when the type variable
223224
/// gets merged or bound to a fixed type.
224-
///
225-
/// The resulting set of constraints may contain duplicates.
226-
void gatherConstraints(TypeVariableType *typeVar,
227-
SmallVectorImpl<Constraint *> &constraints,
228-
GatheringKind kind);
225+
void
226+
gatherConstraints(TypeVariableType *typeVar,
227+
llvm::SetVector<Constraint *> &constraints,
228+
GatheringKind kind,
229+
llvm::function_ref<bool(Constraint *)> acceptConstraint =
230+
[](Constraint *constraint) { return true; });
229231

230232
/// Retrieve the type variables that correspond to nodes in the graph.
231233
///

lib/Sema/ConstraintSystem.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "ConstraintGraph.h"
2020
#include "swift/AST/GenericEnvironment.h"
2121
#include "swift/Basic/Statistic.h"
22+
#include "llvm/ADT/SetVector.h"
2223
#include "llvm/ADT/SmallString.h"
2324
#include "llvm/Support/Compiler.h"
2425
#include "llvm/Support/Format.h"
@@ -213,17 +214,16 @@ void ConstraintSystem::setMustBeMaterializableRecursive(Type type)
213214
void ConstraintSystem::addTypeVariableConstraintsToWorkList(
214215
TypeVariableType *typeVar) {
215216
// Gather the constraints affected by a change to this type variable.
216-
SmallVector<Constraint *, 8> constraints;
217-
CG.gatherConstraints(typeVar, constraints,
218-
ConstraintGraph::GatheringKind::AllMentions);
217+
llvm::SetVector<Constraint *> inactiveConstraints;
218+
CG.gatherConstraints(
219+
typeVar, inactiveConstraints, ConstraintGraph::GatheringKind::AllMentions,
220+
[](Constraint *constraint) { return !constraint->isActive(); });
219221

220222
// Add any constraints that aren't already active to the worklist.
221-
for (auto constraint : constraints) {
222-
if (!constraint->isActive()) {
223-
ActiveConstraints.splice(ActiveConstraints.end(),
224-
InactiveConstraints, constraint);
225-
constraint->setActive(true);
226-
}
223+
for (auto constraint : inactiveConstraints) {
224+
ActiveConstraints.splice(ActiveConstraints.end(), InactiveConstraints,
225+
constraint);
226+
constraint->setActive(true);
227227
}
228228
}
229229

validation-test/Sema/type_checker_perf/fast/rdar19738292.swift.gyb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// RUN: %scale-test --begin 7 --end 12 --step 1 --select incrementScopeCounter %s
22
// REQUIRES: OS=macosx
33
// REQUIRES: asserts
4+
// REQUIRES: rdar42650365
45

56
let a = [[0]]
67
_ = a[0][0]

0 commit comments

Comments
 (0)