Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
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
Loading

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
# for free to join this conversation on GitHub. Already have an account? # 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