Skip to content
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

Treat length of sequences as a "slot" in Useless optimization #586

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

MatthewFluet
Copy link
Member

@MatthewFluet MatthewFluet commented Dec 20, 2024

In the Value.t representation of the Useless optimization, sequence and tuple elements are "slots", which combine a Useful.t lattice element with an Exists.t lattice element. When a value flows, vectors and tuples coerce the Useful.t component of slots but unify the Exists.t component. This ensures that agree on whether or not the element exists even if they disagree that the element is useful. If the "from"'s element is useful (and necessarily existing) but the "to"'s element is not useful, then forcing the "to"'s element to exist avoids a potentially expensive runtime coercion (e.g., to drop a component of a tuple or, worse, to drop a component of a tuple that is the contents of a vector).

Previously, the length of a sequence was represented simply as a Useful.t; this allowed a vector (whose contents were not useful) with a useful length to flow to a vector (whose contents are necessarily not useful) with a useless length. 1aad5fe (Optimize representation of sequences in Useless pass; #569) allowed the Useless optimization to change the representation of sequence with useless contents. In particular, a vector with useless contents but useful length becomes a word64 and a vector with useless contents and useless length becomes a unit.

However, when such vectors are themselves components of a tuple, then the program may have a flow of tuples, where the source tuple is changed from (..., ?? vector, ...) to (..., word64, ...) but the destination tuple is changed from (..., ?? vector, ...) to (..., unit, ...). (Note that the unification of the Exist.t components of the corresponding tuple elements is what makes the destination tuple have a unit element.)

This commit treats the length of arrays and vectors as a "slot", so that they track both usefulness and existence. Now, a vector with useless contents but a length that must exist becomes a word64 (even if the length is useless) and a vector with useless contents and a length that need not exist (and is necessarily useless) becomes a unit.

Fixes #585.

@fweimer
Copy link

fweimer commented Dec 20, 2024

Presumably a vector with useless contents but useless length should be a vector with useful contents but useless length.

In the `Value.t` representation of the Useless optimization, sequence and tuple
elements are "slots", which combine a `Useful.t` lattice element with an
`Exists.t` lattice element.  When a value flows, vectors and tuples coerce the
`Useful.t` component of slots but unify the `Exists.t` component.  This ensures
that agree on whether or not the element exists even if they disagree that the
element is useful.  If the "from"'s element is useful (and necessarily existing)
but the "to"'s element is not useful, then forcing the "to"'s element to exist
avoids a potentially expensive runtime coercion (e.g., to drop a component of a
tuple or, worse, to drop a component of a tuple that is the contents of a
vector).

Previously, the length of a sequence was represented simply as a `Useful.t`;
this allowed a vector (whose contents were not useful) with a useful length to
flow to a vector (whose contents are necessarily not useful) with a useless
length.  1aad5fe (Optimize representation of sequences in Useless pass;
MLton#569) allowed the Useless optimization to change the representation
of sequence with useless contents.  In particular, a vector with useless
contents but useful length becomes a `word64` and a vector with useless contents
and useless length becomes a `unit`.

However, when such vectors are themselves components of a tuple, then the
program may have a flow of tuples, where the source tuple is changed from
`(..., ?? vector, ...)` to `(..., word64, ...)` but the destination tuple is
changed from `(..., ?? vector, ...)` to `(..., unit, ...)`.  (Note that the
unification of the `Exist.t` components of the corresponding tuple elements is
what makes the destination tuple have a `unit` element.)

This commit treats the length of arrays and vectors as a "slot", so that they
track both usefulness and existence.  Now, a vector with useless contents but a
length that must exist becomes a `word64` (even if the length is useless) and a
vector with useless contents and a length that need not exist (and is
necessarily useless) becomes a `unit`.

Fixes MLton#585.
@MatthewFluet
Copy link
Member Author

No, the typo was the other way around. It should read a vector with useless contents but useful length becomes a word64. I'll fix the commit message and the PR text.

@MatthewFluet MatthewFluet merged commit 665812b into MLton:master Dec 20, 2024
12 checks passed
@MatthewFluet MatthewFluet deleted the useless-bugfix branch December 20, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strange split types coercion
2 participants