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

Deduplicate closures #63660

Closed
oli-obk opened this issue Aug 17, 2019 · 3 comments
Closed

Deduplicate closures #63660

oli-obk opened this issue Aug 17, 2019 · 3 comments
Labels
A-closures Area: Closures (`|…| { … }`) A-codegen Area: Code generation A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-enhancement Category: An issue proposing an enhancement or a PR with one. I-heavy Issue: Problems and improvements with respect to binary size of generated code. 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 Aug 17, 2019

#62429 manually deduplicated monomorphizations of closures. We should automate this during building instead of requiring users to do it manually. Ideally we'll revert the parts of #62429 that made things less readable as the last step of closing this issue.

I think the endgame would be that we'd deduplicate closures (in release mode only, so backtraces's file:line locations still make sense in debug mode), as long as their body is the same.

Not sure if that is possible, but random idea:

If we only care about monomorphizations in llvm, we could create a function that takes a closure's mir::Body, cleans it of all "unnecessary" info like Spans and then feeds that through a query (which thus will only be called once with the same arguments), producing the appropriate codegen backend's function handle.

If we want to reduce type lengths, too, we'd need to change the way closure substs are built, by computing the minimal required set ahead of time.

@jonas-schievink jonas-schievink added A-closures Area: Closures (`|…| { … }`) A-codegen Area: Code generation A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-heavy Issue: Problems and improvements with respect to binary size of generated code. labels Aug 17, 2019
@cuviper
Copy link
Member

cuviper commented Aug 17, 2019

Deduplicating is nice, but I believe LLVM is already pretty good about this, so I don't expect too much benefit there. The exploding type lengths were the real practical thing that #62429 addressed. I probably should have emphasized that better in the description.

@Centril
Copy link
Contributor

Centril commented Aug 18, 2019

@eddyb in #62429 (comment):

Isn't this #46477? We should revive those efforts and implement it as "polymorphic" codegen (still-generic over some parameters which aren't actually used), in a similar way to how "polymorphic" CTFE works with miri today.

@jonas-schievink
Copy link
Contributor

Seems like a clear duplicate, so closing in favor of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: Closures (`|…| { … }`) A-codegen Area: Code generation A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-enhancement Category: An issue proposing an enhancement or a PR with one. I-heavy Issue: Problems and improvements with respect to binary size of generated code. 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

4 participants