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

precompute-propagate pass #1179

Merged
merged 21 commits into from Sep 12, 2017

Conversation

Projects
None yet
3 participants
@kripken
Member

kripken commented Sep 12, 2017

Implements #1172: this adds a variant of precompute, "precompute-propagate", which also does constant propagation. Precompute by itself just runs the interpreter on each expression and sees if it is in fact a constant; precompute-propagate also looks at the graph of connections between get and set locals, and propagates those constant values.

This helps with cases as noticed in #1168 - while in most cases LLVM will do this already, it's important when inlining, e.g. inlining of the clamping math functions. This new pass is run when inlining, and otherwise only in -O3/-Oz, as it does increase compilation time noticeably if run on everything (and for almost no benefit if LLVM has run).

Most of the code here is just refactoring out from the ssa pass the get/set graph computation, so it can now be used by both the ssa pass and precompute-propagate.

// TODO: the algorithm here is pretty simple, but also pretty slow,
// we should optimize it. e.g. we rely on set_interaction
// here, and worse we only use it to compute the size...
struct LocalGraph : public PostWalker<LocalGraph> {

This comment has been minimized.

@dschuff

dschuff Sep 12, 2017

Member

Can we put this in a cpp file rather than a header file? My build times thank you for it :)

This comment has been minimized.

@kripken

kripken Sep 12, 2017

Member

Sure, done. Even if I kind of think it's more elegant the other way... ;)

I wasn't sure about the file name. We seem to use FileName.cpp but file-name.h for other things...

@jgravelle-google

Overall looks fine to me

@kripken kripken merged commit c672940 into master Sep 12, 2017

4 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@kripken kripken deleted the precompute-locals branch Sep 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment