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.

Sign up for GitHub

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

Jump to bottom

Disable alignment checks on i686-pc-windows-msvc #112684

Merged
merged 2 commits into from
Jun 17, 2023
Merged

Disable alignment checks on i686-pc-windows-msvc #112684

merged 2 commits into from
Jun 17, 2023

Conversation

saethlin
Copy link
Member

@saethlin saethlin commented Jun 15, 2023 •
edited by wesleywiser
Loading

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 15, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jun 15, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@wesleywiser wesleywiser added beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Jun 15, 2023
@saethlin
Copy link
Member Author

Thanks for the tweak. I was thinking it would be good to cite something but I wasn't sure where.

@wesleywiser
Copy link
Member

You're welcome. Thanks for the quick turn around on this!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 15, 2023

📌 Commit 55d680b has been approved by wesleywiser

It is now in the queue for this repository.

@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 15, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 16, 2023
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 16, 2023
@matthiaskrgr
Rollup merge of rust-lang#112684 - saethlin:ignore-windows-alignment,…
91f428d 
… r=wesleywiser

Disable alignment checks on i686-pc-windows-msvc

r? `@wesleywiser` Because you were in the Zulip discussion of this: https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202023-06-15

cc rust-lang#112480
scottmcm
scottmcm reviewed Jun 16, 2023
View reviewed changes
compiler/rustc_mir_transform/src/check_alignment.rs
@@ -14,6 +14,10 @@ pub struct CheckAlignment;

impl<'tcx> MirPass<'tcx> for CheckAlignment {
fn is_enabled(&self, sess: &Session) -> bool {
// FIXME(#112480) MSVC and rustc disagree on minimum stack alignment on x86 Windows
if sess.target.llvm_target == "i686-pc-windows-msvc" {
Copy link
Member

Choose a reason for hiding this comment

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

Is this potentially going to make some of the MIR tests for this fail on i686-msvc? Are there any tests that should get something like a ignore-i686-pc-windows-msvc?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think just the test for the alignment check itself. The mir-opt tests have debug assertions disabled and I think I was otherwise pretty careful to modify the other tests so that they are not perturbed by them, but that was some time ago.

I'd prefer to be able to run the msvc test suite locally but I don't think that's supported on a Linux host?

@compiler-errors
Copy link
Member

@bors r- failed in rollup: #112689 (comment)

@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 16, 2023
@saethlin saethlin force-pushed the ignore-windows-alignment branch from 55d680b to 0a445af Compare June 16, 2023 13:05
@saethlin saethlin force-pushed the ignore-windows-alignment branch from 0a445af to c54672e Compare June 16, 2023 13:06
@oli-obk
Copy link
Contributor

oli-obk commented Jun 16, 2023

@bors r=wesleywiser

@bors
Copy link
Contributor

bors commented Jun 16, 2023

📌 Commit c54672e has been approved by wesleywiser

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 16, 2023
@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 16, 2023

📌 Commit 4ef316f has been approved by wesleywiser

It is now in the queue for this repository.

@compiler-errors compiler-errors mentioned this pull request Jun 16, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 16, 2023
@bors
Auto merge of rust-lang#112716 - compiler-errors:rollup-h77daia, r=co…
670a0ed 
…mpiler-errors

Rollup of 7 pull requests

Successful merges:

 - rust-lang#111074 (Relax implicit `T: Sized` bounds on `BufReader<T>`, `BufWriter<T>` and `LineWriter<T>`)
 - rust-lang#112226 (std: available_parallelism using native netbsd api first)
 - rust-lang#112474 (Support 128-bit enum variant in debuginfo codegen)
 - rust-lang#112662 (`#[lang_item]` for `core::ptr::Unique`)
 - rust-lang#112665 (Make assumption functions in new solver take `Binder<'tcx, Clause<'tcx>>`)
 - rust-lang#112684 (Disable alignment checks on i686-pc-windows-msvc)
 - rust-lang#112706 (Add `SyntaxContext::is_root`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a5f8859 into rust-lang:master Jun 17, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 17, 2023
@apiraino
Copy link
Contributor

Backports approved as per compiler team on Zulip

@rustbot label +beta-accepted +stable-accepted

@rustbot rustbot added beta-accepted Accepted for backporting to the compiler in the beta channel. stable-accepted Accepted for backporting to the compiler in the stable channel. labels Jun 22, 2023
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 24, 2023
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.72.0, 1.71.0 Jun 24, 2023
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Jun 24, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 24, 2023
@bors
Auto merge of rust-lang#113000 - Mark-Simulacrum:beta-backport, r=Mar…
dbf31f1 
…k-Simulacrum

[beta] backport

This PR backports:

- rust-lang#112684: Disable alignment checks on i686-pc-windows-msvc
- rust-lang#112581: [rustdoc] Fix URL encoding of % sign
- rust-lang#112312: Update to LLVM 16.0.5
- rust-lang#112266: Fix type-inference regression in rust-lang#112225
- rust-lang#112062: Make struct layout not depend on unsizeable tail

r? `@Mark-Simulacrum`
@saethlin saethlin deleted the ignore-windows-alignment branch June 27, 2023 02:23
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 5, 2023
Closed
@RalfJung RalfJung mentioned this pull request Jul 5, 2023
Open
@RalfJung
Copy link
Member

RalfJung commented Jul 5, 2023

I don't understand why the tags here indicate a stable backport? #98112 is on the 1.70 train, this here landed for the 1.71 train, so a beta backport should be sufficient.

@ChrisDenton
Copy link
Member

Because if there were a 1.70.1 release then it would be worth including it.

@RalfJung
Copy link
Member

RalfJung commented Jul 5, 2023 •
edited
Loading

Oh, when I said this landed on the 1.71 train I was wrong -- the milestone in this PR got changed and no longer reflects the train it originally landed in. That lead me down the wrong trail. This landed in the 1.72 train.

@Mark-Simulacrum Mark-Simulacrum removed the stable-nominated Nominated for backporting to the compiler in the stable channel. label Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottmcm scottmcm scottmcm left review comments

@wesleywiser wesleywiser wesleywiser approved these changes

Assignees

@wesleywiser wesleywiser

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. stable-accepted Accepted for backporting to the compiler in the stable channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.