Skip to content

[ConstraintSystem] Guard all the performance hacks with a flag #82320

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 2 commits into from
Jun 20, 2025
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
3 changes: 0 additions & 3 deletions include/swift/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -975,9 +975,6 @@ namespace swift {
/// is for testing purposes.
std::vector<std::string> DebugForbidTypecheckPrefixes;

/// Disable the shrink phase of the expression type checker.
bool SolverDisableShrink = false;

/// Enable experimental operator designated types feature.
bool EnableOperatorDesignatedTypes = false;

Expand Down
4 changes: 0 additions & 4 deletions include/swift/Option/FrontendOptions.td
Original file line number Diff line number Diff line change
Expand Up @@ -852,10 +852,6 @@ def solver_scope_threshold_EQ : Joined<["-"], "solver-scope-threshold=">,
def solver_trail_threshold_EQ : Joined<["-"], "solver-trail-threshold=">,
HelpText<"Expression type checking trail change limit">;

def solver_disable_shrink :
Flag<["-"], "solver-disable-shrink">,
HelpText<"Disable the shrink phase of expression type checking">;

def solver_disable_splitter : Flag<["-"], "solver-disable-splitter">,
HelpText<"Disable the component splitter phase of expression type checking">;

Expand Down
7 changes: 7 additions & 0 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -3588,6 +3588,13 @@ class ConstraintSystem {
return Options.contains(ConstraintSystemFlags::ForCodeCompletion);
}

/// Check whether type-checker performance hacks has been explicitly
/// disabled by a flag.
bool performanceHacksEnabled() const {
return !getASTContext()
.TypeCheckerOpts.DisableConstraintSolverPerformanceHacks;
}

/// Log and record the application of the fix. Return true iff any
/// subsequent solution would be worse than the best known solution.
bool recordFix(ConstraintFix *fix, unsigned impact = 1);
Expand Down
3 changes: 0 additions & 3 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2015,9 +2015,6 @@ static bool ParseTypeCheckerArgs(TypeCheckerOptions &Opts, ArgList &Args,
Opts.DebugForbidTypecheckPrefixes.push_back(A);
}

if (Args.getLastArg(OPT_solver_disable_shrink))
Opts.SolverDisableShrink = true;

if (Args.getLastArg(OPT_solver_disable_splitter))
Opts.SolverDisableSplitter = true;

Expand Down
40 changes: 24 additions & 16 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1168,11 +1168,13 @@ namespace {
/*trailingClosureMatching=*/std::nullopt,
CurDC, fnLocator);

Type fixedOutputType =
CS.getFixedTypeRecursive(outputTy, /*wantRValue=*/false);
if (!fixedOutputType->isTypeVariableOrMember()) {
CS.setFavoredType(anchor, fixedOutputType.getPointer());
outputTy = fixedOutputType;
if (CS.performanceHacksEnabled()) {
Type fixedOutputType =
CS.getFixedTypeRecursive(outputTy, /*wantRValue=*/false);
if (!fixedOutputType->isTypeVariableOrMember()) {
CS.setFavoredType(anchor, fixedOutputType.getPointer());
outputTy = fixedOutputType;
}
}

return outputTy;
Expand Down Expand Up @@ -1614,9 +1616,11 @@ namespace {
}
}

if (!knownType->hasPlaceholder()) {
// Set the favored type for this expression to the known type.
CS.setFavoredType(E, knownType.getPointer());
if (CS.performanceHacksEnabled()) {
if (!knownType->hasPlaceholder()) {
// Set the favored type for this expression to the known type.
CS.setFavoredType(E, knownType.getPointer());
}
}
}
}
Expand Down Expand Up @@ -2085,8 +2089,10 @@ namespace {
CS.getASTContext());
}

if (auto favoredTy = CS.getFavoredType(expr->getSubExpr())) {
CS.setFavoredType(expr, favoredTy);
if (CS.performanceHacksEnabled()) {
if (auto favoredTy = CS.getFavoredType(expr->getSubExpr())) {
CS.setFavoredType(expr, favoredTy);
}
}
return CS.getType(expr->getSubExpr());
}
Expand Down Expand Up @@ -3373,11 +3379,13 @@ namespace {

// If we ended up resolving the result type variable to a concrete type,
// set it as the favored type for this expression.
Type fixedType =
CS.getFixedTypeRecursive(resultType, /*wantRvalue=*/true);
if (!fixedType->isTypeVariableOrMember()) {
CS.setFavoredType(expr, fixedType.getPointer());
resultType = fixedType;
if (CS.performanceHacksEnabled()) {
Type fixedType =
CS.getFixedTypeRecursive(resultType, /*wantRvalue=*/true);
if (!fixedType->isTypeVariableOrMember()) {
CS.setFavoredType(expr, fixedType.getPointer());
resultType = fixedType;
}
}

return resultType;
Expand Down Expand Up @@ -5184,7 +5192,7 @@ ConstraintSystem::applyPropertyWrapperToParameter(
}

void ConstraintSystem::optimizeConstraints(Expr *e) {
if (getASTContext().TypeCheckerOpts.DisableConstraintSolverPerformanceHacks)
if (!performanceHacksEnabled())
return;

SmallVector<Expr *, 16> linkedExprs;
Expand Down
3 changes: 0 additions & 3 deletions lib/Sema/CSRanking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,6 @@ void ConstraintSystem::clearScore() {
}

bool ConstraintSystem::worseThanBestSolution() const {
if (getASTContext().TypeCheckerOpts.DisableConstraintSolverPerformanceHacks)
return false;

if (!solverState || !solverState->BestScore ||
CurrentScore <= *solverState->BestScore)
return false;
Expand Down
48 changes: 26 additions & 22 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10191,13 +10191,15 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
if (path.back().getKind() == ConstraintLocator::ConstructorMember)
isImplicitInit = true;

if (auto *applyExpr = getAsExpr<ApplyExpr>(anchor)) {
if (auto *argExpr = applyExpr->getArgs()->getUnlabeledUnaryExpr()) {
favoredType = getFavoredType(argExpr);

if (!favoredType) {
optimizeConstraints(argExpr);
if (performanceHacksEnabled()) {
if (auto *applyExpr = getAsExpr<ApplyExpr>(anchor)) {
if (auto *argExpr = applyExpr->getArgs()->getUnlabeledUnaryExpr()) {
favoredType = getFavoredType(argExpr);

if (!favoredType) {
optimizeConstraints(argExpr);
favoredType = getFavoredType(argExpr);
}
}
}
}
Expand Down Expand Up @@ -10312,22 +10314,24 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
// If the invocation's argument expression has a favored type,
// use that information to determine whether a specific overload for
// the candidate should be favored.
if (isa<ConstructorDecl>(decl) && favoredType &&
result.FavoredChoice == ~0U) {
auto *ctor = cast<ConstructorDecl>(decl);

// Only try and favor monomorphic unary initializers.
if (!ctor->isGenericContext()) {
if (!ctor->getMethodInterfaceType()->hasError()) {
// The constructor might have an error type because we don't skip
// invalid decls for code completion
auto args = ctor->getMethodInterfaceType()
->castTo<FunctionType>()
->getParams();
if (args.size() == 1 && !args[0].hasLabel() &&
args[0].getPlainType()->isEqual(favoredType)) {
if (!isDeclUnavailable(decl, memberLocator))
result.FavoredChoice = result.ViableCandidates.size();
if (performanceHacksEnabled()) {
if (isa<ConstructorDecl>(decl) && favoredType &&
result.FavoredChoice == ~0U) {
auto *ctor = cast<ConstructorDecl>(decl);

// Only try and favor monomorphic unary initializers.
if (!ctor->isGenericContext()) {
if (!ctor->getMethodInterfaceType()->hasError()) {
// The constructor might have an error type because we don't skip
// invalid decls for code completion
auto args = ctor->getMethodInterfaceType()
->castTo<FunctionType>()
->getParams();
if (args.size() == 1 && !args[0].hasLabel() &&
args[0].getPlainType()->isEqual(favoredType)) {
if (!isDeclUnavailable(decl, memberLocator))
result.FavoredChoice = result.ViableCandidates.size();
}
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions lib/Sema/CSSolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ Solution ConstraintSystem::finalize() {

// Update the best score we've seen so far.
auto &ctx = getASTContext();
assert(ctx.TypeCheckerOpts.DisableConstraintSolverPerformanceHacks ||
!solverState->BestScore || CurrentScore <= *solverState->BestScore);
assert(!solverState->BestScore || CurrentScore <= *solverState->BestScore);
Comment on lines 76 to +77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xedin i noticed during a recent rebase & build that the ctx var is now marked unused after this change. was the new performanceHacksEnabled() method intended to continue to be consulted in this assertion, or is it safe to delete the now-unused var?

Copy link
Contributor Author

@xedin xedin Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s safe to remove the context variable!


if (!solverState->BestScore || CurrentScore <= *solverState->BestScore) {
solverState->BestScore = CurrentScore;
Expand Down Expand Up @@ -983,7 +982,7 @@ void ConstraintSystem::Candidate::applySolutions(
}

void ConstraintSystem::shrink(Expr *expr) {
if (getASTContext().TypeCheckerOpts.SolverDisableShrink)
if (!performanceHacksEnabled())
return;

using DomainMap = llvm::SmallDenseMap<Expr *, ArrayRef<ValueDecl *>>;
Expand Down
8 changes: 0 additions & 8 deletions lib/Sema/CSStep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -638,9 +638,6 @@ bool DisjunctionStep::shouldSkip(const DisjunctionChoice &choice) const {
if (choice.isUnavailable() && !CS.shouldAttemptFixes())
return skip("unavailable");

if (ctx.TypeCheckerOpts.DisableConstraintSolverPerformanceHacks)
return false;

// If the solver already found a solution with a better overload choice that
// can be unconditionally substituted by the current choice, skip the current
// choice.
Expand Down Expand Up @@ -726,15 +723,10 @@ bool swift::isSIMDOperator(ValueDecl *value) {

bool DisjunctionStep::shortCircuitDisjunctionAt(
Constraint *currentChoice, Constraint *lastSuccessfulChoice) const {
auto &ctx = CS.getASTContext();

// Anything without a fix is better than anything with a fix.
if (currentChoice->getFix() && !lastSuccessfulChoice->getFix())
return true;

if (ctx.TypeCheckerOpts.DisableConstraintSolverPerformanceHacks)
return false;

if (auto restriction = currentChoice->getRestriction()) {
// Non-optional conversions are better than optional-to-optional
// conversions.
Expand Down
3 changes: 3 additions & 0 deletions lib/Sema/CSStep.h
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,9 @@ class DisjunctionStep final : public BindingStep<DisjunctionChoiceProducer> {
// chained together. If so, disable choices which differ
// from currently selected representative.
void pruneOverloadSet(Constraint *disjunction) {
if (!CS.performanceHacksEnabled())
return;

auto *choice = disjunction->getNestedConstraints().front();
if (choice->getKind() != ConstraintKind::BindOverload)
return;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-typecheck-verify-swift -solver-disable-shrink
// RUN: %target-typecheck-verify-swift -disable-constraint-solver-performance-hacks

// Self-contained test case
protocol P1 {}; func f<T: P1>(_: T, _: T) -> T { fatalError() }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 -solver-disable-shrink
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1
// REQUIRES: tools-release,no_asan

struct Date {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 -solver-disable-shrink
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 -disable-constraint-solver-performance-hacks
// REQUIRES: tools-release,no_asan
// UNSUPPORTED: swift_test_mode_optimize_none && OS=linux-gnu

Expand Down