Skip to content

Commit 725bd91

Browse files
authored
[ConstraintSystem] Revert new disjunction favoring algorithm (#79128)
* Revert "[CSOptimizer] Look through `OptionalEvaluationExpr`s when dealing with unapplied disjunctions" This reverts commit 72340f3. * Revert "[CSOptimizer] Don't consider disabled overloads when checking whether disjunction is supported" This reverts commit 6bc23b5. * Revert "[CSOptimizer] Disjunctions with IUO overload choices are unsupported" This reverts commit 471ee21. * Revert "[CSOptimizer] MemberImportVisibility: Don't consider overloads that come from implicit imports" This reverts commit aa4a2b9. * Revert "[CSOptimizer] Don't consider CGFloat widening when explicit initializer is used" This reverts commit 3cc76ea. * Revert "[CSOptimizer] Literal arguments should cause score reset only for operators" This reverts commit e3987be. * Revert "[CSOptimizer] NFC: check whether a choice is of operator instead of whole disjunction" This reverts commit 6c82892. * Revert "[CSOptimizer/Tests] NFC: Add a perf test-case fixed by improved literal array handling" This reverts commit cfd34e5. * Revert "[CSOptimizer] Extend candidate/parameter matching to support array literals" This reverts commit 8a304f8. * Revert "[CSOptimizer] Favor choices that don't require application" This reverts commit 0737542. * Revert "[CSOptimizer] Disable CGFloat -> Double conversion for unary operators" This reverts commit bc3a15f. * Revert "[CSOptimizer] Mark bitwise operators as supported" This reverts commit 860ae08. * Revert "[CSOptimizer] Simplify handling of non-applied disjunctions" This reverts commit 43ca7df. * Revert "[ConstraintSystem] Fix `getEffectiveOverloadType` handling of `mutating` methods" This reverts commit c767f7a. * Revert "[CSOptimizer] Reduce overload types before ranking" This reverts commit 95b47ae. * Revert "[CSOptimizer] Implement special prioritization rules for result builder contexts" This reverts commit 56d6635. * Revert "[CSOptimizer] Allow only widening CGFloat->Double conversions while matching candidate arguments" This reverts commit bf8ae3b. * Revert "[CSSimplify] CGFloat-Double: Rank narrowing correctly when result is injected into an optional" This reverts commit cb876cb. * Revert "[CSBindings] Prevent `BindingSet::isViable` from dropping viable bindings (v2)" This reverts commit b7e7493. * Revert "[CSOptimizer] Add support for chained members without arguments" This reverts commit 87cd5f8. * Revert "[CSOptimizer] Mark compiler synthesized disjunctions as optimized" This reverts commit 867e641. * Revert "[CSOptimizer] Make a light-weight generic overload check if some requirements are unsatisfiable" This reverts commit 15c773b. * Revert "[CSOptimizer] Fix `selectDisjunction` to use favored choices even if disjunction was not optimized" This reverts commit c2a5588. * Revert "[CSOptimizer] Limit "old" behavior compatibility to unlabeled unary arguments" This reverts commit 9fb7314. * Revert "[Tests] NFC: Update a couple of type-checker tests" This reverts commit ff8663f. * Revert "[Tests] NFC: Move simd related test-case from `slow` to `fast`" This reverts commit 28396a6. * Revert "[CSGen] NFC: Remove obsolete `ConstraintSystem::{get, set}FavoredType`" This reverts commit 8bd2884. * Revert "[CSOptimizer] Allow literal arguments to match parameters that conform to `ExpressibleBy{Integer, Float}Literal`" This reverts commit 2fdd4b6. * Revert "[CSOptimizer] Adjust `scoreCandidateMatch` to indicate when match cannot be decided" This reverts commit 9b62c84. * Revert "[CSOptimizer] Fix Double<->CGFloat implicit conversion support when arguments are literals" This reverts commit 6caf1cc. * Revert "[CSOptimizer] A more comprehensive generic overload checking when candidates are fully resolved" This reverts commit e30587b. * Revert "[CSOptimizer] Restore old hack behavior which used to favor overloads based on arity matches" This reverts commit a3a3ec4. * Revert "[CSOptimizer] Desugar types before checking for equality" This reverts commit 802f5cd. * Revert "[ConstraintSystem] Narrowly disable `tryOptimizeGenericDisjunction` when some of the arguments are number literals" This reverts commit 8d5cb11. * Revert "[CSOptimizer] Infer argument candidates from calls to `Double` and CGFloat constructors" This reverts commit f2a6677. * Revert "[CSOptimizer] Score all of the overload choices matching on literals uniformly" This reverts commit 59109c2. * Revert "[CSOptimizer] Enable ranking of `Int*`, `Float{80}` and `Double` initializers" This reverts commit 6fb6d1c. * Revert "[CSOptimizer] Rank disjunctions based on score only if both sides are supported" This reverts commit 8818d39. * Revert "[CSOptimizer] Rank results of operators regardless of whether anything is known about parameters" This reverts commit 3996b25. * Revert "[Tests] NFC: Add more test-cases that were previously solved due to old hacks behavior" This reverts commit d0ff6c8. * Revert "[CSOptimizer] Average score should reflect number of defaulted parameters" This reverts commit 23589ad. * Revert "[Tests] NFC: Adjust a couple of improved tests" This reverts commit 6698136. * Revert "[CSOptimizer] Don't optimize (implicit) calls with code completion arguments" This reverts commit 8a918e2. * Revert "[CSOptimizer] attempt to rank only standard/simd operators and fully concrete overload sets" This reverts commit deca9b6. * Revert "[CSOptimizer] Record best scores for each disjunction and use them in `selectDisjunction`" This reverts commit 3819ddf. * Revert "[CSOptimizer] Let `determineBestChoicesInContext` return the best disjunction if one is available" This reverts commit cf05405. * Revert "[CSOptimizer] Emulate old behavior related to favoring of unary calls to members" This reverts commit 527de22. * Revert "[Tests] NFC: Add a test-case for rdar://133340307 which is now fast" This reverts commit 670127a. * Revert "[CSOptimizer] Prefer homogeneous arithmetic operator overloads when argument(s) or result match" This reverts commit d69b6a0. * Revert "[CSOptimizer] Remove an outdated optimization to compare resolved argument types with all else equal" This reverts commit 1760bd1. * Revert "[CSOptimizer] NFC: Switch from llvm::Optional to std::optional post-rebase" This reverts commit c429f5b. * Revert "[CSOptimizer] Increase score when type matches opaque type" This reverts commit 2869dff. * Revert "[CSOptimizer] NFC: Switch to llvm::Optional" This reverts commit 0fc6806. * Revert "[CSOptimizer] NFC: Adjust conformance check to use `ConstraintSystem::lookupConformance`" This reverts commit da65333. * Revert "[CSOptimizer] Treat all type parameters equally" This reverts commit 957a5f4. * Revert "[CSStep] Remove disjunction pruning logic from DisjunctionStep" This reverts commit 2c44e37. * Revert "[CSOptimizer] Relax candidate type requirements from equality to set of no-impact conversions" This reverts commit 11b897b. * Revert "[CSOptimizer] Use `matchCallArguments` to establish argument-to-parameter relationships" This reverts commit cb1cb20. * Revert "[CSOptimizer] Don't attempt to optimize calls with code completion token(s) in argument position" This reverts commit 14e2a16. * Revert "[CSOptimizer] Allow generic operator overloads without associated type parameters" This reverts commit bc5f70a. * Revert "[CSOptimizer] Make sure that all parameters without arguments are defaulted" This reverts commit 7c1c46d. * Revert "[CSStep] Don't favor choices until the disjunction is picked" This reverts commit e404ed7. * Revert "[CSOptimizer] Keep track of mismatches while evaluating candidates" This reverts commit a094c3e. * Revert "[CSOptimizer] Favor SIMD related arithmetic operator choices if argument is SIMD<N> type" This reverts commit c2f7451. * Revert "[CSOptimizer] Initial implementation of disjunction choice favoring algorithm" This reverts commit 672ae3d. * Revert "[ConstraintSystem] Add skeleton of constraint optimizer" This reverts commit b5f08a4. * Revert "[CSGen] Remove ConstraintOptimizer and all favoring logic" This reverts commit 4432c51. * Revert "[ConstraintSystem] Remove `shrink`" This reverts commit 757ca24. * [TypeChecker] NFC: Remove resurrected use of `SolverShrinkUnsolvedThreshold` * [TypeChecker] Bring back `SolverDisableShrink` * [Tests] NFC: Mark tests affected by solver-perf revert as slow * [Tests] NFC: Adjust async tests that are affected by performance hacks
1 parent 48a4013 commit 725bd91

File tree

56 files changed

+1729
-2215
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+1729
-2215
lines changed

include/swift/Basic/LangOptions.h

+3
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,9 @@ namespace swift {
905905
/// is for testing purposes.
906906
std::vector<std::string> DebugForbidTypecheckPrefixes;
907907

908+
/// Disable the shrink phase of the expression type checker.
909+
bool SolverDisableShrink = false;
910+
908911
/// Enable experimental operator designated types feature.
909912
bool EnableOperatorDesignatedTypes = false;
910913

include/swift/Option/FrontendOptions.td

+4
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,10 @@ def solver_scope_threshold_EQ : Joined<["-"], "solver-scope-threshold=">,
836836
def solver_trail_threshold_EQ : Joined<["-"], "solver-trail-threshold=">,
837837
HelpText<"Expression type checking trail change limit">;
838838

839+
def solver_disable_shrink :
840+
Flag<["-"], "solver-disable-shrink">,
841+
HelpText<"Disable the shrink phase of expression type checking">;
842+
839843
def solver_disable_splitter : Flag<["-"], "solver-disable-splitter">,
840844
HelpText<"Disable the component splitter phase of expression type checking">;
841845

include/swift/Sema/Constraint.h

+5
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,11 @@ class Constraint final : public llvm::ilist_node<Constraint>,
840840
});
841841
}
842842

843+
/// Returns the number of resolved argument types for an applied disjunction
844+
/// constraint. This is always zero for disjunctions that do not represent
845+
/// an applied overload.
846+
unsigned countResolvedArgumentTypes(ConstraintSystem &cs) const;
847+
843848
/// Determine if this constraint represents explicit conversion,
844849
/// e.g. coercion constraint "as X" which forms a disjunction.
845850
bool isExplicitConversion() const;

include/swift/Sema/ConstraintSystem.h

+100-21
Original file line numberDiff line numberDiff line change
@@ -488,14 +488,6 @@ class TypeVariableType::Implementation {
488488
/// literal (represented by `ArrayExpr` and `DictionaryExpr` in AST).
489489
bool isCollectionLiteralType() const;
490490

491-
/// Determine whether this type variable represents a literal such
492-
/// as an integer value, a floating-point value with and without a sign.
493-
bool isNumberLiteralType() const;
494-
495-
/// Determine whether this type variable represents a result type of a
496-
/// function call.
497-
bool isFunctionResult() const;
498-
499491
/// Retrieve the representative of the equivalence class to which this
500492
/// type variable belongs.
501493
///
@@ -2250,6 +2242,10 @@ class ConstraintSystem {
22502242

22512243
llvm::SetVector<TypeVariableType *> TypeVariables;
22522244

2245+
/// Maps expressions to types for choosing a favored overload
2246+
/// type in a disjunction constraint.
2247+
llvm::DenseMap<Expr *, TypeBase *> FavoredTypes;
2248+
22532249
/// Maps discovered closures to their types inferred
22542250
/// from declared parameters/result and body.
22552251
///
@@ -2462,6 +2458,74 @@ class ConstraintSystem {
24622458
SynthesizedConformances;
24632459

24642460
private:
2461+
/// Describe the candidate expression for partial solving.
2462+
/// This class used by shrink & solve methods which apply
2463+
/// variation of directional path consistency algorithm in attempt
2464+
/// to reduce scopes of the overload sets (disjunctions) in the system.
2465+
class Candidate {
2466+
Expr *E;
2467+
DeclContext *DC;
2468+
llvm::BumpPtrAllocator &Allocator;
2469+
2470+
// Contextual Information.
2471+
Type CT;
2472+
ContextualTypePurpose CTP;
2473+
2474+
public:
2475+
Candidate(ConstraintSystem &cs, Expr *expr, Type ct = Type(),
2476+
ContextualTypePurpose ctp = ContextualTypePurpose::CTP_Unused)
2477+
: E(expr), DC(cs.DC), Allocator(cs.Allocator), CT(ct), CTP(ctp) {}
2478+
2479+
/// Return underlying expression.
2480+
Expr *getExpr() const { return E; }
2481+
2482+
/// Try to solve this candidate sub-expression
2483+
/// and re-write it's OSR domains afterwards.
2484+
///
2485+
/// \param shrunkExprs The set of expressions which
2486+
/// domains have been successfully shrunk so far.
2487+
///
2488+
/// \returns true on solver failure, false otherwise.
2489+
bool solve(llvm::SmallSetVector<OverloadSetRefExpr *, 4> &shrunkExprs);
2490+
2491+
/// Apply solutions found by solver as reduced OSR sets for
2492+
/// for current and all of it's sub-expressions.
2493+
///
2494+
/// \param solutions The solutions found by running solver on the
2495+
/// this candidate expression.
2496+
///
2497+
/// \param shrunkExprs The set of expressions which
2498+
/// domains have been successfully shrunk so far.
2499+
void applySolutions(
2500+
llvm::SmallVectorImpl<Solution> &solutions,
2501+
llvm::SmallSetVector<OverloadSetRefExpr *, 4> &shrunkExprs) const;
2502+
2503+
/// Check if attempt at solving of the candidate makes sense given
2504+
/// the current conditions - number of shrunk domains which is related
2505+
/// to the given candidate over the total number of disjunctions present.
2506+
static bool
2507+
isTooComplexGiven(ConstraintSystem *const cs,
2508+
llvm::SmallSetVector<OverloadSetRefExpr *, 4> &shrunkExprs) {
2509+
SmallVector<Constraint *, 8> disjunctions;
2510+
cs->collectDisjunctions(disjunctions);
2511+
2512+
unsigned unsolvedDisjunctions = disjunctions.size();
2513+
for (auto *disjunction : disjunctions) {
2514+
auto *locator = disjunction->getLocator();
2515+
if (!locator)
2516+
continue;
2517+
2518+
if (auto *OSR = getAsExpr<OverloadSetRefExpr>(locator->getAnchor())) {
2519+
if (shrunkExprs.count(OSR) > 0)
2520+
--unsolvedDisjunctions;
2521+
}
2522+
}
2523+
2524+
// The threshold used to be `TypeCheckerOpts.SolverShrinkUnsolvedThreshold`
2525+
return unsolvedDisjunctions >= 10;
2526+
}
2527+
};
2528+
24652529
/// Describes the current solver state.
24662530
struct SolverState {
24672531
SolverState(ConstraintSystem &cs,
@@ -2985,6 +3049,15 @@ class ConstraintSystem {
29853049
return nullptr;
29863050
}
29873051

3052+
TypeBase* getFavoredType(Expr *E) {
3053+
assert(E != nullptr);
3054+
return this->FavoredTypes[E];
3055+
}
3056+
void setFavoredType(Expr *E, TypeBase *T) {
3057+
assert(E != nullptr);
3058+
this->FavoredTypes[E] = T;
3059+
}
3060+
29883061
/// Set the type in our type map for the given node, and record the change
29893062
/// in the trail.
29903063
///
@@ -5245,11 +5318,19 @@ class ConstraintSystem {
52455318
/// \returns true if an error occurred, false otherwise.
52465319
bool solveSimplified(SmallVectorImpl<Solution> &solutions);
52475320

5321+
/// Find reduced domains of disjunction constraints for given
5322+
/// expression, this is achieved to solving individual sub-expressions
5323+
/// and combining resolving types. Such algorithm is called directional
5324+
/// path consistency because it goes from children to parents for all
5325+
/// related sub-expressions taking union of their domains.
5326+
///
5327+
/// \param expr The expression to find reductions for.
5328+
void shrink(Expr *expr);
5329+
52485330
/// Pick a disjunction from the InactiveConstraints list.
52495331
///
5250-
/// \returns The selected disjunction and a set of it's favored choices.
5251-
std::optional<std::pair<Constraint *, llvm::TinyPtrVector<Constraint *>>>
5252-
selectDisjunction();
5332+
/// \returns The selected disjunction.
5333+
Constraint *selectDisjunction();
52535334

52545335
/// Pick a conjunction from the InactiveConstraints list.
52555336
///
@@ -5438,6 +5519,11 @@ class ConstraintSystem {
54385519
bool applySolutionToBody(TapExpr *tapExpr,
54395520
SyntacticElementTargetRewriter &rewriter);
54405521

5522+
/// Reorder the disjunctive clauses for a given expression to
5523+
/// increase the likelihood that a favored constraint will be successfully
5524+
/// resolved before any others.
5525+
void optimizeConstraints(Expr *e);
5526+
54415527
void startExpressionTimer(ExpressionTimer::AnchorType anchor);
54425528

54435529
/// Determine if we've already explored too many paths in an
@@ -6178,8 +6264,7 @@ class DisjunctionChoiceProducer : public BindingProducer<DisjunctionChoice> {
61786264
public:
61796265
using Element = DisjunctionChoice;
61806266

6181-
DisjunctionChoiceProducer(ConstraintSystem &cs, Constraint *disjunction,
6182-
llvm::TinyPtrVector<Constraint *> &favorites)
6267+
DisjunctionChoiceProducer(ConstraintSystem &cs, Constraint *disjunction)
61836268
: BindingProducer(cs, disjunction->shouldRememberChoice()
61846269
? disjunction->getLocator()
61856270
: nullptr),
@@ -6189,11 +6274,6 @@ class DisjunctionChoiceProducer : public BindingProducer<DisjunctionChoice> {
61896274
assert(disjunction->getKind() == ConstraintKind::Disjunction);
61906275
assert(!disjunction->shouldRememberChoice() || disjunction->getLocator());
61916276

6192-
// Mark constraints as favored. This information
6193-
// is going to be used by partitioner.
6194-
for (auto *choice : favorites)
6195-
cs.favorConstraint(choice);
6196-
61976277
// Order and partition the disjunction choices.
61986278
partitionDisjunction(Ordering, PartitionBeginning);
61996279
}
@@ -6238,9 +6318,8 @@ class DisjunctionChoiceProducer : public BindingProducer<DisjunctionChoice> {
62386318
// Partition the choices in the disjunction into groups that we will
62396319
// iterate over in an order appropriate to attempt to stop before we
62406320
// have to visit all of the options.
6241-
void
6242-
partitionDisjunction(SmallVectorImpl<unsigned> &Ordering,
6243-
SmallVectorImpl<unsigned> &PartitionBeginning);
6321+
void partitionDisjunction(SmallVectorImpl<unsigned> &Ordering,
6322+
SmallVectorImpl<unsigned> &PartitionBeginning);
62446323

62456324
/// Partition the choices in the range \c first to \c last into groups and
62466325
/// order the groups in the best order to attempt based on the argument

lib/Frontend/CompilerInvocation.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -1856,6 +1856,9 @@ static bool ParseTypeCheckerArgs(TypeCheckerOptions &Opts, ArgList &Args,
18561856
Opts.DebugForbidTypecheckPrefixes.push_back(A);
18571857
}
18581858

1859+
if (Args.getLastArg(OPT_solver_disable_shrink))
1860+
Opts.SolverDisableShrink = true;
1861+
18591862
if (Args.getLastArg(OPT_solver_disable_splitter))
18601863
Opts.SolverDisableSplitter = true;
18611864

lib/Sema/CMakeLists.txt

-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ add_swift_host_library(swiftSema STATIC
1313
CSStep.cpp
1414
CSTrail.cpp
1515
CSFix.cpp
16-
CSOptimizer.cpp
1716
CSDiagnostics.cpp
1817
CodeSynthesis.cpp
1918
CodeSynthesisDistributedActor.cpp

lib/Sema/CSBindings.cpp

+2-31
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,14 @@ using namespace swift;
3131
using namespace constraints;
3232
using namespace inference;
3333

34+
3435
void ConstraintGraphNode::initBindingSet() {
3536
ASSERT(!hasBindingSet());
3637
ASSERT(forRepresentativeVar());
3738

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

41-
/// Check whether there exists a type that could be implicitly converted
42-
/// to a given type i.e. is the given type is Double or Optional<..> this
43-
/// function is going to return true because CGFloat could be converted
44-
/// to a Double and non-optional value could be injected into an optional.
45-
static bool hasConversions(Type);
46-
4742
static std::optional<Type> checkTypeOfBinding(TypeVariableType *typeVar,
4843
Type type);
4944

@@ -1338,31 +1333,7 @@ bool BindingSet::isViable(PotentialBinding &binding, bool isTransitive) {
13381333
if (!existingNTD || NTD != existingNTD)
13391334
continue;
13401335

1341-
// What is going on in this method needs to be thoroughly re-evaluated!
1342-
//
1343-
// This logic aims to skip dropping bindings if
1344-
// collection type has conversions i.e. in situations like:
1345-
//
1346-
// [$T1] conv $T2
1347-
// $T2 conv [(Int, String)]
1348-
// $T2.Element equal $T5.Element
1349-
//
1350-
// `$T1` could be bound to `(i: Int, v: String)` after
1351-
// `$T2` is bound to `[(Int, String)]` which is is a problem
1352-
// because it means that `$T2` was attempted to early
1353-
// before the solver had a chance to discover all viable
1354-
// bindings.
1355-
//
1356-
// Let's say existing binding is `[(Int, String)]` and
1357-
// relation is "exact", in this case there is no point
1358-
// tracking `[$T1]` because upcasts are only allowed for
1359-
// subtype and other conversions.
1360-
if (existing->Kind != AllowedBindingKind::Exact) {
1361-
if (existingType->isKnownStdlibCollectionType() &&
1362-
hasConversions(existingType)) {
1363-
continue;
1364-
}
1365-
}
1336+
// FIXME: What is going on here needs to be thoroughly re-evaluated.
13661337

13671338
// If new type has a type variable it shouldn't
13681339
// be considered viable.

0 commit comments

Comments
 (0)