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

Permit MIR inlining without #[inline] #109247

Merged
merged 3 commits into from
Apr 17, 2023
Merged
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
10 changes: 2 additions & 8 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,14 +350,8 @@ impl<'tcx> Inliner<'tcx> {
callsite: &CallSite<'tcx>,
callee_attrs: &CodegenFnAttrs,
) -> Result<(), &'static str> {
match callee_attrs.inline {
InlineAttr::Never => return Err("never inline hint"),
InlineAttr::Always | InlineAttr::Hint => {}
InlineAttr::None => {
if self.tcx.sess.mir_opt_level() <= 2 {
return Err("at mir-opt-level=2, only #[inline] is inlined");
}
}
if let InlineAttr::Never = callee_attrs.inline {
return Err("never inline hint");
}

// Only inline local functions if they would be eligible for cross-crate
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags:-Zprint-mono-items=eager
// compile-flags:-Zprint-mono-items=eager -Zinline-mir=no

#![deny(dead_code)]
#![feature(start)]
Expand Down
3 changes: 1 addition & 2 deletions tests/codegen-units/item-collection/function-as-argument.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//
// compile-flags:-Zprint-mono-items=eager
// compile-flags:-Zprint-mono-items=eager -Zinline-mir=no

#![deny(dead_code)]
#![feature(start)]
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen-units/item-collection/generic-functions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags:-Zprint-mono-items=eager
// compile-flags:-Zprint-mono-items=eager -Zinline-mir=no

#![deny(dead_code)]
#![feature(start)]
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen-units/item-collection/generic-impl.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags:-Zprint-mono-items=eager
// compile-flags:-Zprint-mono-items=eager -Zinline-mir=no

#![deny(dead_code)]
#![feature(start)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags:-Zprint-mono-items=eager
// compile-flags:-Zprint-mono-items=eager -Zinline-mir=no

#![deny(dead_code)]
#![feature(start)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//
// compile-flags:-Zprint-mono-items=eager
// compile-flags:-Zprint-mono-items=eager -Zinline-mir=no

#![deny(dead_code)]
#![feature(start)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags:-Zprint-mono-items=eager -Zpolymorphize=on
// compile-flags:-Zprint-mono-items=eager -Zpolymorphize=on -Zinline-mir=no

#![deny(dead_code)]
#![feature(start)]
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/array-map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub fn short_integer_map(x: [u32; 8]) -> [u32; 8] {
pub fn short_integer_zip_map(x: [u32; 8], y: [u32; 8]) -> [u32; 8] {
// CHECK: %[[A:.+]] = load <8 x i32>
// CHECK: %[[B:.+]] = load <8 x i32>
// CHECK: sub <8 x i32> %[[A]], %[[B]]
// CHECK: sub <8 x i32> %[[B]], %[[A]]
Copy link
Contributor

Choose a reason for hiding this comment

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

uh, what happened here? Is this just a reordering of the loads but not a reordering of the actual subtraction?

Copy link
Contributor

Choose a reason for hiding this comment

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

r=me after making sure that this test doesn't get miscompiled

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know exactly what is going on here but the test isn't miscompiled. It's our aggressive abbreviations in this codegen test that make it look suspicious. Using -Zmir-opt-level=3 to simulate the effect of this PR because it lifts the inline attribute requirement: https://godbolt.org/z/EP7aTcz1x

Before:

define void @_ZN7example21short_integer_zip_map17hbd5b1e22189a0746E(ptr noalias nocapture noundef writeonly sret([8 x i32]) dereferenceable(32) %0, ptr noalias nocapture noundef readonly dereferenceable(32) %x, ptr noalias nocapture noundef readonly dereferenceable(32) %y) unnamed_addr #0 personality ptr @rust_eh_personality !dbg !6 {
  %1 = load <8 x i32>, ptr %x, align 4, !dbg !11
  %2 = load <8 x i32>, ptr %y, align 4, !dbg !11
  %3 = sub <8 x i32> %1, %2, !dbg !12
  store <8 x i32> %3, ptr %0, align 4, !dbg !62, !alias.scope !63, !noalias !66
  ret void, !dbg !68
}

After:

define void @_ZN7example21short_integer_zip_map17hbd5b1e22189a0746E(ptr noalias nocapture noundef writeonly sret([8 x i32]) dereferenceable(32) %0, ptr noalias nocapture noundef readonly dereferenceable(32) %x, ptr noalias nocapture noundef readonly dereferenceable(32) %y) unnamed_addr #0 personality ptr @rust_eh_personality !dbg !6 {
  %1 = load <8 x i32>, ptr %y, align 4, !dbg !11
  %2 = load <8 x i32>, ptr %x, align 4, !dbg !18
  %3 = sub <8 x i32> %2, %1, !dbg !19
  store <8 x i32> %3, ptr %0, align 4, !dbg !65, !alias.scope !66, !noalias !69
  ret void, !dbg !71
}

// CHECK: store <8 x i32>
x.zip(y).map(|(x, y)| x - y)
}
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/inline-hint.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Checks that closures, constructors, and shims except
// for a drop glue receive inline hint by default.
//
// compile-flags: -Cno-prepopulate-passes -Csymbol-mangling-version=v0
// compile-flags: -Cno-prepopulate-passes -Csymbol-mangling-version=v0 -Zinline-mir=no
#![crate_type = "lib"]

pub fn f() {
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/local-generics-in-exe-internalized.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags: -C no-prepopulate-passes -Zshare-generics=yes
// compile-flags: -C no-prepopulate-passes -Zshare-generics=yes -Zinline-mir=no

// Check that local generics are internalized if they are in the same CGU

Expand Down
4 changes: 3 additions & 1 deletion tests/codegen/remap_path_prefix/auxiliary/xcrate-generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@

#![crate_type = "lib"]

pub fn foo<T>() {}
pub fn foo<T: Default>() -> T {
T::default()
}
2 changes: 1 addition & 1 deletion tests/codegen/remap_path_prefix/xcrate-generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
extern crate xcrate_generic;

pub fn foo() {
xcrate_generic::foo::<u32>();
println!("{}", xcrate_generic::foo::<u32>());
}

// Here we check that local debuginfo is mapped correctly.
Expand Down