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

Rollup of 11 pull requests #73438

Closed
wants to merge 43 commits into from

Conversation

Dylan-DPC-zz
Copy link

Successful merges:

Failed merges:

r? @ghost

mibac138 and others added 30 commits May 20, 2020 20:42
When parsing `let x: i8 += 1` the compiler interprets `i8` as a trait
which makes it more complicated to do error recovery. More advanced
error recovery is not implemented in this commit.
We have been seeing some very inefficient code that went away when using
`-Cforce-frame-pointers=no`. For instance `core::ptr::drop_in_place` at
`-Oz` was compiled into a function which consisted entirely of saving
registers to the stack, then using the frame pointer to restore the same
registers (without any instructions between the prolog and epilog).

The RISC-V LLVM backend supports frame pointer elimination, so it makes
sense to allow this to happen when using Rust. It's not clear to me that
frame pointers have ever been required in the general case.

In rust-lang#61675 it was pointed out that this made reassembling
stack traces easier, which is true, but there is a code generation
option for forcing frame pointers, and I feel the default should not be
to require frame pointers, given it demonstrably makes code size worse
(around 10% in some embedded applications).

The kinds of targets mentioned in rust-lang#61675 are popular, but
should not dictate that code generation should be worse for all RISC-V
targets, especially as there is a way to use CFI information to
reconstruct the stack when the frame pointer is eliminated. It is also
a misconception that `fp` is always used for the frame pointer. `fp` is
an ABI name for `x8` (aka `s0`), and if no frame pointer is required,
`x8` may be used for other callee-saved values.

This commit does ensure that the standard library is built with unwind
tables, so that users do not need to rebuild the standard library in
order to get a backtrace that includes standard library calls (which is
the original reason for forcing frame pointers).
androm3da and others added 13 commits June 16, 2020 08:58
Fixed sentence by removing a word.
… r=hanna-kruppe,Mark-Simulacrum

[RISC-V] Do not force frame pointers

We have been seeing some very inefficient code that went away when using
`-Cforce-frame-pointers=no`. For instance `core::ptr::drop_in_place` at
`-Oz` was compiled into a function which consisted entirely of saving
registers to the stack, then using the frame pointer to restore the same
registers (without any instructions between the prolog and epilog).

The RISC-V LLVM backend supports frame pointer elimination, so it makes
sense to allow this to happen when using Rust. It's not clear to me that
frame pointers have ever been required in the general case.

In rust-lang#61675 it was pointed out that this made reassembling
stack traces easier, which is true, but there is a code generation
option for forcing frame pointers, and I feel the default should not be
to require frame pointers, given it demonstrably makes code size worse
(around 10% in some embedded applications).

The kinds of targets mentioned in rust-lang#61675 are popular, but
should not dictate that code generation should be worse for all RISC-V
targets, especially as there is a way to use CFI information to
reconstruct the stack when the frame pointer is eliminated. It is also
a misconception that `fp` is always used for the frame pointer. `fp` is
an ABI name for `x8` (aka `s0`), and if no frame pointer is required,
`x8` may be used for other callee-saved values.

---

I am partly posting this to get feedback from @fintelia who introduced the change to require frame pointers, and @hanna-kruppe who had issues with the original PR. I would understand if we wanted to remove this setting on only a subset of RISC-V targets, but my preference would be to remove this setting everywhere.

There are more details on the code size savings seen in Tock here: tock/tock#1660
Improve diagnostics for `let x += 1`

Fixes(?) rust-lang#66736

The code responsible for the `E0404` errors is [here](https://github.com/rust-lang/rust/blob/master/src/librustc_parse/parser/ty.rs#L399-L424) which I don't think can be easily modified to prevent emitting an error in one specific case. Because of this I couldn't get rid of `E0404` and instead added `E0067` along with a help message which will fix the problem.

r? @estebank
…akis

add raw_ref macros

In rust-lang#64490, various people were in favor of exposing `&raw` as a macro first before making the actual syntax stable. So this PR (unstably) introduces those macros.

I'll create the tracking issue if we're okay moving forward with this.
…ing-drop, r=matthewjasper,nikomatsakis

Report error when casting an C-like enum implementing Drop

Following approach described in rust-lang#35941
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
… r=petrochenkov

Disallow loading crates with non-ascii identifier name.

This turns off external crate loading with non-ascii identifier names.

cc rust-lang#55467.
…, r=kinnison

Clean up some weird command strings

r? @kinnison
Test that bounds checks are elided when slice len is checked up-front

Closes rust-lang#69101
Fix typo in librustc_ast docs

Fixed sentence by removing a word.
@Dylan-DPC-zz
Copy link
Author

@bors r+ p=11 rollup=never

@bors
Copy link
Contributor

bors commented Jun 17, 2020

📌 Commit edb339b has been approved by Dylan-DPC

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 17, 2020
@bors
Copy link
Contributor

bors commented Jun 17, 2020

⌛ Testing commit edb339b with merge 7d37c7fe58cde95c8aa28087a8f46b629c141f79...

@Dylan-DPC-zz
Copy link
Author

some hiccup happened this contains a merged pr (i retried the other but i guess it got merged :P ) so will re-roll after the beta nominations

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 17, 2020
@Dylan-DPC-zz Dylan-DPC-zz deleted the rollup-1tx5q3p branch June 17, 2020 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.