[CSSolver] Implementation of disjunction choice favoring algorithm #63585
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please test source compatibility |
xedin
force-pushed
the
solver-perf
branch
2 times, most recently
from
b45df09 to
2a3a206
Compare
|
@swift-ci please test source compatibility |
|
Please test with following pull request: @swift-ci please test source compatibility |
|
Please test with following pull request: @swift-ci please test source compatibility |
|
Please test with following pull request: @swift-ci please test compiler performance |
1 similar comment
|
Please test with following pull request: @swift-ci please test compiler performance |
xedin
changed the title
xedin
changed the title
|
@swift-ci please test source compatibility |
|
@swift-ci please test source compatibility |
|
@swift-ci please test source compatibility |
|
@swift-ci please test compiler performance |
Summary for main fullRegressions found (see below) Debug-batchdebug-batch briefRegressed (0)
Improved (1)
Unchanged (delta < 1.0% or delta < 100.0ms) (1)
debug-batch detailedRegressed (7)
Improved (18)
Unchanged (delta < 1.0% or delta < 100.0ms) (285)
Releaserelease briefRegressed (0)
Improved (1)
Unchanged (delta < 1.0% or delta < 100.0ms) (1)
release detailedRegressed (6)
Improved (17)
Unchanged (delta < 1.0% or delta < 100.0ms) (287)
|
|
Slightly better - 60% reduction in |
|
@swift-ci please test source compatibility |
1 similar comment
|
@swift-ci please test source compatibility |
|
@swift-ci please test compiler performance |
|
@swift-ci please test source compatibility release |
Summary for main fullRegressions found (see below) Debug-batchdebug-batch briefRegressed (0)
Improved (1)
Unchanged (delta < 1.0% or delta < 100.0ms) (1)
debug-batch detailedRegressed (13)
Improved (19)
Unchanged (delta < 1.0% or delta < 100.0ms) (278)
Releaserelease briefRegressed (1)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (1)
release detailedRegressed (25)
Improved (14)
Unchanged (delta < 1.0% or delta < 100.0ms) (271)
|
…disjunction was not optimized Some disjunctions e.g. explicit coercions, compositions of restrictions, and implicit CGFloat initializers have favored choices set independently from optimization algorithm, `selectDisjunction` should account for such situations.
…irements are unsatisfiable If some of the requirements of a generic overload reference other generic parameters, the optimizer won't be able to satisfy them because it only has candidates for one (current) parameter. In cases like that, let's fallback to a light-weight protocol conformance check instead of skipping an overload choice altogether.
If a disjunction has favored choices, let's mark it as optimized with a high score to make sure that such disjunctions are prioritized since only disjunctions that could have their choices fovored independently from the optimization algorithm are compiler synthesized ones for things like IUO references, explicit coercions etc.
If disjunction represents a member reference that has no arguments applied, let's score that as `1` to indicate that it should be priorized. This helps in situations like `a.b + 1` where resolving `a.b` member chain helps to establish context for `+`.
…ings (v2) The original attempt to do this was reverted by swiftlang#77653 The problem is that the fix was too broad, I narrowed it down to non-exact uses of stdlib collections that support upcasts.
…injected into an optional
If result of `CGFloat` -> `Double` conversion is injected into an optional
it should be ranked based on depth just like when locator is fully simplified.
For example:
```swift
func test(v: CGFloat?) {
_ = v ?? 2.0 / 3.0
}
```
In this expression division should be performed on `Double` and result
narrowed down (based on the rule that narrowing conversion should always
be delayed) but `Double` -> `CGFloat?` was given an incorrect score and
instead of picking `?? (_: T?, _: T) -> T` overload, the solver would
use `?? (_: T?, _: T?) -> T?`.
…atching candidate arguments Allow CGFloat -> Double widening conversions between candidate argument types and parameter types. This would make sure that Double is always preferred over CGFloat when using literals and ranking supported disjunction choices. Narrowing conversion (Double -> CGFloat) should be delayed as much as possible.
…er contexts
Prioritize `build{Block, Expression, ...}` and any chained
members that are connected to individual builder elements
i.e. `ForEach(...) { ... }.padding(...)`, once `ForEach`
is resolved, `padding` should be prioritized because its
requirements can help prune the solution space before the
body is checked.
Let's not perform $T? -> $T for closure result types to avoid having to re-discover solutions that differ only in location of optional injection. The pattern with such type variables is: ``` $T_body <conv/subtype> $T_result <conv/subtype> $T_contextual_result ``` When `$T_contextual_result` is `Optional<$U>`, the optional injection can either happen from `$T_body` or from `$T_result` (if `return` expression is non-optional), if we allow both the solver would find two solutions that differ only in location of optional injection.
This would make sure that any same-type requirements to a concrete
type are substituted with such types which is especially important
for SIMD operators like `&{+, -}` because they have `Scalar == (U)Int*`
requirements.
…izer calls with a single unlabeled parameter
Mitigation for a historic incorrect type-checker behavior
caused by one of the performance hacks that used to favor
sync constructor overload over async one in async context
if initializers take a single unlabeled argument.
```swift
struct S {
init(_: Int) {}
init(_: Int) async {}
}
func test(v: Int) async { S(v) }
```
The type-checker should use `init(_: Int) async` in `test` context
but used to select the sync overload. The hack is now gone but we
need to downgrade an error to a warning to give the developers time
to fix their code.
If a disjunction doesn't have an application, let's prefer it if it's passed as an argument to an operator application or involved in a member chain, in such situations attempting a disjunction could provide context to parent call/member chain. If disjunction is passed as an unapplied reference to some parameter i.e. `<base>.map(String.init(describing:))` we don't favor it for now because optimizer cannot do enough checking to determine whether preferring such disjunction would help make forward progress in solving by pruning some space or providing additional context.
Some of the unary operators, i.e. prefix `-`, don't have CGFloat variants and expect generic `FloatingPoint` overload to match CGFloat type. Let's not attempt `CGFloat` -> `Double` conversion for unary operators because it always leads to a worse solutions vs. generic overloads.
xedin
requested review from
a team,
hborla,
slavapestov,
ahoppen,
bnbarham,
hamishknight and
rintaro
as code owners
|
@swift-ci please clean test |
|
@swift-ci please test source compatibility |
This was referenced Dec 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This algorithm attempts to ensure that the solver always picks a disjunction
it knows the most about given the previously deduced type information.
For example in chains of operators like:
let _: (Double) -> Void = { 1 * 2 + $0 - 5 }The solver is going to start from
2 + $0because$0is known to beDoubleandthen proceed to
1 * ...and only after that to... - 5.The algorithm is pretty simple:
Resolves: rdar://60347805
Resolves: rdar://74387235
Resolves: rdar://78845822
Resolves: rdar://23682605
Resolves: rdar://18994321
Resolves: rdar://107263264
Resolves: rdar://135523718