Skip to content

Commit 19f2dc7

Browse files
borsbors
committed
Auto merge of rust-lang#139175 - m-ou-se:format-args-opt-remove, r=<try>
Remove 'simple array' lowering of format_args!(). format_args!() uses a simpler (faster to compile) lowering in simple cases. However, selecting that case is somewhat complicated, as it does not work if any argument beyond the first one contains a yield point, so we have to check for that. As part of the solution for rust-lang#92698 and rust-lang#139136, it might need to get even more complicated, checking for any lifetime-extended temporaries or any const-promotable expressions. This is an experiment to see the impact of just removing this optimazation. This has been tried before with [slightly negative results](rust-lang#106770 (comment)), but maybe things have changed by now. :)
2 parents 10a76d6 + e039260 commit 19f2dc7

File tree

12 files changed

+130
-200
lines changed

12 files changed

+130
-200
lines changed

‎compiler/rustc_ast_lowering/src/format.rs

-66
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
use core::ops::ControlFlow;
21
use std::borrow::Cow;
32

4-
use rustc_ast::visit::Visitor;
53
use rustc_ast::*;
64
use rustc_data_structures::fx::FxIndexMap;
75
use rustc_hir as hir;
@@ -476,17 +474,6 @@ fn expand_format_args<'hir>(
476474
return hir::ExprKind::Call(new, new_args);
477475
}
478476

479-
// If the args array contains exactly all the original arguments once,
480-
// in order, we can use a simple array instead of a `match` construction.
481-
// However, if there's a yield point in any argument except the first one,
482-
// we don't do this, because an Argument cannot be kept across yield points.
483-
//
484-
// This is an optimization, speeding up compilation about 1-2% in some cases.
485-
// See https://github.com/rust-lang/rust/pull/106770#issuecomment-1380790609
486-
let use_simple_array = argmap.len() == arguments.len()
487-
&& argmap.iter().enumerate().all(|(i, (&(j, _), _))| i == j)
488-
&& arguments.iter().skip(1).all(|arg| !may_contain_yield_point(&arg.expr));
489-
490477
let args = if arguments.is_empty() {
491478
// Generate:
492479
// &<core::fmt::Argument>::none()
@@ -510,31 +497,6 @@ fn expand_format_args<'hir>(
510497
));
511498
let none = ctx.expr_call(macsp, none_fn, &[]);
512499
ctx.expr(macsp, hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, none))
513-
} else if use_simple_array {
514-
// Generate:
515-
// &[
516-
// <core::fmt::Argument>::new_display(&arg0),
517-
// <core::fmt::Argument>::new_lower_hex(&arg1),
518-
// <core::fmt::Argument>::new_debug(&arg2),
519-
// …
520-
// ]
521-
let elements = ctx.arena.alloc_from_iter(arguments.iter().zip(argmap).map(
522-
|(arg, ((_, ty), placeholder_span))| {
523-
let placeholder_span =
524-
placeholder_span.unwrap_or(arg.expr.span).with_ctxt(macsp.ctxt());
525-
let arg_span = match arg.kind {
526-
FormatArgumentKind::Captured(_) => placeholder_span,
527-
_ => arg.expr.span.with_ctxt(macsp.ctxt()),
528-
};
529-
let arg = ctx.lower_expr(&arg.expr);
530-
let ref_arg = ctx.arena.alloc(ctx.expr(
531-
arg_span,
532-
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, arg),
533-
));
534-
make_argument(ctx, placeholder_span, ref_arg, ty)
535-
},
536-
));
537-
ctx.expr_array_ref(macsp, elements)
538500
} else {
539501
// Generate:
540502
// &match (&arg0, &arg1, &…) {
@@ -635,34 +597,6 @@ fn expand_format_args<'hir>(
635597
}
636598
}
637599

638-
fn may_contain_yield_point(e: &ast::Expr) -> bool {
639-
struct MayContainYieldPoint;
640-
641-
impl Visitor<'_> for MayContainYieldPoint {
642-
type Result = ControlFlow<()>;
643-
644-
fn visit_expr(&mut self, e: &ast::Expr) -> ControlFlow<()> {
645-
if let ast::ExprKind::Await(_, _) | ast::ExprKind::Yield(_) = e.kind {
646-
ControlFlow::Break(())
647-
} else {
648-
visit::walk_expr(self, e)
649-
}
650-
}
651-
652-
fn visit_mac_call(&mut self, _: &ast::MacCall) -> ControlFlow<()> {
653-
// Macros should be expanded at this point.
654-
unreachable!("unexpanded macro in ast lowering");
655-
}
656-
657-
fn visit_item(&mut self, _: &ast::Item) -> ControlFlow<()> {
658-
// Do not recurse into nested items.
659-
ControlFlow::Continue(())
660-
}
661-
}
662-
663-
MayContainYieldPoint.visit_expr(e).is_break()
664-
}
665-
666600
fn for_all_argument_indexes(template: &mut [FormatArgsPiece], mut f: impl FnMut(&mut usize)) {
667601
for piece in template {
668602
let FormatArgsPiece::Placeholder(placeholder) = piece else { continue };

‎tests/mir-opt/sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff

+88-47
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,27 @@
1717
let mut _15: &[core::fmt::rt::Argument<'_>; 2];
1818
let _16: &[core::fmt::rt::Argument<'_>; 2];
1919
let _17: [core::fmt::rt::Argument<'_>; 2];
20-
let mut _18: core::fmt::rt::Argument<'_>;
20+
let mut _18: (&std::boxed::Box<dyn std::fmt::Display>, &u32);
2121
let mut _19: &std::boxed::Box<dyn std::fmt::Display>;
22-
let _20: &std::boxed::Box<dyn std::fmt::Display>;
23-
let mut _21: core::fmt::rt::Argument<'_>;
24-
let mut _22: &u32;
25-
let _23: &u32;
26-
let mut _25: bool;
27-
let mut _26: isize;
28-
let mut _27: isize;
29-
let mut _28: isize;
30-
+ let _29: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>;
31-
+ let _30: u32;
22+
let mut _20: &u32;
23+
let mut _22: core::fmt::rt::Argument<'_>;
24+
let mut _23: &std::boxed::Box<dyn std::fmt::Display>;
25+
let mut _24: core::fmt::rt::Argument<'_>;
26+
let mut _25: &u32;
27+
let mut _27: &std::boxed::Box<dyn std::fmt::Display>;
28+
let mut _28: &u32;
29+
let mut _29: bool;
30+
let mut _30: isize;
31+
let mut _31: isize;
32+
let mut _32: isize;
33+
+ let _33: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>;
34+
+ let _34: u32;
35+
+ let mut _37: &std::boxed::Box<dyn std::fmt::Display>;
36+
+ let mut _38: &u32;
3237
scope 1 {
3338
- debug foo => _1;
34-
+ debug ((foo: Foo<T>).0: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>) => _29;
35-
+ debug ((foo: Foo<T>).1: u32) => _30;
39+
+ debug ((foo: Foo<T>).0: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>) => _33;
40+
+ debug ((foo: Foo<T>).1: u32) => _34;
3641
let _5: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>;
3742
scope 2 {
3843
debug x => _5;
@@ -42,17 +47,25 @@
4247
scope 4 {
4348
debug x => _8;
4449
let _8: std::boxed::Box<dyn std::fmt::Display>;
45-
let mut _24: &[&str; 3];
50+
let _21: (&std::boxed::Box<dyn std::fmt::Display>, &u32);
51+
let mut _26: &[&str; 3];
52+
+ let _35: &std::boxed::Box<dyn std::fmt::Display>;
53+
+ let _36: &u32;
54+
scope 5 {
55+
- debug args => _21;
56+
+ debug ((args: (&Box<dyn std::fmt::Display>, &u32)).0: &std::boxed::Box<dyn std::fmt::Display>) => _35;
57+
+ debug ((args: (&Box<dyn std::fmt::Display>, &u32)).1: &u32) => _36;
58+
}
4659
}
4760
}
4861
}
4962
}
5063

5164
bb0: {
52-
_25 = const false;
65+
_29 = const false;
5366
- StorageLive(_1);
54-
+ StorageLive(_29);
55-
+ StorageLive(_30);
67+
+ StorageLive(_33);
68+
+ StorageLive(_34);
5669
+ nop;
5770
StorageLive(_2);
5871
StorageLive(_3);
@@ -66,59 +79,85 @@
6679
_2 = Result::<Box<dyn std::fmt::Display>, <T as Err>::Err>::Ok(move _3);
6780
StorageDead(_3);
6881
- _1 = Foo::<T> { x: move _2, y: const 7_u32 };
69-
+ _29 = move _2;
70-
+ _30 = const 7_u32;
82+
+ _33 = move _2;
83+
+ _34 = const 7_u32;
7184
+ nop;
7285
StorageDead(_2);
7386
StorageLive(_5);
74-
_25 = const true;
87+
_29 = const true;
7588
- _5 = move (_1.0: std::result::Result<std::boxed::Box<dyn std::fmt::Display>, <T as Err>::Err>);
76-
+ _5 = move _29;
89+
+ _5 = move _33;
7790
StorageLive(_6);
7891
- _6 = copy (_1.1: u32);
79-
+ _6 = copy _30;
92+
+ _6 = copy _34;
8093
_7 = discriminant(_5);
8194
switchInt(move _7) -> [0: bb2, otherwise: bb7];
8295
}
8396

8497
bb2: {
8598
StorageLive(_8);
86-
_25 = const false;
99+
_29 = const false;
87100
_8 = move ((_5 as Ok).0: std::boxed::Box<dyn std::fmt::Display>);
88101
StorageLive(_9);
89102
StorageLive(_10);
90103
StorageLive(_11);
91104
StorageLive(_12);
92105
StorageLive(_13);
93-
_24 = const foo::<T>::promoted[0];
94-
_13 = &(*_24);
106+
_26 = const foo::<T>::promoted[0];
107+
_13 = &(*_26);
95108
_12 = &(*_13);
96109
StorageLive(_15);
97110
StorageLive(_16);
98111
StorageLive(_17);
99-
StorageLive(_18);
112+
- StorageLive(_18);
113+
+ StorageLive(_37);
114+
+ StorageLive(_38);
115+
+ nop;
100116
StorageLive(_19);
117+
_19 = &_8;
101118
StorageLive(_20);
102-
_20 = &_8;
103-
_19 = &(*_20);
104-
_18 = core::fmt::rt::Argument::<'_>::new_display::<Box<dyn std::fmt::Display>>(move _19) -> [return: bb3, unwind unreachable];
105-
}
106-
107-
bb3: {
119+
_20 = &_6;
120+
- _18 = (move _19, move _20);
121+
+ _37 = move _19;
122+
+ _38 = move _20;
123+
+ nop;
124+
StorageDead(_20);
108125
StorageDead(_19);
109-
StorageLive(_21);
126+
- StorageLive(_21);
127+
- _21 = copy _18;
128+
+ StorageLive(_35);
129+
+ StorageLive(_36);
130+
+ nop;
131+
+ _35 = copy _37;
132+
+ _36 = copy _38;
133+
+ nop;
110134
StorageLive(_22);
111135
StorageLive(_23);
112-
_23 = &_6;
113-
_22 = &(*_23);
114-
_21 = core::fmt::rt::Argument::<'_>::new_display::<u32>(move _22) -> [return: bb4, unwind unreachable];
136+
- _27 = deref_copy (_21.0: &std::boxed::Box<dyn std::fmt::Display>);
137+
+ _27 = deref_copy _35;
138+
_23 = &(*_27);
139+
_22 = core::fmt::rt::Argument::<'_>::new_display::<Box<dyn std::fmt::Display>>(move _23) -> [return: bb3, unwind unreachable];
140+
}
141+
142+
bb3: {
143+
StorageDead(_23);
144+
StorageLive(_24);
145+
StorageLive(_25);
146+
- _28 = deref_copy (_21.1: &u32);
147+
+ _28 = deref_copy _36;
148+
_25 = &(*_28);
149+
_24 = core::fmt::rt::Argument::<'_>::new_display::<u32>(move _25) -> [return: bb4, unwind unreachable];
115150
}
116151

117152
bb4: {
153+
StorageDead(_25);
154+
_17 = [move _22, move _24];
155+
StorageDead(_24);
118156
StorageDead(_22);
119-
_17 = [move _18, move _21];
120-
StorageDead(_21);
121-
StorageDead(_18);
157+
- StorageDead(_21);
158+
+ StorageDead(_35);
159+
+ StorageDead(_36);
160+
+ nop;
122161
_16 = &_17;
123162
_15 = &(*_16);
124163
_11 = Arguments::<'_>::new_v1::<3, 2>(move _12, move _15) -> [return: bb5, unwind unreachable];
@@ -132,8 +171,10 @@
132171

133172
bb6: {
134173
StorageDead(_11);
135-
StorageDead(_23);
136-
StorageDead(_20);
174+
- StorageDead(_18);
175+
+ StorageDead(_37);
176+
+ StorageDead(_38);
177+
+ nop;
137178
StorageDead(_17);
138179
StorageDead(_16);
139180
StorageDead(_13);
@@ -156,22 +197,22 @@
156197

157198
bb9: {
158199
StorageDead(_6);
159-
_26 = discriminant(_5);
160-
switchInt(move _26) -> [0: bb11, otherwise: bb13];
200+
_30 = discriminant(_5);
201+
switchInt(move _30) -> [0: bb11, otherwise: bb13];
161202
}
162203

163204
bb10: {
164-
_25 = const false;
205+
_29 = const false;
165206
StorageDead(_5);
166207
- StorageDead(_1);
167-
+ StorageDead(_29);
168-
+ StorageDead(_30);
208+
+ StorageDead(_33);
209+
+ StorageDead(_34);
169210
+ nop;
170211
return;
171212
}
172213

173214
bb11: {
174-
switchInt(copy _25) -> [0: bb10, otherwise: bb12];
215+
switchInt(copy _29) -> [0: bb10, otherwise: bb12];
175216
}
176217

177218
bb12: {

‎tests/ui/borrowck/clone-on-ref.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ LL | drop(x);
3030
| ^ move out of `x` occurs here
3131
LL |
3232
LL | println!("{b}");
33-
| --- borrow later used here
33+
| - borrow later used here
3434
|
3535
help: if `T` implemented `Clone`, you could clone the value
3636
--> $DIR/clone-on-ref.rs:11:8
@@ -57,7 +57,7 @@ LL | drop(x);
5757
| ^ move out of `x` occurs here
5858
LL |
5959
LL | println!("{b:?}");
60-
| ----- borrow later used here
60+
| - borrow later used here
6161
|
6262
note: if `A` implemented `Clone`, you could clone the value
6363
--> $DIR/clone-on-ref.rs:19:1

‎tests/ui/codegen/equal-pointers-unequal/as-cast/print3.rs

-23
This file was deleted.

‎tests/ui/codegen/equal-pointers-unequal/exposed-provenance/print3.rs

-25
This file was deleted.

0 commit comments

Comments
 (0)