[NFC] Use principal types in ChildTyper #7816
Conversation
Refactor ChildTyper to report type constraints using a single callback that takes a sequence of VarTypes from principal-type.h rather than using several callbacks for the various kinds of constraints. The previous approach using several callbacks did not scale well to the various kinds of polymorphism introduced in new Wasm proposals, such as polymorphism over sharedness in `ref.eq` or polymorphism over exactness in `ref.get_desc`. NFC despite better modeling these contraints because ChildTyper is so far only used from TypeSSA, which only uses it to look for exact types, and from IRBuilder, which only uses it to control the construction of unreachable IR. Follow-on changes will use ChildTyper in new ways that require this improved precision.
src/ir/child-typer.h
Outdated
| // noteAnyTupleType(Expression** childp, size_t arity) - The child may have | ||
| // any tuple type with the given arity. Used for the children of polymorphic | ||
| // tuple instructions like `tuple.drop` and `tuple.extract`. | ||
| // Multiple VarTypes if and only if the child must be a tuple. In that case, |
There was a problem hiding this comment.
The first sentence here seems incomplete? Should it be "Multiple VarTypes are provided [..]" perhaps?
src/ir/child-typer.h
Outdated
| // branch target. This callback will only be used when the label type is not | ||
| // passed directly as an argument to the branch visitor method (see below). | ||
| // branch target. This callback will only be used when a the label type is not | ||
| // passed directly as an argumnet to the branch visitor method (see below). |
There was a problem hiding this comment.
Maybe I'll start asking Gemini to proofread my PRs...
| for (auto& expr : curr->operands) { | ||
| noteAny(&expr); | ||
| for (Index i = 0; i < curr->operands.size(); ++i) { | ||
| note(&curr->operands[i], VarType{Index(curr->operands.size() - i - 1)}); |
There was a problem hiding this comment.
Why is this not
| note(&curr->operands[i], VarType{Index(curr->operands.size() - i - 1)}); | |
| note(&curr->operands[i], VarType{Index(i)}); |
?
There was a problem hiding this comment.
Because of the rules about the order in which variables are introduced in PrincipalTypes. The variables have to be introduced in order with zero at the end. Notably we're not actually using PrincipalTypes here, but this will eventually be used to construct PrincipalTypes.
There was a problem hiding this comment.
What does "in order" mean here? I'm not sure what order this is matching (the spec, or principal-types.h, or something else; I do see that principal-types.h mentions that params are stored in reverse order, but (1) that is an internal detail, and (2) you mentioned variables, not params specifically, so I'm not sure if it's related or not)
There was a problem hiding this comment.
From principal-type.h:
// ... The parameters are stored in reverse order
// for efficiency. Variable indices for each kind of variable must be introduced
// contiguously in order starting at zero from the end to beginning of the
// params (beginning to end of the reversed params).
There was a problem hiding this comment.
Oh, thanks, I missed that. So while the user provides the params in order, the indices must be in reverse order... perhaps the user should provide the params already reversed, as this is observable anyhow, and not just an internal detail? (But that would be separate from this PR)
There was a problem hiding this comment.
Yes, I think it's likely that we'll reverse the ChildTyper visitation order in the future.
There was a problem hiding this comment.
Oh, you were talking about when the params are provided to the PrincipalType constructor. Yes, that's certainly on the table as a potential simplification, too.
Refactor ChildTyper to report type constraints using a single callback
that takes a sequence of VarTypes from principal-type.h rather than
using several callbacks for the various kinds of constraints. The
previous approach using several callbacks did not scale well to the
various kinds of polymorphism introduced in new Wasm proposals, such
as polymorphism over sharedness in
ref.eqor polymorphism overexactness in
ref.get_desc.NFC despite better modeling these contraints because ChildTyper is so
far only used from TypeSSA, which only uses it to look for exact types,
and from IRBuilder, which only uses it to control the construction of
unreachable IR. Follow-on changes will use ChildTyper in new ways that
require this improved precision.