Skip to content

Save 2 pointers in TerminatorKind (96 → 80 bytes) #126784

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

Merged
merged 1 commit into from
Jun 24, 2024
+72 −72
Merged

Save 2 pointers in TerminatorKind (96 → 80 bytes) #126784

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
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/mir/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1817,11 +1817,11 @@ mod size_asserts {
use super::*;
use rustc_data_structures::static_assert_size;
// tidy-alphabetical-start
static_assert_size!(BasicBlockData<'_>, 144);
static_assert_size!(BasicBlockData<'_>, 128);
static_assert_size!(LocalDecl<'_>, 40);
static_assert_size!(SourceScopeData<'_>, 64);
static_assert_size!(Statement<'_>, 32);
static_assert_size!(Terminator<'_>, 112);
static_assert_size!(Terminator<'_>, 96);
static_assert_size!(VarDebugInfo<'_>, 88);
// tidy-alphabetical-end
}
8 changes: 4 additions & 4 deletions compiler/rustc_middle/src/mir/syntax.rs
View file
Original file line number Diff line number Diff line change
@@ -730,7 +730,7 @@ pub enum TerminatorKind<'tcx> {
/// reused across function calls without duplicating the contents.
/// The span for each arg is also included
/// (e.g. `a` and `b` in `x.foo(a, b)`).
args: Vec<Spanned<Operand<'tcx>>>,
args: Box<[Spanned<Operand<'tcx>>]>,
/// Where the returned value will be written
destination: Place<'tcx>,
/// Where to go after this call returns. If none, the call necessarily diverges.
@@ -837,7 +837,7 @@ pub enum TerminatorKind<'tcx> {
template: &'tcx [InlineAsmTemplatePiece],

/// The operands for the inline assembly, as `Operand`s or `Place`s.
operands: Vec<InlineAsmOperand<'tcx>>,
operands: Box<[InlineAsmOperand<'tcx>]>,

/// Miscellaneous options for the inline assembly.
options: InlineAsmOptions,
@@ -849,7 +849,7 @@ pub enum TerminatorKind<'tcx> {
/// Valid targets for the inline assembly.
/// The first element is the fallthrough destination, unless
/// InlineAsmOptions::NORETURN is set.
targets: Vec<BasicBlock>,
targets: Box<[BasicBlock]>,

/// Action to be taken if the inline assembly unwinds. This is present
/// if and only if InlineAsmOptions::MAY_UNWIND is set.
@@ -1561,6 +1561,6 @@ mod size_asserts {
static_assert_size!(PlaceElem<'_>, 24);
static_assert_size!(Rvalue<'_>, 40);
static_assert_size!(StatementKind<'_>, 16);
static_assert_size!(TerminatorKind<'_>, 96);
static_assert_size!(TerminatorKind<'_>, 80);
// tidy-alphabetical-end
}
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/custom/parse/instruction.rs
View file
Original file line number Diff line number Diff line change
@@ -170,7 +170,7 @@ impl<'tcx, 'body> ParseCtxt<'tcx, 'body> {
.map(|arg|
Ok(Spanned { node: self.parse_operand(*arg)?, span: self.thir.exprs[*arg].span } )
)
.collect::<PResult<Vec<_>>>()?;
.collect::<PResult<Box<[_]>>>()?;
Ok(TerminatorKind::Call {
func: fun,
args,
5 changes: 3 additions & 2 deletions compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
View file
Original file line number Diff line number Diff line change
@@ -152,10 +152,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
source_info,
TerminatorKind::Call {
func: exchange_malloc,
args: vec![
args: [
Spanned { node: Operand::Move(size), span: DUMMY_SP },
Spanned { node: Operand::Move(align), span: DUMMY_SP },
],
]
.into(),
destination: storage,
target: Some(success),
unwind: UnwindAction::Continue,
4 changes: 2 additions & 2 deletions compiler/rustc_mir_build/src/build/expr/into.rs
View file
Original file line number Diff line number Diff line change
@@ -238,7 +238,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
ExprKind::Call { ty: _, fun, ref args, from_hir_call, fn_span } => {
let fun = unpack!(block = this.as_local_operand(block, fun));
let args: Vec<_> = args
let args: Box<[_]> = args
.into_iter()
.copied()
.map(|arg| Spanned {
@@ -485,7 +485,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
operands,
options,
line_spans,
targets,
targets: targets.into_boxed_slice(),
unwind: if options.contains(InlineAsmOptions::MAY_UNWIND) {
UnwindAction::Continue
} else {
7 changes: 4 additions & 3 deletions compiler/rustc_mir_build/src/build/matches/test.rs
View file
Original file line number Diff line number Diff line change
@@ -324,7 +324,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
user_ty: None,
const_: method,
})),
args: vec![Spanned { node: Operand::Move(ref_src), span }],
args: [Spanned { node: Operand::Move(ref_src), span }].into(),
destination: temp,
target: Some(target_block),
unwind: UnwindAction::Continue,
@@ -486,10 +486,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

const_: method,
})),
args: vec![
args: [
Spanned { node: Operand::Copy(val), span: DUMMY_SP },
Spanned { node: expect, span: DUMMY_SP },
],
]
.into(),
destination: eq_result,
target: Some(eq_block),
unwind: UnwindAction::Continue,
6 changes: 2 additions & 4 deletions compiler/rustc_mir_dataflow/src/elaborate_drops.rs
View file
Original file line number Diff line number Diff line change
@@ -650,10 +650,8 @@ where
[ty.into()],
self.source_info.span,
),
args: vec![Spanned {
node: Operand::Move(Place::from(ref_place)),
span: DUMMY_SP,
}],
args: [Spanned { node: Operand::Move(Place::from(ref_place)), span: DUMMY_SP }]
.into(),
destination: unit_temp,
target: Some(succ),
unwind: unwind.into_action(),
4 changes: 2 additions & 2 deletions compiler/rustc_mir_dataflow/src/framework/tests.rs
View file
Original file line number Diff line number Diff line change
@@ -34,7 +34,7 @@ fn mock_body<'tcx>() -> mir::Body<'tcx> {
2,
mir::TerminatorKind::Call {
func: mir::Operand::Copy(dummy_place.clone()),
args: vec![],
args: [].into(),
destination: dummy_place.clone(),
target: Some(mir::START_BLOCK),
unwind: mir::UnwindAction::Continue,
@@ -48,7 +48,7 @@ fn mock_body<'tcx>() -> mir::Body<'tcx> {
4,
mir::TerminatorKind::Call {
func: mir::Operand::Copy(dummy_place.clone()),
args: vec![],
args: [].into(),
destination: dummy_place.clone(),
target: Some(mir::START_BLOCK),
unwind: mir::UnwindAction::Continue,
4 changes: 2 additions & 2 deletions compiler/rustc_mir_transform/src/coroutine.rs
View file
Original file line number Diff line number Diff line change
@@ -677,8 +677,8 @@ fn transform_async_context<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {

fn eliminate_get_context_call<'tcx>(bb_data: &mut BasicBlockData<'tcx>) -> Local {
let terminator = bb_data.terminator.take().unwrap();
if let TerminatorKind::Call { mut args, destination, target, .. } = terminator.kind {
let arg = args.pop().unwrap();
if let TerminatorKind::Call { args, destination, target, .. } = terminator.kind {
let [arg] = *Box::try_from(args).unwrap();
let local = arg.node.place().unwrap().local;

let arg = Rvalue::Use(arg.node);
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/coverage/tests.rs
View file
Original file line number Diff line number Diff line change
@@ -134,7 +134,7 @@ impl<'tcx> MockBlocks<'tcx> {
some_from_block,
TerminatorKind::Call {
func: Operand::Copy(self.dummy_place.clone()),
args: vec![],
args: [].into(),
Copy link
Member

Choose a reason for hiding this comment

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

Box<[T]> implements Default, might be easier to use that

Copy link
Member

Choose a reason for hiding this comment

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

or Box::new([]) and let CoerceUnsized do the work

Copy link
Member Author

Choose a reason for hiding this comment

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

If you don't mind, I'd like to just leave it .into() because that's what I'm using elsewhere in the places where it's not empty.

Copy link
Member

Choose a reason for hiding this comment

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

Well I'd rather you just use Box::new([x, y, z]) everywhere -- I think trailing .into() is just ugly to read and not as analogous to vec![x, y, z] 😆

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 liked it when it was

                        args: [
                            Spanned { node: Operand::Move(size), span: DUMMY_SP },
                            Spanned { node: Operand::Move(size), span: DUMMY_SP },
                            Spanned { node: Operand::Move(align), span: DUMMY_SP },
                            Spanned { node: Operand::Move(align), span: DUMMY_SP },
                        ].into(),

But of course rustfmt won't let us have nice things.

I'm inclined to leave it the one that doesn't say "vec" nor "box" and works with both, but I can change them all to Box::new if you feel strongly 🙃

Copy link
Contributor

Choose a reason for hiding this comment

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

Also note that this particular code is just building fake MIR for unit tests, so it's relatively unimportant.

Copy link
Member

Choose a reason for hiding this comment

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

Well this is one of many examples -- I wasn't going to leave a comment on all of them, so the fact it's building fake MIR or processing real MIR isn't really relevant

destination: self.dummy_place.clone(),
target: Some(TEMP_BLOCK),
unwind: UnwindAction::Continue,
13 changes: 7 additions & 6 deletions compiler/rustc_mir_transform/src/inline.rs
View file
Original file line number Diff line number Diff line change
@@ -624,8 +624,7 @@ impl<'tcx> Inliner<'tcx> {
};

// Copy the arguments if needed.
let args: Vec<_> =
self.make_call_args(args, &callsite, caller_body, &callee_body, return_block);
let args = self.make_call_args(args, &callsite, caller_body, &callee_body, return_block);

let mut integrator = Integrator {
args: &args,
@@ -736,12 +735,12 @@ impl<'tcx> Inliner<'tcx> {

fn make_call_args(
&self,
args: Vec<Spanned<Operand<'tcx>>>,
args: Box<[Spanned<Operand<'tcx>>]>,
callsite: &CallSite<'tcx>,
caller_body: &mut Body<'tcx>,
callee_body: &Body<'tcx>,
return_block: Option<BasicBlock>,
) -> Vec<Local> {
) -> Box<[Local]> {
let tcx = self.tcx;

// There is a bit of a mismatch between the *caller* of a closure and the *callee*.
@@ -768,7 +767,8 @@ impl<'tcx> Inliner<'tcx> {
//
// and the vector is `[closure_ref, tmp0, tmp1, tmp2]`.
if callsite.fn_sig.abi() == Abi::RustCall && callee_body.spread_arg.is_none() {
let mut args = args.into_iter();
// FIXME(edition_2024): switch back to a normal method call.
let mut args = <_>::into_iter(args);
let self_ = self.create_temp_if_necessary(
args.next().unwrap().node,
callsite,
@@ -802,7 +802,8 @@ impl<'tcx> Inliner<'tcx> {

closure_ref_arg.chain(tuple_tmp_args).collect()
} else {
args.into_iter()
// FIXME(edition_2024): switch back to a normal method call.
<_>::into_iter(args)
.map(|a| self.create_temp_if_necessary(a.node, callsite, caller_body, return_block))
.collect()
}
4 changes: 3 additions & 1 deletion compiler/rustc_mir_transform/src/instsimplify.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Performs various peephole optimizations.

use crate::simplify::simplify_duplicate_switch_targets;
use crate::take_array;
use rustc_ast::attr;
use rustc_middle::bug;
use rustc_middle::mir::*;
@@ -285,7 +286,8 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> {
return;
}

let Some(arg_place) = args.pop().unwrap().node.place() else { return };
let Ok([arg]) = take_array(args) else { return };
let Some(arg_place) = arg.node.place() else { return };

statements.push(Statement {
source_info: terminator.source_info,
10 changes: 8 additions & 2 deletions compiler/rustc_mir_transform/src/lib.rs
View file
Original file line number Diff line number Diff line change
@@ -160,8 +160,9 @@ fn remap_mir_for_const_eval_select<'tcx>(
} if let ty::FnDef(def_id, _) = *const_.ty().kind()
&& tcx.is_intrinsic(def_id, sym::const_eval_select) =>
{
let [tupled_args, called_in_const, called_at_rt]: [_; 3] =
std::mem::take(args).try_into().unwrap();
let Ok([tupled_args, called_in_const, called_at_rt]) = take_array(args) else {
unreachable!()
};
let ty = tupled_args.node.ty(&body.local_decls, tcx);
let fields = ty.tuple_fields();
let num_args = fields.len();
@@ -211,6 +212,11 @@ fn remap_mir_for_const_eval_select<'tcx>(
body
}

fn take_array<T, const N: usize>(b: &mut Box<[T]>) -> Result<[T; N], Box<[T]>> {
let b: Box<[T; N]> = std::mem::take(b).try_into()?;
Ok(*b)
}

fn is_mir_available(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
tcx.mir_keys(()).contains(&def_id)
}
61 changes: 23 additions & 38 deletions compiler/rustc_mir_transform/src/lower_intrinsics.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Lowers intrinsic calls

use crate::take_array;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::{bug, span_bug};
@@ -50,42 +51,34 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
}
sym::copy_nonoverlapping => {
let target = target.unwrap();
let mut args = args.drain(..);
let Ok([src, dst, count]) = take_array(args) else {
bug!("Wrong arguments for copy_non_overlapping intrinsic");
};
block.statements.push(Statement {
source_info: terminator.source_info,
kind: StatementKind::Intrinsic(Box::new(
NonDivergingIntrinsic::CopyNonOverlapping(
rustc_middle::mir::CopyNonOverlapping {
src: args.next().unwrap().node,
dst: args.next().unwrap().node,
count: args.next().unwrap().node,
src: src.node,
dst: dst.node,
count: count.node,
},
),
)),
});
assert_eq!(
args.next(),
None,
"Extra argument for copy_non_overlapping intrinsic"
);
drop(args);
terminator.kind = TerminatorKind::Goto { target };
}
sym::assume => {
let target = target.unwrap();
let mut args = args.drain(..);
let Ok([arg]) = take_array(args) else {
bug!("Wrong arguments for assume intrinsic");
};
block.statements.push(Statement {
source_info: terminator.source_info,
kind: StatementKind::Intrinsic(Box::new(
NonDivergingIntrinsic::Assume(args.next().unwrap().node),
NonDivergingIntrinsic::Assume(arg.node),
)),
});
assert_eq!(
args.next(),
None,
"Extra argument for copy_non_overlapping intrinsic"
);
drop(args);
terminator.kind = TerminatorKind::Goto { target };
}
sym::wrapping_add
@@ -100,13 +93,9 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
| sym::unchecked_shl
| sym::unchecked_shr => {
let target = target.unwrap();
let lhs;
let rhs;
{
let mut args = args.drain(..);
lhs = args.next().unwrap();
rhs = args.next().unwrap();
}
let Ok([lhs, rhs]) = take_array(args) else {
bug!("Wrong arguments for {} intrinsic", intrinsic.name);
};
let bin_op = match intrinsic.name {
sym::wrapping_add => BinOp::Add,
sym::wrapping_sub => BinOp::Sub,
@@ -132,13 +121,9 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
}
sym::add_with_overflow | sym::sub_with_overflow | sym::mul_with_overflow => {
if let Some(target) = *target {
let lhs;
let rhs;
{
let mut args = args.drain(..);
lhs = args.next().unwrap();
rhs = args.next().unwrap();
}
let Ok([lhs, rhs]) = take_array(args) else {
bug!("Wrong arguments for {} intrinsic", intrinsic.name);
};
let bin_op = match intrinsic.name {
sym::add_with_overflow => BinOp::AddWithOverflow,
sym::sub_with_overflow => BinOp::SubWithOverflow,
@@ -174,7 +159,7 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
}
}
sym::read_via_copy => {
let [arg] = args.as_slice() else {
let Ok([arg]) = take_array(args) else {
span_bug!(terminator.source_info.span, "Wrong number of arguments");
};
let derefed_place = if let Some(place) = arg.node.place()
@@ -207,7 +192,7 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
}
sym::write_via_move => {
let target = target.unwrap();
let Ok([ptr, val]) = <[_; 2]>::try_from(std::mem::take(args)) else {
let Ok([ptr, val]) = take_array(args) else {
span_bug!(
terminator.source_info.span,
"Wrong number of arguments for write_via_move intrinsic",
@@ -247,7 +232,7 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
}
sym::offset => {
let target = target.unwrap();
let Ok([ptr, delta]) = <[_; 2]>::try_from(std::mem::take(args)) else {
let Ok([ptr, delta]) = take_array(args) else {
span_bug!(
terminator.source_info.span,
"Wrong number of arguments for offset intrinsic",
@@ -264,7 +249,7 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
}
sym::transmute | sym::transmute_unchecked => {
let dst_ty = destination.ty(local_decls, tcx).ty;
let Ok([arg]) = <[_; 1]>::try_from(std::mem::take(args)) else {
let Ok([arg]) = take_array(args) else {
span_bug!(
terminator.source_info.span,
"Wrong number of arguments for transmute intrinsic",
@@ -289,7 +274,7 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
}
}
sym::aggregate_raw_ptr => {
let Ok([data, meta]) = <[_; 2]>::try_from(std::mem::take(args)) else {
let Ok([data, meta]) = take_array(args) else {
span_bug!(
terminator.source_info.span,
"Wrong number of arguments for aggregate_raw_ptr intrinsic",
@@ -317,7 +302,7 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
terminator.kind = TerminatorKind::Goto { target };
}
sym::ptr_metadata => {
let Ok([ptr]) = <[_; 1]>::try_from(std::mem::take(args)) else {
let Ok([ptr]) = take_array(args) else {
span_bug!(
terminator.source_info.span,
"Wrong number of arguments for ptr_metadata intrinsic",
Loading