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

Disable the machine outliner by default #86020

Merged
merged 1 commit into from
Jun 10, 2021
Merged

Conversation

nagisa
Copy link
Member

@nagisa nagisa commented Jun 5, 2021

This addresses a codegen-issue that needs to be fixed upstream in LLVM.
While we wait for the fix, we can disable it.

Verified manually that the outliner is no longer run when
-Copt-level=z is specified, and also that you can override this with
-Cllvm-args=-enable-machine-outliner if you need it anyway.

A regression test is not really feasible in this instance, given that we
do not have any minimal reproducers.

Fixes #85351

cc @pnkfelix

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 5, 2021
@nagisa nagisa force-pushed the nagisa/outliner branch from d881feb to 92d4db5 Compare June 5, 2021 11:43
@rust-log-analyzer

This comment has been minimized.

This addresses a codegen-issue that needs to be fixed upstream in LLVM.
While we wait for the fix, we can disable it.

Verified manually that the outliner is no longer run when
`-Copt-level=z` is specified, and also that you can override this with
`-Cllvm-args=-enable-machine-outliner` if you need it anyway.

A regression test is not really feasible in this instance, given that we
do not have any minimal reproducers.

Fixes rust-lang#85351
@nagisa nagisa force-pushed the nagisa/outliner branch from 92d4db5 to c63a1c0 Compare June 5, 2021 11:57
@nagisa nagisa added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. beta-nominated Nominated for backporting to the compiler in the beta channel. labels Jun 6, 2021
@pnkfelix
Copy link
Member

@bors r+ p=10

@bors
Copy link
Contributor

bors commented Jun 10, 2021

📌 Commit c63a1c0 has been approved by pnkfelix

@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 Jun 10, 2021
@wesleywiser
Copy link
Member

@bors rollup=never

Changing LLVM optimization options can affect perf.

@bors
Copy link
Contributor

bors commented Jun 10, 2021

⌛ Testing commit c63a1c0 with merge 1f949e9...

@apiraino
Copy link
Contributor

apiraino commented Jun 10, 2021

Beta backport accepted as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 10, 2021
@bors
Copy link
Contributor

bors commented Jun 10, 2021

☀️ Test successful - checks-actions
Approved by: pnkfelix
Pushing 1f949e9 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 10, 2021
@bors bors merged commit 1f949e9 into rust-lang:master Jun 10, 2021
@rustbot rustbot added this to the 1.54.0 milestone Jun 10, 2021
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.54.0, 1.53.0 Jun 11, 2021
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 11, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 11, 2021
…ulacrum

[beta] backports

* Disable the machine outliner by default rust-lang#86020
* Fix incorrect gating of nonterminals in key-value attributes rust-lang#85445
* Build crtbegin.o/crtend.o from source code rust-lang#85395
* Bring back x86_64-sun-solaris target to rustup rust-lang#85252
* Preserve SyntaxContext for invalid/dummy spans in crate metadata rust-lang#85211
* [beta] backport: Remove unsound TrustedRandomAccess implementations rust-lang#86222

r? `@Mark-Simulacrum`
robin-nitrokey added a commit to robin-nitrokey/solo2 that referenced this pull request Jul 5, 2021
Rust 1.53.0 automatically sets the --enable-machine-outliner=never LLVM
argument [0] [1].  As we also set this option in
runners/lpc55/.cargo/config, we currently get a compliation error.  This
patch removes the option from our cargo configuration to fix the build.

[0] rust-lang/rust@c63a1c0
[1] rust-lang/rust#86020
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 28, 2021
Revert "Disable the machine outliner by default"

The fix commit is already in the fork: rust-lang/llvm-project@6c78dbd4ca1f
Linked:
- rust-lang#85351
- rust-lang#86020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Miscompilation on ARM-M with nightly-2021-04-23
10 participants