Skip to content

Rollup of 9 pull requests #123645

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

Merged
merged 23 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
3674032
Rework the ByMoveBody shim to actually work correctly
compiler-errors Apr 5, 2024
0f13bd4
Add some helpful comments
compiler-errors Apr 5, 2024
49c4ebc
Check the base of the place too!
compiler-errors Apr 5, 2024
ad0fcac
Account for an additional reborrow inserted by UniqueImmBorrow and Mu…
compiler-errors Apr 5, 2024
e0af5ea
bootstrap: remove unused pub fns
klensy Apr 6, 2024
00bd247
Don't emit divide-by-zero panic paths in `StepBy::len`
scottmcm Apr 6, 2024
54f8db8
add non-regression test for issue 123275
lqd Apr 6, 2024
68b4257
Revert "remove `pred_known_to_hold_modulo_regions`"
lqd Apr 6, 2024
e38dfc4
Remove unnecessary cast from `LLVMRustGetInstrProfIncrementIntrinsic`
Zalathar Apr 7, 2024
009280c
Fix argument ABI for overaligned structs on ppc64le
nikic Mar 20, 2024
1b7342b
force_array -> is_consecutive
nikic Mar 21, 2024
3a0d8d8
parser: reduce visibility of unnecessary public `UnmatchedDelim`
ohno418 Apr 7, 2024
3aa14e3
Compute transmutability from `rustc_target::abi::Layout`
jswrenn Mar 19, 2024
284da5d
CFI: Fix ICE in KCFI non-associated function pointers
maurer Apr 8, 2024
ecfc338
Rollup merge of #122781 - nikic:ppc-abi-fix, r=cuviper
matthiaskrgr Apr 8, 2024
0e27c99
Rollup merge of #123367 - jswrenn:layoutify, r=compiler-errors
matthiaskrgr Apr 8, 2024
36d1915
Rollup merge of #123518 - compiler-errors:by-move-fixes, r=oli-obk
matthiaskrgr Apr 8, 2024
5627497
Rollup merge of #123547 - klensy:bs-pubs, r=onur-ozkan
matthiaskrgr Apr 8, 2024
9570ac4
Rollup merge of #123564 - scottmcm:step-by-div-zero, r=joboet
matthiaskrgr Apr 8, 2024
984767e
Rollup merge of #123578 - lqd:regression-123275, r=compiler-errors
matthiaskrgr Apr 8, 2024
a38911a
Rollup merge of #123591 - Zalathar:useless-cast, r=cuviper
matthiaskrgr Apr 8, 2024
c0ac5d8
Rollup merge of #123632 - ohno418:fix-UnmatchedDelim-visibility, r=co…
matthiaskrgr Apr 8, 2024
0520200
Rollup merge of #123635 - maurer:kcfi-no-assoc, r=compiler-errors
matthiaskrgr Apr 8, 2024
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
5 changes: 4 additions & 1 deletion compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ impl LlvmType for CastTarget {
// Simplify to a single unit or an array if there's no prefix.
// This produces the same layout, but using a simpler type.
if self.prefix.iter().all(|x| x.is_none()) {
if rest_count == 1 {
// We can't do this if is_consecutive is set and the unit would get
// split on the target. Currently, this is only relevant for i128
// registers.
if rest_count == 1 && (!self.rest.is_consecutive || self.rest.unit != Reg::i128()) {
return rest_ll_unit;
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1524,8 +1524,8 @@ extern "C" LLVMValueRef LLVMRustBuildCall(LLVMBuilderRef B, LLVMTypeRef Ty, LLVM
}

extern "C" LLVMValueRef LLVMRustGetInstrProfIncrementIntrinsic(LLVMModuleRef M) {
return wrap(llvm::Intrinsic::getDeclaration(unwrap(M),
(llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_increment));
return wrap(llvm::Intrinsic::getDeclaration(
unwrap(M), llvm::Intrinsic::instrprof_increment));
}

extern "C" LLVMValueRef LLVMRustBuildMemCpy(LLVMBuilderRef B,
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,10 @@ impl<'tcx> Instance<'tcx> {
// Reify `Trait::method` implementations if KCFI is enabled
// FIXME(maurer) only reify it if it is a vtable-safe function
_ if tcx.sess.is_sanitizer_kcfi_enabled()
&& tcx.associated_item(def_id).trait_item_def_id.is_some() =>
&& tcx
.opt_associated_item(def_id)
.and_then(|assoc| assoc.trait_item_def_id)
.is_some() =>
{
// If this function could also go in a vtable, we need to `ReifyShim` it with
// KCFI because it can only attach one type per function.
Expand Down
197 changes: 162 additions & 35 deletions compiler/rustc_mir_transform/src/coroutine/by_move_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,24 @@
//! borrowing from the outer closure, and we simply peel off a `deref` projection
//! from them. This second body is stored alongside the first body, and optimized
//! with it in lockstep. When we need to resolve a body for `FnOnce` or `AsyncFnOnce`,
//! we use this "by move" body instead.

use itertools::Itertools;
//! we use this "by-move" body instead.
//!
//! ## How does this work?
//!
//! This pass essentially remaps the body of the (child) closure of the coroutine-closure
//! to take the set of upvars of the parent closure by value. This at least requires
//! changing a by-ref upvar to be by-value in the case that the outer coroutine-closure
//! captures something by value; however, it may also require renumbering field indices
//! in case precise captures (edition 2021 closure capture rules) caused the inner coroutine
//! to split one field capture into two.

use rustc_data_structures::unord::UnordSet;
use rustc_data_structures::unord::UnordMap;
use rustc_hir as hir;
use rustc_middle::hir::place::{PlaceBase, Projection, ProjectionKind};
use rustc_middle::mir::visit::MutVisitor;
use rustc_middle::mir::{self, dump_mir, MirPass};
use rustc_middle::ty::{self, InstanceDef, Ty, TyCtxt, TypeVisitableExt};
use rustc_target::abi::FieldIdx;
use rustc_target::abi::{FieldIdx, VariantIdx};

pub struct ByMoveBody;

Expand Down Expand Up @@ -116,32 +124,116 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody {
.tuple_fields()
.len();

let mut by_ref_fields = UnordSet::default();
for (idx, (coroutine_capture, parent_capture)) in tcx
let mut field_remapping = UnordMap::default();

// One parent capture may correspond to several child captures if we end up
// refining the set of captures via edition-2021 precise captures. We want to
// match up any number of child captures with one parent capture, so we keep
// peeking off this `Peekable` until the child doesn't match anymore.
let mut parent_captures =
tcx.closure_captures(parent_def_id).iter().copied().enumerate().peekable();
// Make sure we use every field at least once, b/c why are we capturing something
// if it's not used in the inner coroutine.
let mut field_used_at_least_once = false;

for (child_field_idx, child_capture) in tcx
.closure_captures(coroutine_def_id)
.iter()
.copied()
// By construction we capture all the args first.
.skip(num_args)
.zip_eq(tcx.closure_captures(parent_def_id))
.enumerate()
{
// This upvar is captured by-move from the parent closure, but by-ref
// from the inner async block. That means that it's being borrowed from
// the outer closure body -- we need to change the coroutine to take the
// upvar by value.
if coroutine_capture.is_by_ref() && !parent_capture.is_by_ref() {
assert_ne!(
coroutine_kind,
ty::ClosureKind::FnOnce,
"`FnOnce` coroutine-closures return coroutines that capture from \
their body; it will always result in a borrowck error!"
loop {
let Some(&(parent_field_idx, parent_capture)) = parent_captures.peek() else {
bug!("we ran out of parent captures!")
};

let PlaceBase::Upvar(parent_base) = parent_capture.place.base else {
bug!("expected capture to be an upvar");
};
let PlaceBase::Upvar(child_base) = child_capture.place.base else {
bug!("expected capture to be an upvar");
};

assert!(
child_capture.place.projections.len() >= parent_capture.place.projections.len()
);
by_ref_fields.insert(FieldIdx::from_usize(num_args + idx));
// A parent matches a child they share the same prefix of projections.
// The child may have more, if it is capturing sub-fields out of
// something that is captured by-move in the parent closure.
if parent_base.var_path.hir_id != child_base.var_path.hir_id
|| !std::iter::zip(
&child_capture.place.projections,
&parent_capture.place.projections,
)
.all(|(child, parent)| child.kind == parent.kind)
{
// Make sure the field was used at least once.
assert!(
field_used_at_least_once,
"we captured {parent_capture:#?} but it was not used in the child coroutine?"
);
field_used_at_least_once = false;
// Skip this field.
let _ = parent_captures.next().unwrap();
continue;
}

// Store this set of additional projections (fields and derefs).
// We need to re-apply them later.
let child_precise_captures =
&child_capture.place.projections[parent_capture.place.projections.len()..];

// If the parent captures by-move, and the child captures by-ref, then we
// need to peel an additional `deref` off of the body of the child.
let needs_deref = child_capture.is_by_ref() && !parent_capture.is_by_ref();
if needs_deref {
assert_ne!(
coroutine_kind,
ty::ClosureKind::FnOnce,
"`FnOnce` coroutine-closures return coroutines that capture from \
their body; it will always result in a borrowck error!"
);
}

// Finally, store the type of the parent's captured place. We need
// this when building the field projection in the MIR body later on.
let mut parent_capture_ty = parent_capture.place.ty();
parent_capture_ty = match parent_capture.info.capture_kind {
ty::UpvarCapture::ByValue => parent_capture_ty,
ty::UpvarCapture::ByRef(kind) => Ty::new_ref(
tcx,
tcx.lifetimes.re_erased,
parent_capture_ty,
kind.to_mutbl_lossy(),
),
};

field_remapping.insert(
FieldIdx::from_usize(child_field_idx + num_args),
(
FieldIdx::from_usize(parent_field_idx + num_args),
parent_capture_ty,
needs_deref,
child_precise_captures,
),
);

field_used_at_least_once = true;
break;
}
}

// Pop the last parent capture
if field_used_at_least_once {
let _ = parent_captures.next().unwrap();
}
assert_eq!(parent_captures.next(), None, "leftover parent captures?");

// Make sure we're actually talking about the same capture.
// FIXME(async_closures): We could look at the `hir::Upvar` instead?
assert_eq!(coroutine_capture.place.ty(), parent_capture.place.ty());
if coroutine_kind == ty::ClosureKind::FnOnce {
assert_eq!(field_remapping.len(), tcx.closure_captures(parent_def_id).len());
return;
}

let by_move_coroutine_ty = tcx
Expand All @@ -157,7 +249,7 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody {
);

let mut by_move_body = body.clone();
MakeByMoveBody { tcx, by_ref_fields, by_move_coroutine_ty }.visit_body(&mut by_move_body);
MakeByMoveBody { tcx, field_remapping, by_move_coroutine_ty }.visit_body(&mut by_move_body);
dump_mir(tcx, false, "coroutine_by_move", &0, &by_move_body, |_, _| Ok(()));
by_move_body.source = mir::MirSource::from_instance(InstanceDef::CoroutineKindShim {
coroutine_def_id: coroutine_def_id.to_def_id(),
Expand All @@ -168,7 +260,7 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody {

struct MakeByMoveBody<'tcx> {
tcx: TyCtxt<'tcx>,
by_ref_fields: UnordSet<FieldIdx>,
field_remapping: UnordMap<FieldIdx, (FieldIdx, Ty<'tcx>, bool, &'tcx [Projection<'tcx>])>,
by_move_coroutine_ty: Ty<'tcx>,
}

Expand All @@ -183,24 +275,59 @@ impl<'tcx> MutVisitor<'tcx> for MakeByMoveBody<'tcx> {
context: mir::visit::PlaceContext,
location: mir::Location,
) {
// Initializing an upvar local always starts with `CAPTURE_STRUCT_LOCAL` and a
// field projection. If this is in `field_remapping`, then it must not be an
// arg from calling the closure, but instead an upvar.
if place.local == ty::CAPTURE_STRUCT_LOCAL
&& let Some((&mir::ProjectionElem::Field(idx, ty), projection)) =
&& let Some((&mir::ProjectionElem::Field(idx, _), projection)) =
place.projection.split_first()
&& self.by_ref_fields.contains(&idx)
&& let Some(&(remapped_idx, remapped_ty, needs_deref, additional_projections)) =
self.field_remapping.get(&idx)
{
let (begin, end) = projection.split_first().unwrap();
// FIXME(async_closures): I'm actually a bit surprised to see that we always
// initially deref the by-ref upvars. If this is not actually true, then we
// will at least get an ICE that explains why this isn't true :^)
assert_eq!(*begin, mir::ProjectionElem::Deref);
// Peel one ref off of the ty.
let peeled_ty = ty.builtin_deref(true).unwrap().ty;
// As noted before, if the parent closure captures a field by value, and
// the child captures a field by ref, then for the by-move body we're
// generating, we also are taking that field by value. Peel off a deref,
// since a layer of reffing has now become redundant.
let final_deref = if needs_deref {
let Some((mir::ProjectionElem::Deref, projection)) = projection.split_first()
else {
bug!(
"There should be at least a single deref for an upvar local initialization, found {projection:#?}"
);
};
// There may be more derefs, since we may also implicitly reborrow
// a captured mut pointer.
projection
} else {
projection
};

// The only thing that should be left is a deref, if the parent captured
// an upvar by-ref.
std::assert_matches::assert_matches!(final_deref, [] | [mir::ProjectionElem::Deref]);

// For all of the additional projections that come out of precise capturing,
// re-apply these projections.
let additional_projections =
additional_projections.iter().map(|elem| match elem.kind {
ProjectionKind::Deref => mir::ProjectionElem::Deref,
ProjectionKind::Field(idx, VariantIdx::ZERO) => {
mir::ProjectionElem::Field(idx, elem.ty)
}
_ => unreachable!("precise captures only through fields and derefs"),
});

// We start out with an adjusted field index (and ty), representing the
// upvar that we get from our parent closure. We apply any of the additional
// projections to make sure that to the rest of the body of the closure, the
// place looks the same, and then apply that final deref if necessary.
*place = mir::Place {
local: place.local,
projection: self.tcx.mk_place_elems_from_iter(
[mir::ProjectionElem::Field(idx, peeled_ty)]
[mir::ProjectionElem::Field(remapped_idx, remapped_ty)]
.into_iter()
.chain(end.iter().copied()),
.chain(additional_projections)
.chain(final_deref.iter().copied()),
),
};
}
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_parse/src/lexer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ use unescape_error_reporting::{emit_unescape_error, escaped_char};
rustc_data_structures::static_assert_size!(rustc_lexer::Token, 12);

#[derive(Clone, Debug)]
pub struct UnmatchedDelim {
pub expected_delim: Delimiter,
pub(crate) struct UnmatchedDelim {
pub found_delim: Option<Delimiter>,
pub found_span: Span,
pub unclosed_span: Option<Span>,
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_parse/src/lexer/tokentrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ impl<'psess, 'src> TokenTreesReader<'psess, 'src> {
for &(_, sp) in &self.diag_info.open_braces {
err.span_label(sp, "unclosed delimiter");
self.diag_info.unmatched_delims.push(UnmatchedDelim {
expected_delim: Delimiter::Brace,
found_delim: None,
found_span: self.token.span,
unclosed_span: Some(sp),
Expand Down Expand Up @@ -163,9 +162,8 @@ impl<'psess, 'src> TokenTreesReader<'psess, 'src> {
candidate = Some(*brace_span);
}
}
let (tok, _) = self.diag_info.open_braces.pop().unwrap();
let (_, _) = self.diag_info.open_braces.pop().unwrap();
self.diag_info.unmatched_delims.push(UnmatchedDelim {
expected_delim: tok,
found_delim: Some(close_delim),
found_span: self.token.span,
unclosed_span: unclosed_delimiter,
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_target/src/abi/call/aarch64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ where
RegKind::Vector => size.bits() == 64 || size.bits() == 128,
};

valid_unit.then_some(Uniform { unit, total: size })
valid_unit.then_some(Uniform::consecutive(unit, size))
})
}

Expand Down Expand Up @@ -60,7 +60,7 @@ where
let size = ret.layout.size;
let bits = size.bits();
if bits <= 128 {
ret.cast_to(Uniform { unit: Reg::i64(), total: size });
ret.cast_to(Uniform::new(Reg::i64(), size));
return;
}
ret.make_indirect();
Expand Down Expand Up @@ -100,9 +100,9 @@ where
};
if size.bits() <= 128 {
if align.bits() == 128 {
arg.cast_to(Uniform { unit: Reg::i128(), total: size });
arg.cast_to(Uniform::new(Reg::i128(), size));
} else {
arg.cast_to(Uniform { unit: Reg::i64(), total: size });
arg.cast_to(Uniform::new(Reg::i64(), size));
}
return;
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_target/src/abi/call/arm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ where
RegKind::Vector => size.bits() == 64 || size.bits() == 128,
};

valid_unit.then_some(Uniform { unit, total: size })
valid_unit.then_some(Uniform::consecutive(unit, size))
})
}

Expand Down Expand Up @@ -49,7 +49,7 @@ where
let size = ret.layout.size;
let bits = size.bits();
if bits <= 32 {
ret.cast_to(Uniform { unit: Reg::i32(), total: size });
ret.cast_to(Uniform::new(Reg::i32(), size));
return;
}
ret.make_indirect();
Expand Down Expand Up @@ -78,7 +78,7 @@ where

let align = arg.layout.align.abi.bytes();
let total = arg.layout.size;
arg.cast_to(Uniform { unit: if align <= 4 { Reg::i32() } else { Reg::i64() }, total });
arg.cast_to(Uniform::consecutive(if align <= 4 { Reg::i32() } else { Reg::i64() }, total));
}

pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/abi/call/csky.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn classify_ret<Ty>(arg: &mut ArgAbi<'_, Ty>) {
if total.bits() > 64 {
arg.make_indirect();
} else if total.bits() > 32 {
arg.cast_to(Uniform { unit: Reg::i32(), total });
arg.cast_to(Uniform::new(Reg::i32(), total));
} else {
arg.cast_to(Reg::i32());
}
Expand All @@ -38,7 +38,7 @@ fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {
if arg.layout.is_aggregate() {
let total = arg.layout.size;
if total.bits() > 32 {
arg.cast_to(Uniform { unit: Reg::i32(), total });
arg.cast_to(Uniform::new(Reg::i32(), total));
} else {
arg.cast_to(Reg::i32());
}
Expand Down
Loading
Loading