Translate Wasm block params into Variables instead of CLIF block params#13711
Translate Wasm block params into Variables instead of CLIF block params#13711fitzgen wants to merge 1 commit into
Variables instead of CLIF block params#13711Conversation
…rams This allows SSA construction to determine whether they need to actually become block params or not, and cuts down on the number of unnecessary block parameters we pessimistically introduce during Wasm-to-CLIF translation.
cfallin
left a comment
cfallin
left a comment
There was a problem hiding this comment.
Thanks! This technically looks fine to me, and I agree it's nice to use our existing algorithm to avoid creating blockparams where they are actually trivial/unneeded.
I'm curious, however, since this does make things a little more complex (some blocks take vars, some don't), and leans more on an implicit SSA construction algorithm, which does have some cost, with the hypothesis that we'll get better code / faster compiles with fewer blockparams in the IR: have you measured the improvement? Do we see faster compile or faster runtimes out of this?
| /// For a Wasm control-flow target (i.e. a block with associated parameter | ||
| /// `Variable`s) we `use_var` each of those variables. For a block with real | ||
| /// CLIF block parameters (the function's exit block) we read those parameters | ||
| /// directly. |
There was a problem hiding this comment.
This distinction seems to me like it has potential to be error-prone or at least lead to subtle cases (e.g. there's the implicit invariant that the exit block is not also a Wasm target; we're creating a new bifurcation of kinds-of-blocks). Is there a reason we can't have the exit-block take its return value(s) as variables, too?
2 participants
This allows SSA construction to determine whether they need to actually become block params or not, and cuts down on the number of unnecessary block parameters we pessimistically introduce during Wasm-to-CLIF translation.