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.

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

Missed optimization: codegen test repeat-trusted-len.rs fails when stdlib is compiled with overflow-checks=on #72549

Closed
alex opened this issue May 24, 2020 · 7 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alex
Copy link
Member

alex commented May 24, 2020

This is another discovery in my work on #58475 / rust-lang/rfcs#2635 -- trying to see if it's practical to enable overflow checks by default in release builds.

This fails with:

stderr:
------------------------------------------
/home/alex/p/rust/src/test/codegen/repeat-trusted-len.rs:11:11: error: CHECK: expected string not found in input
// CHECK: call void @llvm.memset.p0i8.i{{[0-9]+}}(i8* {{(nonnull )?}}align 1{{.*}} %{{[0-9]+}}, i8 42, i{{[0-9]+}} 100000, i1 false)
          ^
/home/alex/p/rust/build/x86_64-unknown-linux-gnu/test/codegen/repeat-trusted-len/repeat-trusted-len.ll:51:33: note: scanning from here
define void @repeat_take_collect(%"alloc::vec::Vec<u8>"* noalias nocapture sret dereferenceable(24) %0) unnamed_addr #2 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {

The issue seems to be that the LLVM loop-idiom recognizer runs before the optimizations that remove the overflow checks, therefor you get a loop in assembly, instead of a call to memset. I've filed a bug against LLVM with this: https://bugs.llvm.org/show_bug.cgi?id=46057 , but I'm also filing a bug here so there's a way to track the rust side impact of it

@Elinvynia Elinvynia added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 31, 2020
@the8472
Copy link
Member

the8472 commented Oct 11, 2021

Odd, this doesn't reproduce on godbolt (i.e. there's a memset), even when I go back to a release from 2020.

https://rust.godbolt.org/z/WjM7jz619

@alex
Copy link
Member Author

alex commented Oct 11, 2021 via email

@the8472
Copy link
Member

the8472 commented Oct 11, 2021

Right, I forgot, although it depends on whether the methods in question are annotated with #[rustc_inherit_overflow_checks].

@the8472
Copy link
Member

the8472 commented Oct 11, 2021

We could work around that LLVM shortcoming by using unchecked add/sub which have been added in the meantime. #85122

@hkratz
Copy link
Contributor

hkratz commented Oct 12, 2021

Can't seem to reproduce it. Maybe it is gone with the new LLVM pass manager?

@alex
Copy link
Member Author

alex commented Oct 12, 2021

Maybe? The C reproucer I filed the LLVM bug with still reproduces for me.

@alex
Copy link
Member Author

alex commented Dec 30, 2023

The LLVM bug I filed no longer reproduces, so I'm going to close this.

@alex alex closed this as completed Dec 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants