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 evaluate promoteds for each monomorphization if it does not depend on generic parameters #67176

Open
oli-obk opened this issue Dec 9, 2019 · 6 comments
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-enhancement Category: An issue proposing an enhancement or a PR with one. I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Dec 9, 2019

In the following code snippet

fn foo<T>() -> &'static i32 {
    &(5 + 6)
}

we evaluate the &(5 + 6) for every monomorphization of foo, even though we could just do this once. We should find a way to get rid of this duplicate evaluation. Maybe we can already do this at the time promoteds are created (const qualif + promote.rs) by checking promoted_mir.needs_substs() and if it's false, make the ty::Const that refers to it not have any substs itself. const_eval will then automatically memoize all the evaluations of this promoted.

Alternatively const prop could attempt to evaluate the promoted and if it fails, assume it has generics and leave it to monomorphization. This would require more CPU time than the const qualif version, since we'd do a lot of prospetive evaluations that may not succeed.

@oli-obk
Copy link
Contributor Author

oli-obk commented Dec 9, 2019

cc @rust-lang/wg-codegen @rust-lang/wg-compiler-performance this could have a not insignificant impact on compile times as a lot of panic handling code is promoted (all the datastructures for line and file information). If this could stop being evaluated for each monomorphization of generic functions that panic, this may save us quite a bit of time. I'm thinking about .unwrap() and friends here

@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) labels Dec 9, 2019
@michaelwoerister
Copy link
Member

Interesting! Do I remember correctly that @davidtwco is working on something related for their master's thesis?

@nox
Copy link
Contributor

nox commented Dec 9, 2019

Pretty sure there is an issue about this somewhere already.

@est31
Copy link
Member

est31 commented Dec 9, 2019

A similar but not the same bug: #46477

@davidtwco
Copy link
Member

I believe this would be handled by a fix for #46477, which @michaelwoerister is correct that I'm working on as part of my master's thesis. There's a working group page and Zulip stream.

@michaelwoerister
Copy link
Member

That's really exciting, @davidtwco! I also appreciate that you set up a proper working group for this effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-enhancement Category: An issue proposing an enhancement or a PR with one. I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants