Skip to content

Default to disabling the new pass manager for the s390x arch targets. - #89666

Merged
bors merged 1 commit into
rust-lang:masterfrom
rusticstuff:disable_new_llvm_pass_manager_on_s390x_take_two
Oct 8, 2021
Merged

Default to disabling the new pass manager for the s390x arch targets.#89666
bors merged 1 commit into
rust-lang:masterfrom
rusticstuff:disable_new_llvm_pass_manager_on_s390x_take_two

Conversation

@hkratz

@hkratzhkratz commented Oct 8, 2021

Copy link
Copy Markdown
Contributor

This hack disables the new LLVM pass manager by default for s390x arch targets until the performance issues are fixed (see #89609). The command line option -Z new-llvm-pass-manager=(yes|no) continues to take precedence over this default.

@rust-highfive

rust-highfive commented Oct 8, 2021

Copy link
Copy Markdown
Contributor

r? @jackh726

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

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 8, 2021
@hkratzhkratz mentioned this pull request Oct 8, 2021
Closed
@hkratz
hkratz marked this pull request as ready for review October 8, 2021 11:00
nagisa
nagisa reviewed Oct 8, 2021
View reviewed changes
Comment threadcompiler/rustc_codegen_ssa/src/back/write.rs Outdated
@hkratz
hkratzforce-pushed the disable_new_llvm_pass_manager_on_s390x_take_two branch from 84cfa17 to 91801b1CompareOctober 8, 2021 12:51
@hkratz
hkratzforce-pushed the disable_new_llvm_pass_manager_on_s390x_take_two branch from 91801b1 to 4593d78CompareOctober 8, 2021 13:05
@nagisa

nagisa commented Oct 8, 2021

Copy link
Copy Markdown
Member

@bors r+

Thanks!

@bors

bors commented Oct 8, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit 4593d78 has been approved by nagisa

@borsbors 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 Oct 8, 2021
@nagisa

nagisa commented Oct 8, 2021

Copy link
Copy Markdown
Member

cc @nikic@cuviper

@camelid

camelid commented Oct 8, 2021•
edited
Loading

Copy link
Copy Markdown
Member

Should the priority of this be bumped since it fixes CI timeouts and in general may speed up CI? E.g., to p=1.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Oct 8, 2021
@GuillaumeGomez
Rollup merge ofrust-lang#89666- rusticstuff:disable_new_llvm_pass_m…
d7bb8a7
…anager_on_s390x_take_two, r=nagisa
Default to disabling the new pass manager for the s390x arch targets.
This hack disables the new LLVM pass manager by default for s390x arch targets until the performance issues are fixed (see rust-lang#89609). The command line option `-Z new-llvm-pass-manager=(yes|no)` continues to take precedence over this default.
@GuillaumeGomezGuillaumeGomez mentioned this pull request Oct 8, 2021
Closed
@camelid

camelid commented Oct 8, 2021

Copy link
Copy Markdown
Member

@bors p=1

@bors

bors commented Oct 8, 2021

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 4593d78 with merge 54bb4fe...

@camelid

camelid commented Oct 8, 2021

Copy link
Copy Markdown
Member

The rollup this is a part of failed, and the next PR in the queue was #88379, which is blocked by this one. So, I bumped the priority of this one so that it merges before #88379.

@bors

bors commented Oct 8, 2021

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 54bb4fe to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Oct 8, 2021
@bors
bors merged commit 54bb4fe into rust-lang:masterOct 8, 2021
@rustbotrustbot added this to the 1.57.0 milestone Oct 8, 2021
@cuviper

cuviper commented Oct 8, 2021

Copy link
Copy Markdown
Member

auto (dist-s390x-linux, ubuntu-latest-xl)
succeeded 2 hours ago in 41m 27s

🎉

@rust-timer

rust-timer commented Oct 9, 2021

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (54bb4fe): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@cuvipercuviper mentioned this pull request Oct 21, 2021
Closed
@hkratz
hkratz deleted the disable_new_llvm_pass_manager_on_s390x_take_two branch November 6, 2021 20:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nagisanagisanagisa left review comments

Assignees

@jackh726jackh726

Labels

merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.57.0

Development

Successfully merging this pull request may close these issues.

9 participants

@hkratz@rust-highfive@nagisa@bors@camelid@cuviper@rust-timer@jackh726@rustbot