Skip to content

Reduce size of hir::Expr by boxing more of hir::InlineAsm#66515

Merged
bors merged 1 commit into
rust-lang:masterfrom
Centril:cheaper-inline-asm
Nov 21, 2019
Merged

Reduce size of hir::Expr by boxing more of hir::InlineAsm#66515
bors merged 1 commit into
rust-lang:masterfrom
Centril:cheaper-inline-asm

Conversation

@Centril

@Centril Centril commented Nov 18, 2019

Copy link
Copy Markdown
Contributor

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 18, 2019
@Centril

Centril commented Nov 18, 2019

Copy link
Copy Markdown
Contributor Author

@bors try @rust-timer queue

@rust-timer

rust-timer commented Nov 18, 2019

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion

@bors

bors commented Nov 18, 2019

Copy link
Copy Markdown
Collaborator

⌛ Trying commit 9b7bed080fc3381be02de27bb7167be3a7a74221 with merge 03d05b28688feb26964e046893ba71f5bf7fd69b...

@bors

bors commented Nov 18, 2019

Copy link
Copy Markdown
Collaborator

☀️ Try build successful - checks-azure
Build commit: 03d05b28688feb26964e046893ba71f5bf7fd69b (03d05b28688feb26964e046893ba71f5bf7fd69b)

@rust-timer

rust-timer commented Nov 18, 2019

Copy link
Copy Markdown
Collaborator

Queued 03d05b28688feb26964e046893ba71f5bf7fd69b with parent a0d40f8, future comparison URL.

@rust-timer

rust-timer commented Nov 18, 2019

Copy link
Copy Markdown
Collaborator

Finished benchmarking try commit 03d05b28688feb26964e046893ba71f5bf7fd69b, comparison URL.

@oli-obk

oli-obk commented Nov 19, 2019

Copy link
Copy Markdown
Contributor

Unsure if this has had an effect on max-rss, but if it has, it's an improvement (loads of green in https://perf.rust-lang.org/compare.html?start=a0d40f8bdfcc3c28355467973f97fd4c45ac5876&end=03d05b28688feb26964e046893ba71f5bf7fd69b&stat=max-rss)

@Centril

Centril commented Nov 19, 2019

Copy link
Copy Markdown
Contributor Author

Yeah that's my thinking also; cc @nnethercote

oli-obk
oli-obk reviewed Nov 19, 2019
View reviewed changes
Comment thread src/librustc/hir/mod.rs Outdated
oli-obk
oli-obk reviewed Nov 19, 2019
View reviewed changes
Comment thread src/librustc/hir/mod.rs Outdated
nnethercote
nnethercote reviewed Nov 19, 2019
View reviewed changes
Comment thread src/librustc/hir/mod.rs Outdated

@nnethercote nnethercote Nov 19, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there any need to have InlineAsmInner? Could we just have asm, asm_str_style, outputs, and dialect fields directly in InlineAsm?

@Centril Centril Nov 19, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried but wasn't able to do that; Expr is not Clone and InlineAsmInner needs to be Clone.

@nnethercote

nnethercote commented Nov 19, 2019

Copy link
Copy Markdown
Contributor

max-rss is very noisy so it's hard to know if this effect is real. The change certainly can't hurt, though, especially given that the InlineAsm variant is so rarely used.

@Centril

Centril commented Nov 21, 2019

Copy link
Copy Markdown
Contributor Author

@bors r=oli-obk

@bors

bors commented Nov 21, 2019

Copy link
Copy Markdown
Collaborator

📌 Commit 44cebe5 has been approved by oli-obk

@bors

bors commented Nov 21, 2019

Copy link
Copy Markdown
Collaborator

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 21, 2019
Centril added a commit to Centril/rust that referenced this pull request Nov 21, 2019
@Centril
Rollup merge of rust-lang#66515 - Centril:cheaper-inline-asm, r=oli-obk
90e7976 
Reduce size of `hir::Expr` by boxing more of `hir::InlineAsm`

r? @oli-obk
@Centril Centril mentioned this pull request Nov 21, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Nov 21, 2019
@Centril
Rollup merge of rust-lang#66515 - Centril:cheaper-inline-asm, r=oli-obk
411b298 
Reduce size of `hir::Expr` by boxing more of `hir::InlineAsm`

r? @oli-obk
@Centril Centril mentioned this pull request Nov 21, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Nov 21, 2019
@Centril
Rollup merge of rust-lang#66515 - Centril:cheaper-inline-asm, r=oli-obk
d7bb37d 
Reduce size of `hir::Expr` by boxing more of `hir::InlineAsm`

r? @oli-obk
@Centril Centril mentioned this pull request Nov 21, 2019
Merged
bors added a commit that referenced this pull request Nov 21, 2019
@bors
Auto merge of #66607 - Centril:rollup-yb7cl73, r=Centril
35ef33a 
Rollup of 5 pull requests

Successful merges:

 - #65355 (Stabilize `!` in Rust 1.41.0)
 - #65730 (Suggest to add lifetime constraint at explicit ouput of functions)
 - #66468 (Cleanup Miri SIMD intrinsics)
 - #66515 (Reduce size of `hir::Expr` by boxing more of `hir::InlineAsm`)
 - #66602 (Revert "Update Source Code Pro and include italics")

Failed merges:

r? @ghost
@bors bors merged commit 44cebe5 into rust-lang:master Nov 21, 2019
@Centril Centril deleted the cheaper-inline-asm branch November 21, 2019 17:53
flip1995 added a commit to Manishearth/rust-clippy that referenced this pull request Nov 22, 2019
@flip1995
Rustup to rust-lang/rust#66515
1b2f43d 
Fixes rust-lang#4835
flip1995 added a commit to Manishearth/rust-clippy that referenced this pull request Nov 22, 2019
@flip1995
Rustup to rust-lang/rust#66515
e4636f3 
Fixes rust-lang#4835
@Manishearth Manishearth mentioned this pull request Nov 22, 2019
Merged
bors added a commit to rust-lang/rust-clippy that referenced this pull request Nov 23, 2019
@bors
Auto merge of #4825 - Manishearth:rustup, r=Manishearth
19769b5 
Rustup to rustc 1.41.0-nightly (35ef33a 2019-11-21)

I don't have the right fix for the fmtstr tests, and I'm also hitting problems caused by messense/rustc-test#3

List of rustups:
- rust-lang/rust#66271 (syntax: Keep string literals in ABIs and `asm!` more precisely)
- rust-lang/rust#65355 (Stabilize `!` in Rust 1.41.0)
- rust-lang/rust#66515 (Reduce size of `hir::Expr` by boxing more of `hir::InlineAsm`)
- rust-lang/rust#66389 (Specific labels when referring to "expected" and "found" types)
- rust-lang/rust#66074 ([mir-opt] Turn on the `ConstProp` pass by default)

changelog: none
bors added a commit to rust-lang/rust-clippy that referenced this pull request Nov 23, 2019
@bors
Auto merge of #4825 - Manishearth:rustup, r=Manishearth
3870050 
Rustup to rustc 1.41.0-nightly (35ef33a 2019-11-21)

I don't have the right fix for the fmtstr tests, and I'm also hitting problems caused by messense/rustc-test#3

List of rustups:
- rust-lang/rust#66271 (syntax: Keep string literals in ABIs and `asm!` more precisely)
- rust-lang/rust#65355 (Stabilize `!` in Rust 1.41.0)
- rust-lang/rust#66515 (Reduce size of `hir::Expr` by boxing more of `hir::InlineAsm`)
- rust-lang/rust#66389 (Specific labels when referring to "expected" and "found" types)
- rust-lang/rust#66074 ([mir-opt] Turn on the `ConstProp` pass by default)

changelog: none
bors added a commit to rust-lang/rust-clippy that referenced this pull request Nov 23, 2019
@bors
Auto merge of #4825 - Manishearth:rustup, r=flip1995
60e8413 
Rustup to rustc 1.41.0-nightly (35ef33a 2019-11-21)

I don't have the right fix for the fmtstr tests, and I'm also hitting problems caused by messense/rustc-test#3

List of rustups:
- rust-lang/rust#66271 (syntax: Keep string literals in ABIs and `asm!` more precisely)
- rust-lang/rust#65355 (Stabilize `!` in Rust 1.41.0)
- rust-lang/rust#66515 (Reduce size of `hir::Expr` by boxing more of `hir::InlineAsm`)
- rust-lang/rust#66389 (Specific labels when referring to "expected" and "found" types)
- rust-lang/rust#66074 ([mir-opt] Turn on the `ConstProp` pass by default)

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obk oli-obk oli-obk left review comments
@nnethercote nnethercote nnethercote left review comments

Assignees

@oli-obk oli-obk

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Centril @rust-timer @bors @oli-obk @nnethercote @rust-highfive