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

rewrite type_use #8651

Closed
graydon opened this issue Aug 20, 2013 · 3 comments · Fixed by #9538
Closed

rewrite type_use #8651

graydon opened this issue Aug 20, 2013 · 3 comments · Fixed by #9538
Labels
A-codegen Area: Code generation A-typesystem Area: The type system E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. I-slow Issue: Problems and improvements with respect to performance of generated code.

Comments

@graydon
Copy link
Contributor

graydon commented Aug 20, 2013

As evidenced in http://people.mozilla.org/~graydon/symbols-by-name.txt we are making a whole lot of copies of functions. type_use is supposed to recycle functions that are "effectively" the same due to their type-dependence, but it doesn't do a very good job of it (see other linked bugs in #6819). Probably it could do with being rewritten entirely.

@thestinger
Copy link
Contributor

I suggested on IRC that it might be worth abandoning type_use for now and using LLVM's mergefunc pass. We'll need to get some numbers (code size, runtime performance, compile-time performance) while trying mergefunc in various places (not at all, once at the start, once at the end, or both) with/without monomorphize.

@thestinger
Copy link
Contributor

The mergefunc pass currently doesn't work for Rust because it asserts on casts with undefined behaviour. We really need to fix the LLVM lint errors.

@nikomatsakis
Copy link
Contributor

I just want to note down this idea for posterity: I think if we DO keep type_use, rather than relying on mergefunc, it should work like so:

(1) Generate the first instance of the function normally.
(2) Traverse the LLVM IR that we generate and extract the constraints on the types from there.
(3) Record those constraints and use for future lookups.

This strikes me as much more robust than the existing technique, though there are some challenges. For example, the generic types are not present in the LLVM IR, so we will have to approximate in some way, perhaps if T=int, for example, examining all uses of ints. This might defeat the idea, I'm not sure.

bors added a commit that referenced this issue Sep 27, 2013
This is broken, and results in poor performance due to the undefined
behaviour in the LLVM IR. LLVM's `mergefunc` is a *much* better way of
doing this since it merges based on the equality of the bytecode.

For example, consider `std::repr`. It generates different code per
type, but is not included in the type bounds of generics.

The `mergefunc` pass works for most of our code but currently hits an
assert on libstd. It is receiving attention upstream so it will be
ready soon, but I don't think removing this broken code should wait any
longer. I've opened #9536 about enabling it by default.

Closes #8651
Closes #3547
Closes #2537
Closes #6971
Closes #9222
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-typesystem Area: The type system E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants