askeksa-google
commented
1 year ago I have been contemplating something similar. A few thoughts:
- Re-using locals is a high-level mechanism that doesn't really belong in the
wasm_builder. But it makes sense to have it in theCodeGenerator. - In terms of API, I'd suggest having a separate API on top of
addLocal, i.e.getTempLocal/freeTempLocal. We would then continue to useaddLocalfor locals that don't have temporary scope or need to be distinguishable in the debugger (e.g. source-level local variables). - The main purpose of such an optimization would be to avoid situations where we generate more locals than Wasm supports. We should check the test cases we have that run into this situation to see if this strategy would actually help in those cases.
- Presumably, the end result after optimizing with Binaryen will be the same, since Binaryen performs SSA transformation. But we should make sure that the re-use of locals does not degrade the post-Binaryen code somehow.
Constantsonly emits locals for lazy constants, which is a rare case and is generated into small functions. So the optimization is probably not relevant for this particular case.
osa1
Functions like
CodeGenerator.makeListandConstants.instantiateConstantallocate locals that have very short lifetime. For example, here's a code thatinstantiateConstantgenerates:$88is generated byinstantiateConstantand it has no uses after this tiny block.(In this particular case the local can be eliminated, but in general I think there will be a few instructions between the
local.setandlocal.get)binaryen can probably optimize this kind of trivial cases to use a shared local, but it will change a lot of other things as well which may make debugging more difficult.
I think we can easily reuse these short-lived locals and simplify generated code by maintaining a
Map<ValueType, Set<Local>>state inDefinedFunctionfor locals that are known to be not used again, and add locals to the map with a new methodvoid freeLocal(Local).addLocalwould then use one of the available locals from the map when possible.Functions like
instantiateConstantandmakeListwould then free the locals they add when they are done. Relevant code inConstantswould look like:Seems like this could simplify generated code quite a bit with only one line added in a few places.
In cases where we think reusing a local will make debugging more difficult we can have a global parameter for disabling it, or add an optional argument to
addLocal:addLocal(ValueType type, {bool forceFresh = false}).