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

Always inline `mem::{size_of,align_of}` in debug builds #65016

Merged
merged 1 commit into from Oct 19, 2019

Conversation

@lzutao
Copy link
Contributor

lzutao commented Oct 2, 2019

Those two are const fn and do not have any arguments. Inlining
helps reducing generated code size in debug builds.

See also #64996.

Those two are const fn and do not have any arguments. Inlining
helps reducing generated code size in debug builds.
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Oct 2, 2019

r? @TimNN

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

@lzutao

This comment has been minimized.

Copy link
Contributor Author

lzutao commented Oct 2, 2019

r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned TimNN Oct 2, 2019
@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Oct 3, 2019

@oli-obk Some of #64996 (comment) seems also relevant here.
But also I feel like we should try to always run the MIR inliner for trivial abstractions like this.

Another nice thing would making the functions in mem the intrinsics themselves, using the improved approach to intrinsics, discussed in a couple places before.

@JohnCSimon

This comment has been minimized.

Copy link
Member

JohnCSimon commented Oct 12, 2019

Ping from triage -
This seems to be waiting on a review.
@RalfJung @nagisa @eddyb @oli-obk
Thanks.

@RalfJung

This comment has been minimized.

Copy link
Member

RalfJung commented Oct 12, 2019

Another nice thing would making the functions in mem the intrinsics themselves, using the improved approach to intrinsics, discussed in a couple places before.

Do you have any links for that? I do not know what you are talking about.

With the current bugs in const-qualif, I am quite happy that most intrinsics are not themselves exposed.^^

@oli-obk

This comment has been minimized.

Copy link
Contributor

oli-obk commented Oct 18, 2019

Let's do this PR for now and once we get our const qualif business in order we can improve the intrinsic wrappers

@oli-obk

This comment has been minimized.

Copy link
Contributor

oli-obk commented Oct 18, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 18, 2019

📌 Commit a87b44d has been approved by oli-obk

@oli-obk

This comment has been minimized.

Copy link
Contributor

oli-obk commented Oct 18, 2019

@bors rollup

tmandry added a commit to tmandry/rust that referenced this pull request Oct 18, 2019
Always inline `mem::{size_of,align_of}` in debug builds

Those two are const fn and do not have any arguments. Inlining
helps reducing generated code size in debug builds.

See also rust-lang#64996.
bors added a commit that referenced this pull request Oct 18, 2019
Rollup of 17 pull requests

Successful merges:

 - #65016 (Always inline `mem::{size_of,align_of}` in debug builds)
 - #65197 (Prepare `MutVisitor`s to handle interned projections)
 - #65201 (Disable Go and OCaml bindings when building LLVM)
 - #65364 (Collect occurrences of empty blocks for mismatched braces diagnostic)
 - #65417 (Add more coherence tests)
 - #65434 (Add long error explanation for E0577)
 - #65455 (Avoid unnecessary `TokenTree` to `TokenStream` conversions)
 - #65472 (Use a sharded dep node to dep node index map)
 - #65480 (Speed up `LexicalResolve::expansion()`)
 - #65496 (properly document panics in div_euclid and rem_euclid)
 - #65508 (add option to ping llvm ice-breakers to triagebot)
 - #65511 (save-analysis: Nest tables when processing impl block definitions)
 - #65513 (reorder fmt docs for more clarity)
 - #65532 (doc: make BitSet intro more short)
 - #65540 (show up some extra info when t!() fails)
 - #65549 (Fix left/right shift typo in wrapping rotate docs)
 - #65552 (Clarify diagnostics when using `~` as a unary op)

Failed merges:

 - #65471 (Add long error explanation for E0578)

r? @ghost
tmandry added a commit to tmandry/rust that referenced this pull request Oct 18, 2019
Always inline `mem::{size_of,align_of}` in debug builds

Those two are const fn and do not have any arguments. Inlining
helps reducing generated code size in debug builds.

See also rust-lang#64996.
bors added a commit that referenced this pull request Oct 18, 2019
Rollup of 19 pull requests

Successful merges:

 - #65016 (Always inline `mem::{size_of,align_of}` in debug builds)
 - #65197 (Prepare `MutVisitor`s to handle interned projections)
 - #65201 (Disable Go and OCaml bindings when building LLVM)
 - #65334 (Add long error explanation for E0575)
 - #65364 (Collect occurrences of empty blocks for mismatched braces diagnostic)
 - #65455 (Avoid unnecessary `TokenTree` to `TokenStream` conversions)
 - #65472 (Use a sharded dep node to dep node index map)
 - #65480 (Speed up `LexicalResolve::expansion()`)
 - #65493 (Add long error explanation for E0584)
 - #65496 (properly document panics in div_euclid and rem_euclid)
 - #65498 (Plugins deprecation: don’t suggest simply removing the attribute)
 - #65508 (add option to ping llvm ice-breakers to triagebot)
 - #65511 (save-analysis: Nest tables when processing impl block definitions)
 - #65513 (reorder fmt docs for more clarity)
 - #65532 (doc: make BitSet intro more short)
 - #65535 (rustc: arena-allocate the slice in `ty::GenericsPredicate`, not the whole struct.)
 - #65540 (show up some extra info when t!() fails)
 - #65549 (Fix left/right shift typo in wrapping rotate docs)
 - #65552 (Clarify diagnostics when using `~` as a unary op)

Failed merges:

 - #65390 (Add long error explanation for E0576)
 - #65434 (Add long error explanation for E0577)
 - #65471 (Add long error explanation for E0578)

r? @ghost
bors added a commit that referenced this pull request Oct 19, 2019
Rollup of 19 pull requests

Successful merges:

 - #65016 (Always inline `mem::{size_of,align_of}` in debug builds)
 - #65197 (Prepare `MutVisitor`s to handle interned projections)
 - #65201 (Disable Go and OCaml bindings when building LLVM)
 - #65334 (Add long error explanation for E0575)
 - #65364 (Collect occurrences of empty blocks for mismatched braces diagnostic)
 - #65455 (Avoid unnecessary `TokenTree` to `TokenStream` conversions)
 - #65472 (Use a sharded dep node to dep node index map)
 - #65480 (Speed up `LexicalResolve::expansion()`)
 - #65493 (Add long error explanation for E0584)
 - #65496 (properly document panics in div_euclid and rem_euclid)
 - #65498 (Plugins deprecation: don’t suggest simply removing the attribute)
 - #65508 (add option to ping llvm ice-breakers to triagebot)
 - #65511 (save-analysis: Nest tables when processing impl block definitions)
 - #65513 (reorder fmt docs for more clarity)
 - #65532 (doc: make BitSet intro more short)
 - #65535 (rustc: arena-allocate the slice in `ty::GenericsPredicate`, not the whole struct.)
 - #65540 (show up some extra info when t!() fails)
 - #65549 (Fix left/right shift typo in wrapping rotate docs)
 - #65552 (Clarify diagnostics when using `~` as a unary op)

Failed merges:

 - #65390 (Add long error explanation for E0576)
 - #65434 (Add long error explanation for E0577)
 - #65471 (Add long error explanation for E0578)

r? @ghost
@bors bors merged commit a87b44d into rust-lang:master Oct 19, 2019
4 checks passed
4 checks passed
pr Build #20191002.72 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
@lzutao lzutao deleted the lzutao:inline-mem-constfn branch Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.