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

the type_use code should consider ABI-specific classifications #3547

Closed
nikomatsakis opened this issue Sep 20, 2012 · 4 comments · Fixed by #9538
Closed

the type_use code should consider ABI-specific classifications #3547

nikomatsakis opened this issue Sep 20, 2012 · 4 comments · Fixed by #9538
Labels
A-codegen Area: Code generation

Comments

@nikomatsakis
Copy link
Contributor

I've been digging into the type_use code a bit more. I found at least one broken cast: it considered u64 and double to be equivalent because they have the same size/repr, but this is not valid because scalars and floats go in different registers, at least when passed as parameters.

I've patched over this problem for now using but I am not confident that other cases are correct. ABI rules can be quite complex! We should certainly be considering the ABI classifications when we decide whether a monomorphic instance can be reused.

@nikomatsakis
Copy link
Contributor Author

I left a FIXME in the code.

@catamorphism
Copy link
Contributor

Bumping to 0.6

@catamorphism
Copy link
Contributor

Nominating for milestone 5, production-ready (unless it's a milestone 1 thing?)

@graydon
Copy link
Contributor

graydon commented Jun 20, 2013

accepted for production-ready milestone

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
bors added a commit to rust-lang-ci/rust that referenced this issue May 2, 2020
add size parameter for `vec_box`  lint

changelog: add size threshold for the `vec_box` lint, currently 4096 bytes (one page) (subject to change). Closes rust-lang#3547.

diff is a little bit confusing due to some refactoring (moving free functions to lint struct functions), relevant portion is [this](https://github.com/rust-lang/rust-clippy/compare/master...Areredify:vec_box_threshold?expand=1#diff-1096120ca9143af89dcc9175ea92b54aR294-R298). In hindsight should've been different commits, but oh well.
RalfJung pushed a commit to RalfJung/rust that referenced this issue May 4, 2024
CI: no need to surround if: condition in expansion braces

That seems to be implicit for `if:` (but interestingly, redundant braces are tolerated).
RalfJung pushed a commit to RalfJung/rust that referenced this issue May 4, 2024
CI: no need to surround if: condition in expansion braces

That seems to be implicit for `if:` (but interestingly, redundant braces are tolerated).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants