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

Make codegen tests compatible with extra inlining #78967

Merged
merged 3 commits into from
Nov 17, 2020
Merged

Make codegen tests compatible with extra inlining #78967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
899d9b9
Fix test checking that into_boxed_slice does not panic
tmiasko Nov 12, 2020
28463aa
Ensure that closure call is not removed by MIR optimizations in a test
tmiasko Nov 12, 2020
d54ea4f
Fix codegen test for issue 37945
tmiasko Nov 12, 2020
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
2 changes: 1 addition & 1 deletion src/test/codegen/internalize-closures.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags: -C no-prepopulate-passes
// compile-flags: -C no-prepopulate-passes -Zmir-opt-level=0

pub fn main() {

Expand Down
20 changes: 14 additions & 6 deletions src/test/codegen/issue-37945.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,34 @@
// compile-flags: -O
// compile-flags: -O -Zmerge-functions=disabled
// ignore-x86
// ignore-arm
// ignore-emscripten
// ignore-gnux32
// ignore 32-bit platforms (LLVM has a bug with them)

// See issue #37945.
// Check that LLVM understands that `Iter` pointer is not null. Issue #37945.

#![crate_type = "lib"]

use std::slice::Iter;

// CHECK-LABEL: @is_empty_1
#[no_mangle]
pub fn is_empty_1(xs: Iter<f32>) -> bool {
// CHECK-NOT: icmp eq float* {{.*}}, null
// CHECK-LABEL: @is_empty_1(
// CHECK-NEXT: start:
// CHECK-NEXT: [[A:%.*]] = icmp ne i32* %xs.1, null
Copy link
Member

Choose a reason for hiding this comment

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

Check that LLVM understands that Iter pointer is not null.

If we have icmp ne ..., null, it seems like we're failing the thing we meant to test, no?

I don't think we should try to compare exactly what LLVM IR comes out, but perhaps something like:

/// CHECK-NOT: icmp {{.*}}, null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we have icmp ne ..., null, it seems like we're failing the thing we meant to test, no?

It was fixed upstream by canonicalizing to assume.

Copy link
Member

Choose a reason for hiding this comment

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

By "it was fixed upstream", do you mean that will be in LLVM 12? Is there something we could cherry-pick?

My point is the test meant to check that LLVM knows it's not null, so we had a CHECK-NOT: icmp eq ..., null to that effect. Your change reveals that while we were passing that test, we're getting icmp ne ..., null instead, which seems just as bad. So it seems like a real bug with or without your 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.

The fact that pointer is not-null is expressed through llvm.assume that follows, see next check.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, ok, I see what you mean...

// CHECK-NEXT: tail call void @llvm.assume(i1 [[A]])
// CHECK-NEXT: [[B:%.*]] = icmp eq i32* %xs.0, %xs.1
// CHECK-NEXT: ret i1 [[B:%.*]]
{xs}.next().is_none()
}

// CHECK-LABEL: @is_empty_2
#[no_mangle]
pub fn is_empty_2(xs: Iter<f32>) -> bool {
// CHECK-NOT: icmp eq float* {{.*}}, null
// CHECK-LABEL: @is_empty_2
// CHECK-NEXT: start:
// CHECK-NEXT: [[C:%.*]] = icmp ne i32* %xs.1, null
// CHECK-NEXT: tail call void @llvm.assume(i1 [[C]])
// CHECK-NEXT: [[D:%.*]] = icmp eq i32* %xs.0, %xs.1
// CHECK-NEXT: ret i1 [[D:%.*]]
xs.map(|&x| x).next().is_none()
}
4 changes: 2 additions & 2 deletions src/test/codegen/vec-shrink-panic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ pub fn shrink_to_fit(vec: &mut Vec<u32>) {

// CHECK-LABEL: @issue71861
#[no_mangle]
pub fn issue71861(n: usize) -> Box<[u32]> {
pub fn issue71861(vec: Vec<u32>) -> Box<[u32]> {
// CHECK-NOT: panic
vec![0; n].into_boxed_slice()
vec.into_boxed_slice()
}

// CHECK-LABEL: @issue75636
Expand Down