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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Jul 30, 2017
Merged

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

merged 1 commit into from
Jul 30, 2017

Conversation

alexcrichton
Copy link
Member

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

Closes #43272

@rust-highfive
Copy link
Collaborator

r? @BurntSushi

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton alexcrichton added beta-nominated Nominated for backporting to the compiler in the beta channel. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 27, 2017
BurntSushi
BurntSushi reviewed Jul 27, 2017
View reviewed changes
src/test/codegen/vec-optimizes-away.rs Outdated
#![crate_type="lib"]

#[no_mangle]
#[no_mangle]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intentional to have #[no_mangle] repeated here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, nope!

Mark-Simulacrum
Mark-Simulacrum reviewed Jul 27, 2017
View reviewed changes
src/test/codegen/vec-optimizes-away.rs
// option. This file may not be copied, modified, or distributed
// except according to those terms.
//
// no-system-llvm
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this not work on system LLVMs? Is it the typical behavior where LLVM doesn't recognize our allocation functions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah unfortunately only our LLVM sees our allocation functions.

@alexcrichton alexcrichton force-pushed the inline-repeat branch 2 times, most recently from 76fbe16 to 289142d Compare July 27, 2017 22:15
@alexcrichton alexcrichton added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 27, 2017
@BurntSushi
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 27, 2017

📌 Commit 289142d has been approved by BurntSushi

@Mark-Simulacrum
Copy link
Member

@bors rollup

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jul 29, 2017
@frewsxcv
Rollup merge of rust-lang#43513 - alexcrichton:inline-repeat, r=Burnt…
279d25b 
…Sushi

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

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

Closes rust-lang#43272
@frewsxcv frewsxcv mentioned this pull request Jul 29, 2017
Closed
@frewsxcv
Copy link
Member

@bors r-

#43538

[00:51:43] failures:
[00:51:43] 
[00:51:43] ---- [codegen] codegen/vec-optimizes-away.rs stdout ----
[00:51:43] 	
[00:51:43] error: verification with 'FileCheck' failed
[00:51:43] status: exit code: 1
[00:51:43] command: /checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/bin/FileCheck -input-file=/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/vec-optimizes-away.ll /checkout/src/test/codegen/vec-optimizes-away.rs
[00:51:43] stdout:
[00:51:43] ------------------------------------------
[00:51:43] 
[00:51:43] ------------------------------------------
[00:51:43] stderr:
[00:51:43] ------------------------------------------
[00:51:43] /checkout/src/test/codegen/vec-optimizes-away.rs:18:17: error: expected string not found in input
[00:51:43]  // CHECK-NEXT: bb{{.*}}:
[00:51:43]                 ^
[00:51:43] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/vec-optimizes-away.ll:7:19: note: scanning from here
[00:51:43] define i32 @sum_me() unnamed_addr #0 personality i32 (...)* @rust_eh_personality {
[00:51:43]                   ^
[00:51:43] /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/vec-optimizes-away.ll:7:52: note: possible intended match here
[00:51:43] define i32 @sum_me() unnamed_addr #0 personality i32 (...)* @rust_eh_personality {
[00:51:43]                                                    ^
[00:51:43] 
[00:51:43] ------------------------------------------
[00:51:43] 
[00:51:43] thread '[codegen] codegen/vec-optimizes-away.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2499:8
[00:51:43] note: Run with `RUST_BACKTRACE=1` for a backtrace.

@alexcrichton
std: Mark Layout::repeat as #[inline]
54b6b23 
This fixes an optimization regression by allowing LLVM to see through more
functions.

Closes rust-lang#43272
@alexcrichton
Copy link
Member Author

@bors: r=BurntSushi

1 similar comment
@alexcrichton
Copy link
Member Author

@bors: r=BurntSushi

@bors
Copy link
Contributor

bors commented Jul 29, 2017

📌 Commit 54b6b23 has been approved by BurntSushi

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 30, 2017
@Mark-Simulacrum
Rollup merge of rust-lang#43513 - alexcrichton:inline-repeat, r=Burnt…
a3eadd7 
…Sushi

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

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

Closes rust-lang#43272
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Jul 30, 2017
Merged
bors added a commit that referenced this pull request Jul 30, 2017
@bors
Auto merge of #43551 - Mark-Simulacrum:rollup, r=Mark-Simulacrum
5c71e4e 
Rollup of 8 pull requests

- Successful merges: #43409, #43501, #43509, #43512, #43513, #43536, #43544, #43549
- Failed merges:
@bors bors merged commit 54b6b23 into rust-lang:master Jul 30, 2017
@alexcrichton alexcrichton deleted the inline-repeat branch July 30, 2017 04:10
@oyvindln oyvindln mentioned this pull request Jul 31, 2017
Closed
@alexcrichton alexcrichton added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Aug 1, 2017
@alexcrichton alexcrichton mentioned this pull request Aug 12, 2017
Merged
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Aug 12, 2017
bors added a commit that referenced this pull request Aug 13, 2017
@bors
Auto merge of #43831 - alexcrichton:beta-next, r=alexcrichton
54279df 
Backporting accepted PRs to beta

Backport of:

* #43735
* #43546
* #43513
* #43373
* #43315
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

@BurntSushi BurntSushi BurntSushi left review comments

Assignees

@BurntSushi BurntSushi

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Trivial vec![] creation then iteration doesn't optimize out the allocation
6 participants
@alexcrichton @rust-highfive @BurntSushi @bors @Mark-Simulacrum @frewsxcv