Skip to content

Fix tail calling intrinsics #144815

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

Closed
wants to merge 2 commits into from
Closed
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
8 changes: 8 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

return if let Some(target) = target {
helper.funclet_br(self, bx, target, mergeable_succ)
} else if kind == CallKind::Tail {
let res = bx.load(
bx.backend_type(result.layout),
result.val.llval,
result.val.align,
);
bx.ret(res);
MergingSucc::False
} else {
bx.unreachable();
MergingSucc::False
Expand Down
101 changes: 44 additions & 57 deletions compiler/rustc_mir_transform/src/lower_intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,54 +14,59 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
let local_decls = &body.local_decls;
for block in body.basic_blocks.as_mut() {
let terminator = block.terminator.as_mut().unwrap();
if let TerminatorKind::Call { func, args, destination, target, .. } =
&mut terminator.kind
&& let ty::FnDef(def_id, generic_args) = *func.ty(local_decls, tcx).kind()

let (func, args, destination, cont) = match &mut terminator.kind {
TerminatorKind::Call { func, args, destination, target, .. } => {
(func, args, *destination, target.map(|target| TerminatorKind::Goto { target }))
}
TerminatorKind::TailCall { func, args, fn_span: _ } => {
(func, args, Place::return_place(), Some(TerminatorKind::Return))
}
_ => continue,
};

if let ty::FnDef(def_id, generic_args) = *func.ty(local_decls, tcx).kind()
&& let Some(intrinsic) = tcx.intrinsic(def_id)
{
match intrinsic.name {
sym::unreachable => {
terminator.kind = TerminatorKind::Unreachable;
}
sym::ub_checks => {
let target = target.unwrap();
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::NullaryOp(NullOp::UbChecks, tcx.types.bool),
))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::contract_checks => {
let target = target.unwrap();
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::NullaryOp(NullOp::ContractChecks, tcx.types.bool),
))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::forget => {
let target = target.unwrap();
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::Use(Operand::Constant(Box::new(ConstOperand {
span: terminator.source_info.span,
user_ty: None,
const_: Const::zero_sized(tcx.types.unit),
}))),
))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::copy_nonoverlapping => {
let target = target.unwrap();
let Ok([src, dst, count]) = take_array(args) else {
bug!("Wrong arguments for copy_non_overlapping intrinsic");
};
Expand All @@ -77,10 +82,9 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
),
)),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::assume => {
let target = target.unwrap();
let Ok([arg]) = take_array(args) else {
bug!("Wrong arguments for assume intrinsic");
};
Expand All @@ -90,7 +94,7 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
arg.node,
))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::wrapping_add
| sym::wrapping_sub
Expand All @@ -103,7 +107,6 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
| sym::unchecked_rem
| sym::unchecked_shl
| sym::unchecked_shr => {
let target = target.unwrap();
let Ok([lhs, rhs]) = take_array(args) else {
bug!("Wrong arguments for {} intrinsic", intrinsic.name);
};
Expand All @@ -124,14 +127,13 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::BinaryOp(bin_op, Box::new((lhs.node, rhs.node))),
))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::add_with_overflow | sym::sub_with_overflow | sym::mul_with_overflow => {
let target = target.unwrap();
let Ok([lhs, rhs]) = take_array(args) else {
bug!("Wrong arguments for {} intrinsic", intrinsic.name);
};
Expand All @@ -144,14 +146,13 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::BinaryOp(bin_op, Box::new((lhs.node, rhs.node))),
))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::size_of | sym::align_of => {
let target = target.unwrap();
let tp_ty = generic_args.type_at(0);
let null_op = match intrinsic.name {
sym::size_of => NullOp::SizeOf,
Expand All @@ -161,11 +162,11 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::NullaryOp(null_op, tp_ty),
))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I opened the code change thinking "well, if this is small then whatever we can just do it", but needing all these conts is kinda annoying, and I definitely don't think I'd (personally) be excited to write double the tests to make sure that the call and the become both work the next time I add something to this fine.


How many intrinsics exist in the wild that are allowed to be called from stable? Could we do something simpler just for those? Then we could just do an rust-lang/compiler-team#620 ICE for TailCall'ing any other intrinsic.

Copy link
Member Author

Choose a reason for hiding this comment

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

How many intrinsics exist in the wild that are allowed to be called from stable?

From searching for #[stable in https://doc.rust-lang.org/stable/src/core/intrinsics/mod.rs.html#3830, it appears it's only transmute (there are a few other stable functions, but they are wrapper functions, not intrinsics directly)

Gives this and #144815 (comment) I'm inclined to say it's fine to forbid tail-calling intrinsics. I think I'll change this PR to make it an error to tail call an intrinsic.

}
sym::read_via_copy => {
let Ok([arg]) = take_array(args) else {
Expand All @@ -186,21 +187,16 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::Use(Operand::Copy(derefed_place)),
))),
));
terminator.kind = match *target {
None => {
// No target means this read something uninhabited,
// so it must be unreachable.
TerminatorKind::Unreachable
}
Some(target) => TerminatorKind::Goto { target },
}

// No target means this read something uninhabited,
// so it must be unreachable.
terminator.kind = cont.unwrap_or(TerminatorKind::Unreachable);
}
sym::write_via_move => {
let target = target.unwrap();
let Ok([ptr, val]) = take_array(args) else {
span_bug!(
terminator.source_info.span,
Expand All @@ -221,10 +217,9 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
terminator.source_info,
StatementKind::Assign(Box::new((derefed_place, Rvalue::Use(val.node)))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::discriminant_value => {
let target = target.unwrap();
let Ok([arg]) = take_array(args) else {
span_bug!(
terminator.source_info.span,
Expand All @@ -236,14 +231,13 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::Discriminant(arg),
))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::offset => {
let target = target.unwrap();
let Ok([ptr, delta]) = take_array(args) else {
span_bug!(
terminator.source_info.span,
Expand All @@ -253,14 +247,13 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::BinaryOp(BinOp::Offset, Box::new((ptr.node, delta.node))),
))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::slice_get_unchecked => {
let target = target.unwrap();
let Ok([ptrish, index]) = take_array(args) else {
span_bug!(
terminator.source_info.span,
Expand Down Expand Up @@ -301,9 +294,9 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {

block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((*destination, rvalue))),
StatementKind::Assign(Box::new((destination, rvalue))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::transmute | sym::transmute_unchecked => {
let dst_ty = destination.ty(local_decls, tcx).ty;
Expand All @@ -320,15 +313,11 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::Cast(CastKind::Transmute, arg.node, dst_ty),
))),
));
if let Some(target) = *target {
terminator.kind = TerminatorKind::Goto { target };
} else {
terminator.kind = TerminatorKind::Unreachable;
}
terminator.kind = cont.unwrap_or(TerminatorKind::Unreachable);
}
sym::aggregate_raw_ptr => {
let Ok([data, meta]) = take_array(args) else {
Expand All @@ -337,7 +326,6 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
"Wrong number of arguments for aggregate_raw_ptr intrinsic",
);
};
let target = target.unwrap();
let pointer_ty = generic_args.type_at(0);
let kind = if let ty::RawPtr(pointee_ty, mutability) = pointer_ty.kind() {
AggregateKind::RawPtr(*pointee_ty, *mutability)
Expand All @@ -351,11 +339,11 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::Aggregate(Box::new(kind), fields.into()),
))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
sym::ptr_metadata => {
let Ok([ptr]) = take_array(args) else {
Expand All @@ -364,15 +352,14 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
"Wrong number of arguments for ptr_metadata intrinsic",
);
};
let target = target.unwrap();
block.statements.push(Statement::new(
terminator.source_info,
StatementKind::Assign(Box::new((
*destination,
destination,
Rvalue::UnaryOp(UnOp::PtrMetadata, ptr.node),
))),
));
terminator.kind = TerminatorKind::Goto { target };
terminator.kind = cont.unwrap();
}
_ => {}
}
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/explicit-tail-calls/intrinsics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//@ run-pass
#![feature(explicit_tail_calls, core_intrinsics)]
#![expect(incomplete_features, internal_features)]

fn trans((): ()) {
// transmute is lowered in a mir pass
unsafe { become std::mem::transmute(()) }
}

fn cats(x: u64) -> u32 {
become std::intrinsics::ctlz(x)
}

fn main() {
trans(());
assert_eq!(cats(17), 59);
}

Loading