[5.1] One-way constraints for function builders #26661
Merged
Conversation
This file contains 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
The ConstraintSystem class is on the order of 1000s of bytes in size on the stacka nd is causing issues with dispatch's 64k stack limit. This changes most Small data types which store data on the stack to non small heap based data types.
The constraint graph maintains a fairly heavyweight list of adjacencies that is only used in two operations: connected components and gathering related constraints. Switch connected components over to primarily use the set of constraints (which are necessary for many reasons), reducing the need for the adjacencies list. (cherry picked from commit 6a970a8)
(cherry picked from commit 6737cf9)
…adjacencies list. Use the adjacencies implied by the constraints of a node rather than looking at the "adjacency" list, and try to simplify this code path a bit. The diagnostic change is because we are now uniformly recording the members of the equivalence class. (cherry picked from commit 54bdd7b)
Each constraint graph node maintained adjacency information that was fairly expensive---a vector of type variables and a dense map of extra adjacency information---and that was continuously maintained. Remove this adjacency information, instead recomputing it by walking the constraints (to get their type variables) and having a separate (smaller) list of type variables that are adjacent due to fixed bindings. This simplifies the constraint graph code and reduces per-node memory overhead. (cherry picked from commit dfdd352)
Simplify the interface to gatherConstraints() by performing the uniquing within the function itself and returning only the resulting (uniqued) vector of constraints. (cherry picked from commit 8355f3d)
…riables. The API of the connected-components algorithm asks clients to provide the set of type variables of interest. However, the connected components algorithm itself was operating across the entire set of type variables, then narrowing the result down to the type variables of interest. Instead, only perform connected components on those type variables of interest, so that we are only doing work proportional to the subgraph we're working in. (cherry picked from commit 62502ef)
Address a silly mistake that meant we were miscomputing connected components. (cherry picked from commit 6be6401)
…iables. Simplify the connected-components computation slightly and make sure that it never performs work outside of the subgraph described by the input set of type variables. (cherry picked from commit 14ca9d1)
…ere. When we are associating type variables with components in a “split” step, there is no need to record already-bound type variables with every component. (cherry picked from commit 0b61f86)
…st now (cherry picked from commit c2a9286)
The connected components computation was not forming components when all of the type variables in a component were already bound. Any remaining constraints involving those type variables would then arbitrarily end up in component 0, due to the the default-construction behavior of a map’s operator[] with missing keys, artificially increasing the size of that component with (typically) additional disjunctions. Ensure that all constraints get into a component, creating one to hold the a constraint even when all of the type variables are already bound. Then, assert the invariant that every constraint is associated with a component. In time, we should probably eliminate this notion of disjunctions that remain even when the type variable has been bound. For now, strengthen the invariant to at least ensure that they get solved in isolation. (cherry picked from commit 805b02d)
Have the constraint graph's connected-component implementation be more self-contained, producing a vector containing each of the actual components (where each is defined by a list of type variables and a list of constraints). This simplifies the contract with the client (SplitterStep) and eliminates a bunch of separate mapping steps to interpret the results. It also lets us enrich the Component data structure in the future. (cherry picked from commit 4c04ced)
Maintain the order of constraints when splitting the system into connected components, to match the behavior prior to the refactoring into a separate connected-components algorithm. (cherry picked from commit 5a4af23)
…nents Move the logic for creating connected components of orphaned constraints into the connected-components algorithm code, rather than making it a special part of SplitterStep. (cherry picked from commit ab38be1)
(cherry picked from commit dec149c)
Introduce the notion of "one-way" binding constraints of the form $T0 one-way bind to $T1 which treats the type variables $T0 and $T1 as independent up until the point where $T1 simplifies down to a concrete type, at which point $T0 will be bound to that concrete type. $T0 won't be bound in any other way, so type information ends up being propagated right-to-left, only. This allows a constraint system to be broken up in more components that are solved independently. Specifically, the connected components algorithm now proceeds as follows: 1. Compute connected components, excluding one-way constraints from consideration. 2. Compute a directed graph amongst the components using only the one-way constraints, where an edge A -> B indicates that the type variables in component A need to be solved before those in component B. 3. Using the directed graph, compute the set of components that need to be solved before a given component. To utilize this, implement a new kind of solver step that handles the propagation of partial solutions across one-way constraints. This introduces a new kind of "split" within a connected component, where we collect each combination of partial solutions for the input components and (separately) try to solve the constraints in this component. Any correct solution from any of these attempts will then be recorded as a (partial) solution for this component. For example, consider: let _: Int8 = b ? Builtin.one_way(int8Or16(17)) : Builtin.one_way(int8Or16(42\ )) where int8Or16 is overloaded with types `(Int8) -> Int8` and `(Int16) -> Int16`. There are two one-way components (`int8Or16(17)`) and (`int8Or16(42)`), each of which can produce a value of type `Int8` or `Int16`. Those two components will be solved independently, and the partial solutions for each will be fed into the component that evaluates the ternary operator. There are four ways to attempt that evaluation: ``` [Int8, Int8] [Int8, Int16] [Int16, Int8] [Int16, Int16] To test this, introduce a new expression builtin `Builtin.one_way(x)` that introduces a one-way expression constraint binding the result of the expression 'x'. The builtin is meant to be used for testing purposes, and the one-way constraint expression itself can be synthesized by the type checker to introduce one-way constraints later on. Of these two, there are only two (partial) solutions that can work at all, because the types in the ternary operator need a common supertype: [Int8, Int8] [Int16, Int16] Therefore, these are the partial solutions that will be considered the results of the component containing the ternary expression. Note that only one of them meets the final constraint (convertibility to `Int8`), so the expression is well-formed. Part of rdar://problem/50150793. (cherry picked from commit 3c69f6a)
…ilders When we transform each expression or statement in a function builder, introduce a one-way constraint so that type information does not flow backwards from the context into that statement or expression. This more closely mimics the behavior of normal code, where type inference is per-statement, flowing from top to bottom. This also allows us to isolate different expressions and statements within a closure that's passed into a function builder parameter, reducing the search space and (hopefully) improving compile times for large function builder closures. For now, put this functionality behind the compiler flag `-enable-function-builder-one-way-constraints` for testing purposes; we still have both optimization and correctness work to do to turn this on by default. (cherry picked from commit be73a9d)
…er calls We need both sides of a buildEither constraint to be unified, so don’t inject one-way constraints on the inputs to buildEither. (cherry picked from commit 7aecca0)
…ponents. Helps with debugging the constraint graph. (cherry picked from commit 0b7ef34)
Refactoring so we can use this in a moment. (cherry picked from commit f19016b)
When we encounter a cycle in the one-way component graph due to constraints that (e.g.) tie together the outputs of two one-way constraints, collapse the components along the cycle into a single connected component, because all of those type variables must be solved together. (cherry picked from commit 73e5a64)
We only care about gathering a one-way constraint if (1) the left-hand side is in the set of type variables we care about now, and (2) the type variable we started from is in the right-hand side.
There were a few places where we wanted fast testing to see whether a particular type variable is currently of interest. Instead of building local hash tables in those places, keep type variables in a SetVector for efficient testing.
One-way constraint expressions, which are the only things that introduce one-way constraints at this point, want to look through lvalue types to produce values. Rename OneWayBind to OneWayEqual, map it down to an Equal constraint when it is simplified (to drop lvalue-ness), and apply that coercion during constraint application. Part of rdar://problem/50150793.
Enable one-way constraints by default for function builders, finishing rdar://problem/50150793. (cherry picked from commit b4e80cf)
Function builders perform the expression "pre-check" operation multiple times, which means that nested closures might have a more-nested DeclContext than PreCheck expects. Loosen this assertion to cope with that change. (cherry picked from commit 79c5706)
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
|
@swift-ci please test compiler performance |
|
Build failed |
|
Build failed |
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
Reinstate the list of adjacencies in each constraint graph node, effectively reverting dfdd352. Exclude one-way constraints from this computation; we'll handle them separately.
This reinstates the use of direct adjacency information when gathering constraints, effectively reverting 54bdd7b. Fixes the regression that commit caused, which is tracked by rdar://problem/54274245.
|
@swift-ci please test |
|
@swift-ci please test compiler performance |
|
@swift-ci please test |
|
@swift-ci please test compiler performance |
DougGregor
changed the title
Summary for swift-5.1-branch fullUnexpected test results, excluded stats for SwifterSwift, Base64CoderSwiftUI Regressions found (see below) Debug-batchdebug-batch briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
debug-batch detailedRegressed (3)
Improved (3)
Unchanged (delta < 1.0% or delta < 100.0ms) (104)
Releaserelease briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
release detailedRegressed (3)
Improved (1)
Unchanged (delta < 1.0% or delta < 100.0ms) (106)
|
AnnaZaks
approved these changes
Sep 3, 2019
|
@swift-ci build toolchain |
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.
Cherry-pick the work to enable one-way constraints for function builders to the 5.1 branch as an experiment. Implements rdar://problem/50150793.