Skip to content

Add Three Codegen Tests #134626

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions tests/assembly/indexing-with-bools-no-redundant-instructions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//@ assembly-output: emit-asm
//@ only-x86_64
//@ ignore-sgx Test incompatible with LVI mitigations
//@ compile-flags: -C opt-level=3
//! Ensure that indexing a slice with `bool` does not
//! generate any redundant `jmp` and `and` instructions.
//! Discovered in issue #123216.

#![crate_type = "lib"]

#[no_mangle]
fn f(a: u32, b: bool, c: bool, d: &mut [u128; 2]) {
// CHECK-LABEL: f:
// CHECK: testl %esi, %esi
// CHECK: je
// CHECK: xorb %dl, %dil
// CHECK: orb $1, (%rcx)
// CHECK: movzbl %dil, %eax
// CHECK: andl $1, %eax
// CHECK: shll $4, %eax
// CHECK: orb $1, (%rcx,%rax)
// CHECK-NOT: jmp
Copy link
Member

Choose a reason for hiding this comment

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

Just a question: should this be an assembly test?
The IR change was introduced by llvm/llvm-project#84628, and this is not a backend change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think assembly test would be better because there isn't a big difference in the IR: https://rust.godbolt.org/z/sd7G1rsa7

Thanks

// CHECK-NOT: andl %dil, $1
// CHECK: retq
let mut a = a & 1 != 0;
if b {
a ^= c;
d[0] |= 1;
}
d[a as usize] |= 1;
Comment on lines +25 to +30
Copy link
Member

Choose a reason for hiding this comment

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

This is a bunch of kinda-random unjustified code. What's special about it? What makes it worth us having a test for it? Can we focus it way more somehow?

I ask because this is the kind of test that tends to rot -- people look at it, there's one assembly instruction different, go "eh, probably fine" and just update the CHECKs, quickly leading to it not really testing anything that matters any more.

What changed? Where was it fixed? Can we add a more specific test somehow?

If we don't already have a test for "we can eliminate the bounds check for indexing with bool into a [T; 2]" that sounds like a good test. But this particular combination with xors and such? I'm just not convinced.

Copy link
Member

Choose a reason for hiding this comment

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

Especially since, as you can see by the failure in bors, doing this in assembly coupled it to the calling convention, so it can't pass on both linux and windows with hard-coded register names like this.

}
20 changes: 20 additions & 0 deletions tests/codegen/nonzero-type-not-zero-on-get.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//@ compile-flags: -C opt-level=3
//! Ensure that `.get()` on `std::num::NonZero*` types do not
//! check for zero equivalency.
//! Discovered in issue #49572.

#![crate_type = "lib"]

#[no_mangle]
pub fn foo(x: std::num::NonZeroU32) -> bool {
// CHECK-LABEL: @foo(
// CHECK: ret i1 true
x.get() != 0
}

#[no_mangle]
pub fn bar(x: std::num::NonZeroI64) -> bool {
// CHECK-LABEL: @bar(
// CHECK: ret i1 true
x.get() != 0
}
39 changes: 39 additions & 0 deletions tests/codegen/unreachable-branch-not-generated.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//@ compile-flags: -C opt-level=3
//! Ensure that matching on `x % 5` generates an unreachable
//! branch for values greater than 4.
//! Discovered in issue #93514.

#![crate_type = "lib"]

#[no_mangle]
pub unsafe fn parse0(x: u32) -> u32 {
// CHECK-LABEL: i32 @parse0(
// CHECK: [[_2:%.*]] = urem
// CHECK-NEXT: switch i32 [[_2]], label %[[DEFAULT_UNREACHABLE1:.*]] [
// CHECK-NEXT: i32 0
// CHECK-NEXT: i32 1
// CHECK-NEXT: i32 2
// CHECK-NEXT: i32 3
// CHECK-NEXT: i32 4
// CHECK-NEXT: ]
// CHECK: [[DEFAULT_UNREACHABLE1]]:
// CHECK-NEXT: unreachable
// CHECK: ret i32
match x % 5 {
0 => f1(x),
1 => f2(x),
2 => f3(x),
3 => f4(x),
4 => f5(x),
_ => eliminate_me(),
}
}

extern "Rust" {
fn eliminate_me() -> u32;
fn f1(x: u32) -> u32;
fn f2(x: u32) -> u32;
fn f3(x: u32) -> u32;
fn f4(x: u32) -> u32;
fn f5(x: u32) -> u32;
}
Loading