Skip to content

Remove 'simple array' lowering of format_args!(). #139175

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

Closed
wants to merge 1 commit into from
Closed

Remove 'simple array' lowering of format_args!(). #139175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
66 changes: 0 additions & 66 deletions compiler/rustc_ast_lowering/src/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use core::ops::ControlFlow;
use std::borrow::Cow;

use rustc_ast::visit::Visitor;
use rustc_ast::*;
use rustc_data_structures::fx::FxIndexMap;
use rustc_hir as hir;
Expand Down Expand Up @@ -476,17 +474,6 @@ fn expand_format_args<'hir>(
return hir::ExprKind::Call(new, new_args);
}

// If the args array contains exactly all the original arguments once,
// in order, we can use a simple array instead of a `match` construction.
// However, if there's a yield point in any argument except the first one,
// we don't do this, because an Argument cannot be kept across yield points.
//
// This is an optimization, speeding up compilation about 1-2% in some cases.
// See https://github.com/rust-lang/rust/pull/106770#issuecomment-1380790609
let use_simple_array = argmap.len() == arguments.len()
&& argmap.iter().enumerate().all(|(i, (&(j, _), _))| i == j)
&& arguments.iter().skip(1).all(|arg| !may_contain_yield_point(&arg.expr));

let args = if arguments.is_empty() {
// Generate:
// &<core::fmt::Argument>::none()
Expand All @@ -510,31 +497,6 @@ fn expand_format_args<'hir>(
));
let none = ctx.expr_call(macsp, none_fn, &[]);
ctx.expr(macsp, hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, none))
} else if use_simple_array {
// Generate:
// &[
// <core::fmt::Argument>::new_display(&arg0),
// <core::fmt::Argument>::new_lower_hex(&arg1),
// <core::fmt::Argument>::new_debug(&arg2),
// …
// ]
let elements = ctx.arena.alloc_from_iter(arguments.iter().zip(argmap).map(
|(arg, ((_, ty), placeholder_span))| {
let placeholder_span =
placeholder_span.unwrap_or(arg.expr.span).with_ctxt(macsp.ctxt());
let arg_span = match arg.kind {
FormatArgumentKind::Captured(_) => placeholder_span,
_ => arg.expr.span.with_ctxt(macsp.ctxt()),
};
let arg = ctx.lower_expr(&arg.expr);
let ref_arg = ctx.arena.alloc(ctx.expr(
arg_span,
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, arg),
));
make_argument(ctx, placeholder_span, ref_arg, ty)
},
));
ctx.expr_array_ref(macsp, elements)
} else {
// Generate:
// &match (&arg0, &arg1, &…) {
Expand Down Expand Up @@ -635,34 +597,6 @@ fn expand_format_args<'hir>(
}
}

fn may_contain_yield_point(e: &ast::Expr) -> bool {
struct MayContainYieldPoint;

impl Visitor<'_> for MayContainYieldPoint {
type Result = ControlFlow<()>;

fn visit_expr(&mut self, e: &ast::Expr) -> ControlFlow<()> {
if let ast::ExprKind::Await(_, _) | ast::ExprKind::Yield(_) = e.kind {
ControlFlow::Break(())
} else {
visit::walk_expr(self, e)
}
}

fn visit_mac_call(&mut self, _: &ast::MacCall) -> ControlFlow<()> {
// Macros should be expanded at this point.
unreachable!("unexpanded macro in ast lowering");
}

fn visit_item(&mut self, _: &ast::Item) -> ControlFlow<()> {
// Do not recurse into nested items.
ControlFlow::Continue(())
}
}

MayContainYieldPoint.visit_expr(e).is_break()
}

fn for_all_argument_indexes(template: &mut [FormatArgsPiece], mut f: impl FnMut(&mut usize)) {
for piece in template {
let FormatArgsPiece::Placeholder(placeholder) = piece else { continue };
Expand Down
135 changes: 88 additions & 47 deletions tests/mir-opt/sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,27 @@
let mut _15: &[core::fmt::rt::Argument<'_>; 2];
let _16: &[core::fmt::rt::Argument<'_>; 2];
let _17: [core::fmt::rt::Argument<'_>; 2];
let mut _18: core::fmt::rt::Argument<'_>;
let mut _18: (&std::boxed::Box<dyn std::fmt::Display>, &u32);
let mut _19: &std::boxed::Box<dyn std::fmt::Display>;
let _20: &std::boxed::Box<dyn std::fmt::Display>;
let mut _21: core::fmt::rt::Argument<'_>;
let mut _22: &u32;
let _23: &u32;
let mut _25: bool;
let mut _26: isize;
let mut _27: isize;
let mut _28: isize;
+ let _29: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>;
+ let _30: u32;
let mut _20: &u32;
let mut _22: core::fmt::rt::Argument<'_>;
let mut _23: &std::boxed::Box<dyn std::fmt::Display>;
let mut _24: core::fmt::rt::Argument<'_>;
let mut _25: &u32;
let mut _27: &std::boxed::Box<dyn std::fmt::Display>;
let mut _28: &u32;
let mut _29: bool;
let mut _30: isize;
let mut _31: isize;
let mut _32: isize;
+ let _33: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>;
+ let _34: u32;
+ let mut _37: &std::boxed::Box<dyn std::fmt::Display>;
+ let mut _38: &u32;
scope 1 {
- debug foo => _1;
+ debug ((foo: Foo<T>).0: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>) => _29;
+ debug ((foo: Foo<T>).1: u32) => _30;
+ debug ((foo: Foo<T>).0: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>) => _33;
+ debug ((foo: Foo<T>).1: u32) => _34;
let _5: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>;
scope 2 {
debug x => _5;
Expand All @@ -42,17 +47,25 @@
scope 4 {
debug x => _8;
let _8: std::boxed::Box<dyn std::fmt::Display>;
let mut _24: &[&str; 3];
let _21: (&std::boxed::Box<dyn std::fmt::Display>, &u32);
let mut _26: &[&str; 3];
+ let _35: &std::boxed::Box<dyn std::fmt::Display>;
+ let _36: &u32;
scope 5 {
- debug args => _21;
+ debug ((args: (&Box<dyn std::fmt::Display>, &u32)).0: &std::boxed::Box<dyn std::fmt::Display>) => _35;
+ debug ((args: (&Box<dyn std::fmt::Display>, &u32)).1: &u32) => _36;
}
}
}
}
}

bb0: {
_25 = const false;
_29 = const false;
- StorageLive(_1);
+ StorageLive(_29);
+ StorageLive(_30);
+ StorageLive(_33);
+ StorageLive(_34);
+ nop;
StorageLive(_2);
StorageLive(_3);
Expand All @@ -66,59 +79,85 @@
_2 = Result::<Box<dyn std::fmt::Display>, <T as Err>::Err>::Ok(move _3);
StorageDead(_3);
- _1 = Foo::<T> { x: move _2, y: const 7_u32 };
+ _29 = move _2;
+ _30 = const 7_u32;
+ _33 = move _2;
+ _34 = const 7_u32;
+ nop;
StorageDead(_2);
StorageLive(_5);
_25 = const true;
_29 = const true;
- _5 = move (_1.0: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>);
+ _5 = move _29;
+ _5 = move _33;
StorageLive(_6);
- _6 = copy (_1.1: u32);
+ _6 = copy _30;
+ _6 = copy _34;
_7 = discriminant(_5);
switchInt(move _7) -> [0: bb2, otherwise: bb7];
}

bb2: {
StorageLive(_8);
_25 = const false;
_29 = const false;
_8 = move ((_5 as Ok).0: std::boxed::Box<dyn std::fmt::Display>);
StorageLive(_9);
StorageLive(_10);
StorageLive(_11);
StorageLive(_12);
StorageLive(_13);
_24 = const foo::<T>::promoted[0];
_13 = &(*_24);
_26 = const foo::<T>::promoted[0];
_13 = &(*_26);
_12 = &(*_13);
StorageLive(_15);
StorageLive(_16);
StorageLive(_17);
StorageLive(_18);
- StorageLive(_18);
+ StorageLive(_37);
+ StorageLive(_38);
+ nop;
StorageLive(_19);
_19 = &_8;
StorageLive(_20);
_20 = &_8;
_19 = &(*_20);
_18 = core::fmt::rt::Argument::<'_>::new_display::<Box<dyn std::fmt::Display>>(move _19) -> [return: bb3, unwind unreachable];
}
bb3: {
_20 = &_6;
- _18 = (move _19, move _20);
+ _37 = move _19;
+ _38 = move _20;
+ nop;
StorageDead(_20);
StorageDead(_19);
StorageLive(_21);
- StorageLive(_21);
- _21 = copy _18;
+ StorageLive(_35);
+ StorageLive(_36);
+ nop;
+ _35 = copy _37;
+ _36 = copy _38;
+ nop;
StorageLive(_22);
StorageLive(_23);
_23 = &_6;
_22 = &(*_23);
_21 = core::fmt::rt::Argument::<'_>::new_display::<u32>(move _22) -> [return: bb4, unwind unreachable];
- _27 = deref_copy (_21.0: &std::boxed::Box<dyn std::fmt::Display>);
+ _27 = deref_copy _35;
_23 = &(*_27);
_22 = core::fmt::rt::Argument::<'_>::new_display::<Box<dyn std::fmt::Display>>(move _23) -> [return: bb3, unwind unreachable];
}

bb3: {
StorageDead(_23);
StorageLive(_24);
StorageLive(_25);
- _28 = deref_copy (_21.1: &u32);
+ _28 = deref_copy _36;
_25 = &(*_28);
_24 = core::fmt::rt::Argument::<'_>::new_display::<u32>(move _25) -> [return: bb4, unwind unreachable];
}

bb4: {
StorageDead(_25);
_17 = [move _22, move _24];
StorageDead(_24);
StorageDead(_22);
_17 = [move _18, move _21];
StorageDead(_21);
StorageDead(_18);
- StorageDead(_21);
+ StorageDead(_35);
+ StorageDead(_36);
+ nop;
_16 = &_17;
_15 = &(*_16);
_11 = Arguments::<'_>::new_v1::<3, 2>(move _12, move _15) -> [return: bb5, unwind unreachable];
Expand All @@ -132,8 +171,10 @@

bb6: {
StorageDead(_11);
StorageDead(_23);
StorageDead(_20);
- StorageDead(_18);
+ StorageDead(_37);
+ StorageDead(_38);
+ nop;
StorageDead(_17);
StorageDead(_16);
StorageDead(_13);
Expand All @@ -156,22 +197,22 @@

bb9: {
StorageDead(_6);
_26 = discriminant(_5);
switchInt(move _26) -> [0: bb11, otherwise: bb13];
_30 = discriminant(_5);
switchInt(move _30) -> [0: bb11, otherwise: bb13];
}

bb10: {
_25 = const false;
_29 = const false;
StorageDead(_5);
- StorageDead(_1);
+ StorageDead(_29);
+ StorageDead(_30);
+ StorageDead(_33);
+ StorageDead(_34);
+ nop;
return;
}

bb11: {
switchInt(copy _25) -> [0: bb10, otherwise: bb12];
switchInt(copy _29) -> [0: bb10, otherwise: bb12];
}

bb12: {
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/borrowck/clone-on-ref.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ LL | drop(x);
| ^ move out of `x` occurs here
LL |
LL | println!("{b}");
| --- borrow later used here
| - borrow later used here
|
help: if `T` implemented `Clone`, you could clone the value
--> $DIR/clone-on-ref.rs:11:8
Expand All @@ -57,7 +57,7 @@ LL | drop(x);
| ^ move out of `x` occurs here
LL |
LL | println!("{b:?}");
| ----- borrow later used here
| - borrow later used here
|
note: if `A` implemented `Clone`, you could clone the value
--> $DIR/clone-on-ref.rs:19:1
Expand Down
23 changes: 0 additions & 23 deletions tests/ui/codegen/equal-pointers-unequal/as-cast/print3.rs
View file
Open in desktop

This file was deleted.

25 changes: 0 additions & 25 deletions tests/ui/codegen/equal-pointers-unequal/exposed-provenance/print3.rs
View file
Open in desktop

This file was deleted.

Loading
Loading