Skip to content

Commit fc573b6

Browse files
committed
[ConstraintSystem] Guard all the performance hacks with a flag
Package the flag into `performanceHacksEnabled()` method on `ConstraintSystem` and start using it to wrap all of the hacks in constraint generator and the solver.
1 parent 833b6b1 commit fc573b6

File tree

7 files changed

+62
-52
lines changed

7 files changed

+62
-52
lines changed

include/swift/Sema/ConstraintSystem.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3588,6 +3588,13 @@ class ConstraintSystem {
35883588
return Options.contains(ConstraintSystemFlags::ForCodeCompletion);
35893589
}
35903590

3591+
/// Check whether type-checker performance hacks has been explicitly
3592+
/// disabled by a flag.
3593+
bool performanceHacksEnabled() const {
3594+
return !getASTContext()
3595+
.TypeCheckerOpts.DisableConstraintSolverPerformanceHacks;
3596+
}
3597+
35913598
/// Log and record the application of the fix. Return true iff any
35923599
/// subsequent solution would be worse than the best known solution.
35933600
bool recordFix(ConstraintFix *fix, unsigned impact = 1);

lib/Sema/CSGen.cpp

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,11 +1168,13 @@ namespace {
11681168
/*trailingClosureMatching=*/std::nullopt,
11691169
CurDC, fnLocator);
11701170

1171-
Type fixedOutputType =
1172-
CS.getFixedTypeRecursive(outputTy, /*wantRValue=*/false);
1173-
if (!fixedOutputType->isTypeVariableOrMember()) {
1174-
CS.setFavoredType(anchor, fixedOutputType.getPointer());
1175-
outputTy = fixedOutputType;
1171+
if (CS.performanceHacksEnabled()) {
1172+
Type fixedOutputType =
1173+
CS.getFixedTypeRecursive(outputTy, /*wantRValue=*/false);
1174+
if (!fixedOutputType->isTypeVariableOrMember()) {
1175+
CS.setFavoredType(anchor, fixedOutputType.getPointer());
1176+
outputTy = fixedOutputType;
1177+
}
11761178
}
11771179

11781180
return outputTy;
@@ -1614,9 +1616,11 @@ namespace {
16141616
}
16151617
}
16161618

1617-
if (!knownType->hasPlaceholder()) {
1618-
// Set the favored type for this expression to the known type.
1619-
CS.setFavoredType(E, knownType.getPointer());
1619+
if (CS.performanceHacksEnabled()) {
1620+
if (!knownType->hasPlaceholder()) {
1621+
// Set the favored type for this expression to the known type.
1622+
CS.setFavoredType(E, knownType.getPointer());
1623+
}
16201624
}
16211625
}
16221626
}
@@ -2085,8 +2089,10 @@ namespace {
20852089
CS.getASTContext());
20862090
}
20872091

2088-
if (auto favoredTy = CS.getFavoredType(expr->getSubExpr())) {
2089-
CS.setFavoredType(expr, favoredTy);
2092+
if (CS.performanceHacksEnabled()) {
2093+
if (auto favoredTy = CS.getFavoredType(expr->getSubExpr())) {
2094+
CS.setFavoredType(expr, favoredTy);
2095+
}
20902096
}
20912097
return CS.getType(expr->getSubExpr());
20922098
}
@@ -3373,11 +3379,13 @@ namespace {
33733379

33743380
// If we ended up resolving the result type variable to a concrete type,
33753381
// set it as the favored type for this expression.
3376-
Type fixedType =
3377-
CS.getFixedTypeRecursive(resultType, /*wantRvalue=*/true);
3378-
if (!fixedType->isTypeVariableOrMember()) {
3379-
CS.setFavoredType(expr, fixedType.getPointer());
3380-
resultType = fixedType;
3382+
if (CS.performanceHacksEnabled()) {
3383+
Type fixedType =
3384+
CS.getFixedTypeRecursive(resultType, /*wantRvalue=*/true);
3385+
if (!fixedType->isTypeVariableOrMember()) {
3386+
CS.setFavoredType(expr, fixedType.getPointer());
3387+
resultType = fixedType;
3388+
}
33813389
}
33823390

33833391
return resultType;
@@ -5184,7 +5192,7 @@ ConstraintSystem::applyPropertyWrapperToParameter(
51845192
}
51855193

51865194
void ConstraintSystem::optimizeConstraints(Expr *e) {
5187-
if (getASTContext().TypeCheckerOpts.DisableConstraintSolverPerformanceHacks)
5195+
if (!performanceHacksEnabled())
51885196
return;
51895197

51905198
SmallVector<Expr *, 16> linkedExprs;

lib/Sema/CSRanking.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,6 @@ void ConstraintSystem::clearScore() {
168168
}
169169

170170
bool ConstraintSystem::worseThanBestSolution() const {
171-
if (getASTContext().TypeCheckerOpts.DisableConstraintSolverPerformanceHacks)
172-
return false;
173-
174171
if (!solverState || !solverState->BestScore ||
175172
CurrentScore <= *solverState->BestScore)
176173
return false;

lib/Sema/CSSimplify.cpp

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10191,13 +10191,15 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
1019110191
if (path.back().getKind() == ConstraintLocator::ConstructorMember)
1019210192
isImplicitInit = true;
1019310193

10194-
if (auto *applyExpr = getAsExpr<ApplyExpr>(anchor)) {
10195-
if (auto *argExpr = applyExpr->getArgs()->getUnlabeledUnaryExpr()) {
10196-
favoredType = getFavoredType(argExpr);
10197-
10198-
if (!favoredType) {
10199-
optimizeConstraints(argExpr);
10194+
if (performanceHacksEnabled()) {
10195+
if (auto *applyExpr = getAsExpr<ApplyExpr>(anchor)) {
10196+
if (auto *argExpr = applyExpr->getArgs()->getUnlabeledUnaryExpr()) {
1020010197
favoredType = getFavoredType(argExpr);
10198+
10199+
if (!favoredType) {
10200+
optimizeConstraints(argExpr);
10201+
favoredType = getFavoredType(argExpr);
10202+
}
1020110203
}
1020210204
}
1020310205
}
@@ -10312,22 +10314,24 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
1031210314
// If the invocation's argument expression has a favored type,
1031310315
// use that information to determine whether a specific overload for
1031410316
// the candidate should be favored.
10315-
if (isa<ConstructorDecl>(decl) && favoredType &&
10316-
result.FavoredChoice == ~0U) {
10317-
auto *ctor = cast<ConstructorDecl>(decl);
10318-
10319-
// Only try and favor monomorphic unary initializers.
10320-
if (!ctor->isGenericContext()) {
10321-
if (!ctor->getMethodInterfaceType()->hasError()) {
10322-
// The constructor might have an error type because we don't skip
10323-
// invalid decls for code completion
10324-
auto args = ctor->getMethodInterfaceType()
10325-
->castTo<FunctionType>()
10326-
->getParams();
10327-
if (args.size() == 1 && !args[0].hasLabel() &&
10328-
args[0].getPlainType()->isEqual(favoredType)) {
10329-
if (!isDeclUnavailable(decl, memberLocator))
10330-
result.FavoredChoice = result.ViableCandidates.size();
10317+
if (performanceHacksEnabled()) {
10318+
if (isa<ConstructorDecl>(decl) && favoredType &&
10319+
result.FavoredChoice == ~0U) {
10320+
auto *ctor = cast<ConstructorDecl>(decl);
10321+
10322+
// Only try and favor monomorphic unary initializers.
10323+
if (!ctor->isGenericContext()) {
10324+
if (!ctor->getMethodInterfaceType()->hasError()) {
10325+
// The constructor might have an error type because we don't skip
10326+
// invalid decls for code completion
10327+
auto args = ctor->getMethodInterfaceType()
10328+
->castTo<FunctionType>()
10329+
->getParams();
10330+
if (args.size() == 1 && !args[0].hasLabel() &&
10331+
args[0].getPlainType()->isEqual(favoredType)) {
10332+
if (!isDeclUnavailable(decl, memberLocator))
10333+
result.FavoredChoice = result.ViableCandidates.size();
10334+
}
1033110335
}
1033210336
}
1033310337
}

lib/Sema/CSSolver.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ Solution ConstraintSystem::finalize() {
7474

7575
// Update the best score we've seen so far.
7676
auto &ctx = getASTContext();
77-
assert(ctx.TypeCheckerOpts.DisableConstraintSolverPerformanceHacks ||
78-
!solverState->BestScore || CurrentScore <= *solverState->BestScore);
77+
assert(!solverState->BestScore || CurrentScore <= *solverState->BestScore);
7978

8079
if (!solverState->BestScore || CurrentScore <= *solverState->BestScore) {
8180
solverState->BestScore = CurrentScore;
@@ -983,7 +982,7 @@ void ConstraintSystem::Candidate::applySolutions(
983982
}
984983

985984
void ConstraintSystem::shrink(Expr *expr) {
986-
if (getASTContext().TypeCheckerOpts.DisableConstraintSolverPerformanceHacks)
985+
if (!performanceHacksEnabled())
987986
return;
988987

989988
using DomainMap = llvm::SmallDenseMap<Expr *, ArrayRef<ValueDecl *>>;

lib/Sema/CSStep.cpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -638,9 +638,6 @@ bool DisjunctionStep::shouldSkip(const DisjunctionChoice &choice) const {
638638
if (choice.isUnavailable() && !CS.shouldAttemptFixes())
639639
return skip("unavailable");
640640

641-
if (ctx.TypeCheckerOpts.DisableConstraintSolverPerformanceHacks)
642-
return false;
643-
644641
// If the solver already found a solution with a better overload choice that
645642
// can be unconditionally substituted by the current choice, skip the current
646643
// choice.
@@ -726,15 +723,10 @@ bool swift::isSIMDOperator(ValueDecl *value) {
726723

727724
bool DisjunctionStep::shortCircuitDisjunctionAt(
728725
Constraint *currentChoice, Constraint *lastSuccessfulChoice) const {
729-
auto &ctx = CS.getASTContext();
730-
731726
// Anything without a fix is better than anything with a fix.
732727
if (currentChoice->getFix() && !lastSuccessfulChoice->getFix())
733728
return true;
734729

735-
if (ctx.TypeCheckerOpts.DisableConstraintSolverPerformanceHacks)
736-
return false;
737-
738730
if (auto restriction = currentChoice->getRestriction()) {
739731
// Non-optional conversions are better than optional-to-optional
740732
// conversions.

lib/Sema/CSStep.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,9 @@ class DisjunctionStep final : public BindingStep<DisjunctionChoiceProducer> {
684684
// chained together. If so, disable choices which differ
685685
// from currently selected representative.
686686
void pruneOverloadSet(Constraint *disjunction) {
687+
if (!CS.performanceHacksEnabled())
688+
return;
689+
687690
auto *choice = disjunction->getNestedConstraints().front();
688691
if (choice->getKind() != ConstraintKind::BindOverload)
689692
return;

0 commit comments

Comments
 (0)