m2c icon indicating copy to clipboard operation
m2c copied to clipboard

Don't move loads to after conflicting writes

Open simonlindholm opened this issue 5 years ago • 2 comments

E.g. return x++; currently emits something like

x = x + 1;
return x;

which is wrong. Maybe we could iterate over all registers contents recursively on writes and for loads overlapping the write mark corresponding EvalOnceExpr's as "emit if any new use appears".

I imagine post-increments are the most common case of this (?), so might be worth special-casing.

Edit: function calls are roughly the same as reads in this regard, except they also shouldn't be moved past other function calls, writes, or branches.

simonlindholm avatar Oct 02 '18 15:10 simonlindholm

This is mostly done now, with 2576388464d2efa960074d3f2cd0170de1bb69c3 and e13d9451e9841d94f1f3a9ab244147275b16481a. It turned out to be somewhat trickier than just marking EvalOnceExpr's as "emit if new uses appear", since a) that could lead to more than one variable getting emitted if it's used somewhere in the middle of expression, b) not every expression is an EvalOnceExpr.

I fixed b) by actually making most registers contain EvalOnceExpr's and introducing "trivial" EvalOnceExpr's for the cases where duplicating the expression is fine except for reorderings. (Not sure EvalOnceExpr is a good name any longer...)

I fixed a) by introducing ForceVarOnUse expressions that wrap EvalOnceExpr's and force vars for them when used, and only using those at top level. The difference is probably small in practice compared to adding a flag to EvalOnceExpr, but it feels cleaner.

Function call reorderings still remain. I think what we want to do there is to at the end of each block, and at every write/function call, do the equivalent of prevent_later_uses but for function calls instead of specific subexpressions.

Also, we may want to update uses_expr not to descend through EvalOnceExpr's with need_decl() true. It's sorta iffy to call that function before translation is done, but it may avoid unnecessary var emissions...

simonlindholm avatar Feb 07 '19 13:02 simonlindholm

Another case of this: 2152accdca91d33daa4ee242e3d50881236a3ba1

simonlindholm avatar Mar 11 '19 11:03 simonlindholm