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

Safe code (ui/issues/issue-69225-SCEVAddExpr-wrap-flag.rs) miscompiles under llvm9 + codegen-units=1 #81946

Closed
the8472 opened this issue Feb 10, 2021 · 3 comments · Fixed by #81942
Closed

Safe code (ui/issues/issue-69225-SCEVAddExpr-wrap-flag.rs) miscompiles under llvm9 + codegen-units=1 #81946

the8472 opened this issue Feb 10, 2021 · 3 comments · Fixed by #81942
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority regression-untriaged Untriaged performance or correctness regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@the8472
Copy link
Member

the8472 commented Feb 10, 2021 

---- [ui] ui/issues/issue-69225-SCEVAddExpr-wrap-flag.rs stdout ----

error: error pattern ' index out of bounds: the len is 0 but the index is 16777216' not found!
status: signal: 11
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issues/issue-69225-SCEVAddExpr-wrap-flag/a"

Current reproducer: #81945

Original issue: #69225. According to #69225 (comment) the testcase originally did work fine for any number of codegen units. Seems like a regression.
@jonas-schievink jonas-schievink added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 10, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Feb 10, 2021
@jonas-schievink jonas-schievink added the regression-untriaged Untriaged performance or correctness regression. label Feb 10, 2021
@the8472 the8472 mentioned this issue Feb 10, 2021
Merged
@Stupremee
Copy link
Member

Assigning P-medium as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@Stupremee Stupremee added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Feb 10, 2021
@nagisa
Copy link
Member

nagisa commented Feb 20, 2021 •
edited

I believe we use unpatched/upstream LLVM in for the llvm-9 CI jobs, which would make this technically a not-a-rustc-issue, given that the original fix relied on patching LLVM (10 &later).

Conceptually it'd be upto the distribution maintainers to apply corresponding patches to the LLVM they ship.

@nikic
Copy link
Contributor

nikic commented Feb 20, 2021

Right, I don't think there's anything we can really do here, as it's a bug in external LLVM. I think simply adding a min-llvm-version to the test would be fine.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 27, 2021
@Dylan-DPC
Rollup merge of rust-lang#81942 - the8472:reduce-ui-test-threads, r=M…
a9dac65 
…ark-Simulacrum

reduce threads spawned by ui-tests

The test harness already spawns enough tests to keep all cores busy.
Individual tests should keep their own threading to a minimum to avoid context switch overhead.

When running ui tests with lld enabled this shaves about 10% off that testsuite on my machine.

Resolves rust-lang#81946
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 4, 2021
@Dylan-DPC
Rollup merge of rust-lang#81942 - the8472:reduce-ui-test-threads, r=M…
e3e915c 
…ark-Simulacrum

reduce threads spawned by ui-tests

The test harness already spawns enough tests to keep all cores busy.
Individual tests should keep their own threading to a minimum to avoid context switch overhead.

When running ui tests with lld enabled this shaves about 10% off that testsuite on my machine.

Resolves rust-lang#81946
@bors bors closed this as completed in da0b9b6 Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority regression-untriaged Untriaged performance or correctness regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

reduce threads spawned by ui-tests the8472/rust
6 participants
@nikic @nagisa @the8472 @jonas-schievink @Stupremee @rustbot