Skip to content

[ConstraintSystem] Revert new disjunction favoring algorithm #79128

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 72 commits into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
bccdd70
Revert "[CSOptimizer] Look through `OptionalEvaluationExpr`s when dea…
xedin Feb 4, 2025
d096dfd
Revert "[CSOptimizer] Don't consider disabled overloads when checking…
xedin Feb 4, 2025
e55b763
Revert "[CSOptimizer] Disjunctions with IUO overload choices are unsu…
xedin Feb 4, 2025
31bcba6
Revert "[CSOptimizer] MemberImportVisibility: Don't consider overload…
xedin Feb 4, 2025
0a27ba1
Revert "[CSOptimizer] Don't consider CGFloat widening when explicit i…
xedin Feb 4, 2025
62b4c94
Revert "[CSOptimizer] Literal arguments should cause score reset only…
xedin Feb 4, 2025
955eead
Revert "[CSOptimizer] NFC: check whether a choice is of operator inst…
xedin Feb 4, 2025
25ddf1a
Revert "[CSOptimizer/Tests] NFC: Add a perf test-case fixed by improv…
xedin Feb 4, 2025
51ab75d
Revert "[CSOptimizer] Extend candidate/parameter matching to support …
xedin Feb 4, 2025
0eca9be
Revert "[CSOptimizer] Favor choices that don't require application"
xedin Feb 4, 2025
224deb3
Revert "[CSOptimizer] Disable CGFloat -> Double conversion for unary …
xedin Feb 4, 2025
0ac1fa0
Revert "[CSOptimizer] Mark bitwise operators as supported"
xedin Feb 4, 2025
2f1b186
Revert "[CSOptimizer] Simplify handling of non-applied disjunctions"
xedin Feb 4, 2025
e7aeef8
Revert "[ConstraintSystem] Fix `getEffectiveOverloadType` handling of…
xedin Feb 4, 2025
8dd7f7a
Revert "[CSOptimizer] Reduce overload types before ranking"
xedin Feb 4, 2025
5f59fbc
Revert "[CSOptimizer] Implement special prioritization rules for resu…
xedin Feb 4, 2025
6635a47
Revert "[CSOptimizer] Allow only widening CGFloat->Double conversions…
xedin Feb 4, 2025
fa12735
Revert "[CSSimplify] CGFloat-Double: Rank narrowing correctly when re…
xedin Feb 4, 2025
c65c273
Revert "[CSBindings] Prevent `BindingSet::isViable` from dropping via…
xedin Feb 4, 2025
5235583
Revert "[CSOptimizer] Add support for chained members without arguments"
xedin Feb 4, 2025
6fc9f0b
Revert "[CSOptimizer] Mark compiler synthesized disjunctions as optim…
xedin Feb 4, 2025
7f7bbcb
Revert "[CSOptimizer] Make a light-weight generic overload check if s…
xedin Feb 4, 2025
a34b715
Revert "[CSOptimizer] Fix `selectDisjunction` to use favored choices …
xedin Feb 4, 2025
21bb236
Revert "[CSOptimizer] Limit "old" behavior compatibility to unlabeled…
xedin Feb 4, 2025
18008ce
Revert "[Tests] NFC: Update a couple of type-checker tests"
xedin Feb 4, 2025
202d15b
Revert "[Tests] NFC: Move simd related test-case from `slow` to `fast`"
xedin Feb 4, 2025
14d4469
Revert "[CSGen] NFC: Remove obsolete `ConstraintSystem::{get, set}Fav…
xedin Feb 4, 2025
c3a2359
Revert "[CSOptimizer] Allow literal arguments to match parameters tha…
xedin Feb 4, 2025
722fe0b
Revert "[CSOptimizer] Adjust `scoreCandidateMatch` to indicate when m…
xedin Feb 4, 2025
b0a2770
Revert "[CSOptimizer] Fix Double<->CGFloat implicit conversion suppor…
xedin Feb 4, 2025
d69f64a
Revert "[CSOptimizer] A more comprehensive generic overload checking …
xedin Feb 4, 2025
24d3f2a
Revert "[CSOptimizer] Restore old hack behavior which used to favor o…
xedin Feb 4, 2025
3c1e2b4
Revert "[CSOptimizer] Desugar types before checking for equality"
xedin Feb 4, 2025
169e71f
Revert "[ConstraintSystem] Narrowly disable `tryOptimizeGenericDisjun…
xedin Feb 4, 2025
3c85f81
Revert "[CSOptimizer] Infer argument candidates from calls to `Double…
xedin Feb 4, 2025
a316a44
Revert "[CSOptimizer] Score all of the overload choices matching on l…
xedin Feb 4, 2025
6278adf
Revert "[CSOptimizer] Enable ranking of `Int*`, `Float{80}` and `Doub…
xedin Feb 4, 2025
3b6ddcb
Revert "[CSOptimizer] Rank disjunctions based on score only if both s…
xedin Feb 4, 2025
c90dd99
Revert "[CSOptimizer] Rank results of operators regardless of whether…
xedin Feb 4, 2025
d75bd33
Revert "[Tests] NFC: Add more test-cases that were previously solved …
xedin Feb 4, 2025
3145a02
Revert "[CSOptimizer] Average score should reflect number of defaulte…
xedin Feb 4, 2025
11af1bb
Revert "[Tests] NFC: Adjust a couple of improved tests"
xedin Feb 4, 2025
213428b
Revert "[CSOptimizer] Don't optimize (implicit) calls with code compl…
xedin Feb 4, 2025
b893e8b
Revert "[CSOptimizer] attempt to rank only standard/simd operators an…
xedin Feb 4, 2025
4a1ca59
Revert "[CSOptimizer] Record best scores for each disjunction and use…
xedin Feb 4, 2025
ac4b6a4
Revert "[CSOptimizer] Let `determineBestChoicesInContext` return the …
xedin Feb 4, 2025
81fea7f
Revert "[CSOptimizer] Emulate old behavior related to favoring of una…
xedin Feb 4, 2025
e601140
Revert "[Tests] NFC: Add a test-case for rdar://133340307 which is no…
xedin Feb 4, 2025
48d635b
Revert "[CSOptimizer] Prefer homogeneous arithmetic operator overload…
xedin Feb 4, 2025
90bcaeb
Revert "[CSOptimizer] Remove an outdated optimization to compare reso…
xedin Feb 4, 2025
f6a6d19
Revert "[CSOptimizer] NFC: Switch from llvm::Optional to std::optiona…
xedin Feb 4, 2025
fadf285
Revert "[CSOptimizer] Increase score when type matches opaque type"
xedin Feb 4, 2025
b380264
Revert "[CSOptimizer] NFC: Switch to llvm::Optional"
xedin Feb 4, 2025
0645130
Revert "[CSOptimizer] NFC: Adjust conformance check to use `Constrain…
xedin Feb 4, 2025
bab8134
Revert "[CSOptimizer] Treat all type parameters equally"
xedin Feb 4, 2025
e91817f
Revert "[CSStep] Remove disjunction pruning logic from DisjunctionStep"
xedin Feb 4, 2025
9d812c2
Revert "[CSOptimizer] Relax candidate type requirements from equality…
xedin Feb 4, 2025
83946b8
Revert "[CSOptimizer] Use `matchCallArguments` to establish argument-…
xedin Feb 4, 2025
a170c76
Revert "[CSOptimizer] Don't attempt to optimize calls with code compl…
xedin Feb 4, 2025
e8970d4
Revert "[CSOptimizer] Allow generic operator overloads without associ…
xedin Feb 4, 2025
bb9081e
Revert "[CSOptimizer] Make sure that all parameters without arguments…
xedin Feb 4, 2025
f71037d
Revert "[CSStep] Don't favor choices until the disjunction is picked"
xedin Feb 4, 2025
71f282a
Revert "[CSOptimizer] Keep track of mismatches while evaluating candi…
xedin Feb 4, 2025
ba57099
Revert "[CSOptimizer] Favor SIMD related arithmetic operator choices …
xedin Feb 4, 2025
16da035
Revert "[CSOptimizer] Initial implementation of disjunction choice fa…
xedin Feb 4, 2025
4126a0e
Revert "[ConstraintSystem] Add skeleton of constraint optimizer"
xedin Feb 4, 2025
c1bd2b2
Revert "[CSGen] Remove ConstraintOptimizer and all favoring logic"
xedin Feb 4, 2025
778a7df
Revert "[ConstraintSystem] Remove `shrink`"
xedin Feb 4, 2025
46e058e
[TypeChecker] NFC: Remove resurrected use of `SolverShrinkUnsolvedThr…
xedin Feb 4, 2025
4a61d8f
[TypeChecker] Bring back `SolverDisableShrink`
xedin Feb 4, 2025
92147f8
[Tests] NFC: Mark tests affected by solver-perf revert as slow
xedin Feb 4, 2025
41fd4de
[Tests] NFC: Adjust async tests that are affected by performance hacks
xedin Feb 4, 2025
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: 3 additions & 0 deletions include/swift/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -906,6 +906,9 @@ 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: 4 additions & 0 deletions include/swift/Option/FrontendOptions.td
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,10 @@ 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
5 changes: 5 additions & 0 deletions include/swift/Sema/Constraint.h
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,11 @@ class Constraint final : public llvm::ilist_node<Constraint>,
});
}

/// Returns the number of resolved argument types for an applied disjunction
/// constraint. This is always zero for disjunctions that do not represent
/// an applied overload.
unsigned countResolvedArgumentTypes(ConstraintSystem &cs) const;

/// Determine if this constraint represents explicit conversion,
/// e.g. coercion constraint "as X" which forms a disjunction.
bool isExplicitConversion() const;
Expand Down
121 changes: 100 additions & 21 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -488,14 +488,6 @@ class TypeVariableType::Implementation {
/// literal (represented by `ArrayExpr` and `DictionaryExpr` in AST).
bool isCollectionLiteralType() const;

/// Determine whether this type variable represents a literal such
/// as an integer value, a floating-point value with and without a sign.
bool isNumberLiteralType() const;

/// Determine whether this type variable represents a result type of a
/// function call.
bool isFunctionResult() const;

/// Retrieve the representative of the equivalence class to which this
/// type variable belongs.
///
Expand Down Expand Up @@ -2250,6 +2242,10 @@ class ConstraintSystem {

llvm::SetVector<TypeVariableType *> TypeVariables;

/// Maps expressions to types for choosing a favored overload
/// type in a disjunction constraint.
llvm::DenseMap<Expr *, TypeBase *> FavoredTypes;

/// Maps discovered closures to their types inferred
/// from declared parameters/result and body.
///
Expand Down Expand Up @@ -2462,6 +2458,74 @@ class ConstraintSystem {
SynthesizedConformances;

private:
/// Describe the candidate expression for partial solving.
/// This class used by shrink & solve methods which apply
/// variation of directional path consistency algorithm in attempt
/// to reduce scopes of the overload sets (disjunctions) in the system.
class Candidate {
Expr *E;
DeclContext *DC;
llvm::BumpPtrAllocator &Allocator;

// Contextual Information.
Type CT;
ContextualTypePurpose CTP;

public:
Candidate(ConstraintSystem &cs, Expr *expr, Type ct = Type(),
ContextualTypePurpose ctp = ContextualTypePurpose::CTP_Unused)
: E(expr), DC(cs.DC), Allocator(cs.Allocator), CT(ct), CTP(ctp) {}

/// Return underlying expression.
Expr *getExpr() const { return E; }

/// Try to solve this candidate sub-expression
/// and re-write it's OSR domains afterwards.
///
/// \param shrunkExprs The set of expressions which
/// domains have been successfully shrunk so far.
///
/// \returns true on solver failure, false otherwise.
bool solve(llvm::SmallSetVector<OverloadSetRefExpr *, 4> &shrunkExprs);

/// Apply solutions found by solver as reduced OSR sets for
/// for current and all of it's sub-expressions.
///
/// \param solutions The solutions found by running solver on the
/// this candidate expression.
///
/// \param shrunkExprs The set of expressions which
/// domains have been successfully shrunk so far.
void applySolutions(
llvm::SmallVectorImpl<Solution> &solutions,
llvm::SmallSetVector<OverloadSetRefExpr *, 4> &shrunkExprs) const;

/// Check if attempt at solving of the candidate makes sense given
/// the current conditions - number of shrunk domains which is related
/// to the given candidate over the total number of disjunctions present.
static bool
isTooComplexGiven(ConstraintSystem *const cs,
llvm::SmallSetVector<OverloadSetRefExpr *, 4> &shrunkExprs) {
SmallVector<Constraint *, 8> disjunctions;
cs->collectDisjunctions(disjunctions);

unsigned unsolvedDisjunctions = disjunctions.size();
for (auto *disjunction : disjunctions) {
auto *locator = disjunction->getLocator();
if (!locator)
continue;

if (auto *OSR = getAsExpr<OverloadSetRefExpr>(locator->getAnchor())) {
if (shrunkExprs.count(OSR) > 0)
--unsolvedDisjunctions;
}
}

// The threshold used to be `TypeCheckerOpts.SolverShrinkUnsolvedThreshold`
return unsolvedDisjunctions >= 10;
}
};

/// Describes the current solver state.
struct SolverState {
SolverState(ConstraintSystem &cs,
Expand Down Expand Up @@ -2985,6 +3049,15 @@ class ConstraintSystem {
return nullptr;
}

TypeBase* getFavoredType(Expr *E) {
assert(E != nullptr);
return this->FavoredTypes[E];
}
void setFavoredType(Expr *E, TypeBase *T) {
assert(E != nullptr);
this->FavoredTypes[E] = T;
}

/// Set the type in our type map for the given node, and record the change
/// in the trail.
///
Expand Down Expand Up @@ -5239,11 +5312,19 @@ class ConstraintSystem {
/// \returns true if an error occurred, false otherwise.
bool solveSimplified(SmallVectorImpl<Solution> &solutions);

/// Find reduced domains of disjunction constraints for given
/// expression, this is achieved to solving individual sub-expressions
/// and combining resolving types. Such algorithm is called directional
/// path consistency because it goes from children to parents for all
/// related sub-expressions taking union of their domains.
///
/// \param expr The expression to find reductions for.
void shrink(Expr *expr);

/// Pick a disjunction from the InactiveConstraints list.
///
/// \returns The selected disjunction and a set of it's favored choices.
std::optional<std::pair<Constraint *, llvm::TinyPtrVector<Constraint *>>>
selectDisjunction();
/// \returns The selected disjunction.
Constraint *selectDisjunction();

/// Pick a conjunction from the InactiveConstraints list.
///
Expand Down Expand Up @@ -5432,6 +5513,11 @@ class ConstraintSystem {
bool applySolutionToBody(TapExpr *tapExpr,
SyntacticElementTargetRewriter &rewriter);

/// Reorder the disjunctive clauses for a given expression to
/// increase the likelihood that a favored constraint will be successfully
/// resolved before any others.
void optimizeConstraints(Expr *e);

void startExpressionTimer(ExpressionTimer::AnchorType anchor);

/// Determine if we've already explored too many paths in an
Expand Down Expand Up @@ -6172,8 +6258,7 @@ class DisjunctionChoiceProducer : public BindingProducer<DisjunctionChoice> {
public:
using Element = DisjunctionChoice;

DisjunctionChoiceProducer(ConstraintSystem &cs, Constraint *disjunction,
llvm::TinyPtrVector<Constraint *> &favorites)
DisjunctionChoiceProducer(ConstraintSystem &cs, Constraint *disjunction)
: BindingProducer(cs, disjunction->shouldRememberChoice()
? disjunction->getLocator()
: nullptr),
Expand All @@ -6183,11 +6268,6 @@ class DisjunctionChoiceProducer : public BindingProducer<DisjunctionChoice> {
assert(disjunction->getKind() == ConstraintKind::Disjunction);
assert(!disjunction->shouldRememberChoice() || disjunction->getLocator());

// Mark constraints as favored. This information
// is going to be used by partitioner.
for (auto *choice : favorites)
cs.favorConstraint(choice);

// Order and partition the disjunction choices.
partitionDisjunction(Ordering, PartitionBeginning);
}
Expand Down Expand Up @@ -6232,9 +6312,8 @@ class DisjunctionChoiceProducer : public BindingProducer<DisjunctionChoice> {
// Partition the choices in the disjunction into groups that we will
// iterate over in an order appropriate to attempt to stop before we
// have to visit all of the options.
void
partitionDisjunction(SmallVectorImpl<unsigned> &Ordering,
SmallVectorImpl<unsigned> &PartitionBeginning);
void partitionDisjunction(SmallVectorImpl<unsigned> &Ordering,
SmallVectorImpl<unsigned> &PartitionBeginning);

/// Partition the choices in the range \c first to \c last into groups and
/// order the groups in the best order to attempt based on the argument
Expand Down
3 changes: 3 additions & 0 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1859,6 +1859,9 @@ 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
1 change: 0 additions & 1 deletion lib/Sema/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ add_swift_host_library(swiftSema STATIC
CSStep.cpp
CSTrail.cpp
CSFix.cpp
CSOptimizer.cpp
CSDiagnostics.cpp
CodeSynthesis.cpp
CodeSynthesisDistributedActor.cpp
Expand Down
33 changes: 2 additions & 31 deletions lib/Sema/CSBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,14 @@ using namespace swift;
using namespace constraints;
using namespace inference;


void ConstraintGraphNode::initBindingSet() {
ASSERT(!hasBindingSet());
ASSERT(forRepresentativeVar());

Set.emplace(CG.getConstraintSystem(), TypeVar, Potential);
}

/// Check whether there exists a type that could be implicitly converted
/// to a given type i.e. is the given type is Double or Optional<..> this
/// function is going to return true because CGFloat could be converted
/// to a Double and non-optional value could be injected into an optional.
static bool hasConversions(Type);

static std::optional<Type> checkTypeOfBinding(TypeVariableType *typeVar,
Type type);

Expand Down Expand Up @@ -1310,31 +1305,7 @@ bool BindingSet::isViable(PotentialBinding &binding, bool isTransitive) {
if (!existingNTD || NTD != existingNTD)
continue;

// What is going on in this method needs to be thoroughly re-evaluated!
//
// This logic aims to skip dropping bindings if
// collection type has conversions i.e. in situations like:
//
// [$T1] conv $T2
// $T2 conv [(Int, String)]
// $T2.Element equal $T5.Element
//
// `$T1` could be bound to `(i: Int, v: String)` after
// `$T2` is bound to `[(Int, String)]` which is is a problem
// because it means that `$T2` was attempted to early
// before the solver had a chance to discover all viable
// bindings.
//
// Let's say existing binding is `[(Int, String)]` and
// relation is "exact", in this case there is no point
// tracking `[$T1]` because upcasts are only allowed for
// subtype and other conversions.
if (existing->Kind != AllowedBindingKind::Exact) {
if (existingType->isKnownStdlibCollectionType() &&
hasConversions(existingType)) {
continue;
}
}
// FIXME: What is going on here needs to be thoroughly re-evaluated.

// If new type has a type variable it shouldn't
// be considered viable.
Expand Down
Loading