Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't move loads to after conflicting writes #19

Open
simonlindholm opened this issue Oct 2, 2018 · 2 comments
Open

Don't move loads to after conflicting writes #19

simonlindholm opened this issue Oct 2, 2018 · 2 comments

Comments

@simonlindholm
Copy link
Collaborator

simonlindholm commented Oct 2, 2018

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
Copy link
Collaborator Author

This is mostly done now, with 2576388 and e13d945. 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
Copy link
Collaborator Author

Another case of this: 2152acc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant