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

Miscompilation of libcore with -Zmir-opt-level=3 #108166

Closed
bjorn3 opened this issue Feb 17, 2023 · 3 comments · Fixed by #108208
Closed

Miscompilation of libcore with -Zmir-opt-level=3 #108166

bjorn3 opened this issue Feb 17, 2023 · 3 comments · Fixed by #108208
Labels
A-mir-opt Area: MIR optimizations C-bug Category: This is a bug.

Comments

@bjorn3
Copy link
Member

bjorn3 commented Feb 17, 2023

Compile the standard library with -Zmir-opt-level=3 and then running libcore's test suite with this version of the standard library results in several tests failing. I am seeing this on cg_clif's CI (https://github.com/bjorn3/rustc_codegen_cranelift/actions/runs/4203400456/jobs/7292743420) but I expect it to be reproducible using LLVM too.

Meta

Last known good version: rustc 1.69.0-nightly (065852def 2023-02-13)

First known bad version:

rustc 1.69.0-nightly (9a7cc6c32 2023-02-16)
binary: rustc
commit-hash: 9a7cc6c32f1a690f86827e4724bcda85e506ef35
commit-date: 2023-02-16
host: aarch64-unknown-linux-gnu
release: 1.69.0-nightly
LLVM version: 15.0.7

Regression range: 065852d...9a7cc6c

#107449 stands out as a changed MIR opt.
@bjorn3 bjorn3 added C-bug Category: This is a bug. A-mir-opt Area: MIR optimizations labels Feb 17, 2023
bjorn3 added a commit to rust-lang/rustc_codegen_cranelift that referenced this issue Feb 17, 2023
@bjorn3
Workaround rust-lang/rust#108166
effa8c4
@saethlin
Copy link
Member

saethlin commented Feb 17, 2023 •
edited
Loading

Try adding -Zmir-enable-passes=-DataflowConstProp to your RUSTFLAGS instead of lowering your MIR opt level. That makes the miscompiles go away for me and suggests that #107411 is the cause.

Turning off CopyProp has no effect, so it seems very unlikely that pass is involved. Turning off MIR inlining also makes the miscompiles go away, but that's probably just because that makes a lot of other MIR opts just not fire.

@saethlin
Copy link
Member

saethlin commented Feb 17, 2023 •
edited
Loading

Also cc #105706 + #105736 which are stuck in "I'm not sure how to make this work" limbo, but might/should have prevented this.

@cjgillot cjgillot mentioned this issue Feb 18, 2023
Merged
compiler-errors added a commit to compiler-errors/rust that referenced this issue Feb 21, 2023
@compiler-errors
Rollup merge of rust-lang#108208 - cjgillot:flood-enum, r=oli-obk
ff1bd3d 
Correctly handle aggregates in DataflowConstProp

The previous implementation from rust-lang#107411 flooded target of an aggregate assignment with `Bottom`, corresponding to the `deinit` that the interpreter does.

As a consequence, when assigning `target = Enum::Variant#i(...)` all the `(target as Variant#j)` were at `Bottom` while they should have been `Top`.

This PR replaces that flooding with `Top`.

Aside, it corrects a second bug where the wrong place would be used to assign to enum variant fields, resulting to nothing happening.

Fixes rust-lang#108166
@bors bors closed this as completed in a423fa7 Feb 23, 2023
@bjorn3
Copy link
Member Author

bjorn3 commented Feb 26, 2023

I can confirm the miscompilation is now fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-mir-opt Area: MIR optimizations C-bug Category: This is a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Correctly handle aggregates in DataflowConstProp cjgillot/rust
2 participants
@saethlin @bjorn3