binaryen icon indicating copy to clipboard operation
binaryen copied to clipboard

Further enhancements to ExpressionRunner

Open dcodeIO opened this issue 5 years ago • 8 comments

I've meanwhile integrated the new ExpressionRunner (https://github.com/WebAssembly/binaryen/pull/2702) and it does what I've been hoping for, except for one edge case:

The expression runner allows to evaluate just the value of an expression, but if we have code like

(block
 (drop
  (local.get $0)
 )
 (i32.const 0)
)

and there is no known value for the local.get, it considers the expression a NONCONSTANT_FLOW and stops, instead of recognizing that the local.get is being dropped anyway, returning i32.const(0). Have looked at the code again, but supporting this seems like a non-trivial change. Any suggestions?

dcodeIO avatar Apr 22 '20 04:04 dcodeIO

Perhaps an idea: In case of a NONCONSTANT_FLOW as the value of a drop, utilize EffectAnalyzer to see what caused it, and if the only side effects were local or global gets, continue?

dcodeIO avatar Apr 22 '20 05:04 dcodeIO

Other passes (vacuum) would get rid of the dropped get, so running this after them would be best I think.

kripken avatar Apr 27 '20 20:04 kripken

Yeah, in typical optimization scenarios this isn't an issue, just when using ExpressionRunner as an API, via C, while still generating code. That's kinda the new use case of making it an API, where one can't easily run vacuum.

dcodeIO avatar Apr 27 '20 20:04 dcodeIO

Can't you run the standard set of optimization passes in C API? Or you don't want to run the whole set of passes but just to want vacuum? I think exposing each pass to C API is OK; but maybe we should do that for all passes for consistency? WDYT @kripken?

aheejin avatar Apr 28 '20 01:04 aheejin

Can't you run the standard set of optimization passes in C API?

One can, but only on entire functions. The new ExpressionRunner, on the other hand, can execute a single standalone expression, which is new and very useful on my end.

For a bit of background: What I previously did whenever I wanted to quickly check if an expression evaluates to a constant during codegen was to create a temporary function, move the expression into it as its body, run vacuum and precompute on that function, and obtain the modified body. This has several problems in that it always modifies the expression, which is unfortunate since AssemblyScript has to output strange half-modified code, and has quite some overhead and potential edge cases where locals don't exist and stuff. The ExpressionRunner API solves that, but turns out it has the problem that it cannot deal with drops properly if a local's value is unknown (bails early), even though that local value is irrelevant to the final value. https://github.com/WebAssembly/binaryen/pull/2787 solves part of that.

Regarding vacuum: Currently one cannot run vacuum on just a standalone expression. Even if one could, vacuum in its current form would modify the expression, which is again unfortunate. If the C-API would export something to vacuum a standalone expression without modifying, but cloning it, that would work as well, but from my perspective isn't a generally useful API and it would be better to simply support this case in ExpressionRunner well.

dcodeIO avatar Apr 28 '20 05:04 dcodeIO

For a bit of background: What I previously did whenever I wanted to quickly check if an expression evaluates to a constant during codegen was to create a temporary function, move the expression into it as its body, run vacuum and precompute on that function, and obtain the modified body.

This sounds almost good to me, hmm. You can make a copy, put that in a function, optimize that, and see what the result is. If the optimized body is a constant, you can then decide what to do with that (i.e., nothing is changed without your control).

I think in general adding APIs that make it easy to do arbitrary things like that in "userspace" would be a good idea (like an API for copying if we don't already have one).

kripken avatar Apr 28 '20 21:04 kripken

This sounds almost good to me, hmm.

like an API for copying if we don't already have one

Now that you mention it, yeah. Indeed, an API like BinaryenExpressionClone would have helped in my earlier approach, plus a way to tell the precompute pass that it can ignore side effects if only evaluating, but not going to replace.

dcodeIO avatar Apr 29 '20 02:04 dcodeIO

a way to tell the precompute pass that it can ignore side effects if only evaluating, but not going to replace.

That sounds good to me too. Basically "compute and replace" vs "just compute".

kripken avatar Apr 29 '20 20:04 kripken