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

Trivial vec![] creation then iteration doesn't optimize out the allocation #43272

Closed
bluss opened this issue Jul 16, 2017 · 6 comments · Fixed by #43513
Closed

Trivial vec![] creation then iteration doesn't optimize out the allocation #43272

bluss opened this issue Jul 16, 2017 · 6 comments · Fixed by #43513
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@bluss
Copy link
Member

bluss commented Jul 16, 2017

Rustc used to be able to replace vec![1, 2, 3].iter().sum::<i32>() with a constant when optimizing, not at the moment. It simply looks like a Layout method that is not inlinable.

(This is a regression strictly speaking — stable & beta can do this optimization, but not nightly.)

Code to reproduce:

playground link

pub fn sum_me() -> i32 {
    vec![1, 2, 3].iter().sum::<i32>()
}
@bluss bluss added I-slow Issue: Problems and improvements with respect to performance of generated code. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Jul 16, 2017
@nodakai
Copy link
Contributor

nodakai commented Jul 17, 2017

Are there any ways to write automated tests to check for "regressions" like this? Assertions against LLVM bitcode?

@kennytm
Copy link
Member

kennytm commented Jul 17, 2017

@nodakai Yes codegen tests can be used for this, e.g. https://github.com/rust-lang/rust/blob/master/src/test/codegen/alloc-optimisation.rs. Assuming this is run against the bitcode after optimization, that is.

@alexcrichton alexcrichton added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Jul 23, 2017
@Mark-Simulacrum Mark-Simulacrum added C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 26, 2017
@Mark-Simulacrum
Copy link
Member

cc @arielb1 -- seems LLVM related

@arielb1
Copy link
Contributor

arielb1 commented Jul 27, 2017

Nope. Just Layout::repeat (

pub fn repeat(&self, n: usize) -> Option<(Self, usize)> {
) that is not marked as inline.

@arielb1
Copy link
Contributor

arielb1 commented Jul 27, 2017

cc @alexcrichton

@arielb1 arielb1 added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 27, 2017
@arielb1
Copy link
Contributor

arielb1 commented Jul 27, 2017

I didn't confirm that making Layout::repeat as inline fixes this, so please confirm that/add a test before closing.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 27, 2017
This fixes an optimization regression by allowing LLVM to see through more
functions.

Closes rust-lang#43272
frewsxcv added a commit to frewsxcv/rust that referenced this issue Jul 29, 2017
…Sushi

std: Mark `Layout::repeat` as `#[inline]`

This fixes an optimization regression by allowing LLVM to see through more
functions.

Closes rust-lang#43272
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 29, 2017
This fixes an optimization regression by allowing LLVM to see through more
functions.

Closes rust-lang#43272
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jul 30, 2017
…Sushi

std: Mark `Layout::repeat` as `#[inline]`

This fixes an optimization regression by allowing LLVM to see through more
functions.

Closes rust-lang#43272
alexcrichton added a commit to alexcrichton/rust that referenced this issue Aug 12, 2017
This fixes an optimization regression by allowing LLVM to see through more
functions.

Closes rust-lang#43272
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants