From aff18fa9882c55559d36435175fea5879135eabb Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 08:57:04 +0000 Subject: [PATCH] Switch to changing cp_non_overlap in tform It was suggested to lower this in MIR instead of ssa, so do that instead. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 20 +------ .../rustc_codegen_ssa/src/mir/intrinsic.rs | 6 -- compiler/rustc_middle/src/mir/mod.rs | 2 +- compiler/rustc_mir/src/borrow_check/mod.rs | 6 ++ .../rustc_mir/src/interpret/intrinsics.rs | 12 ++-- compiler/rustc_mir/src/interpret/step.rs | 56 ++++++++++--------- .../rustc_mir/src/transform/check_unsafety.rs | 2 +- .../src/transform/lower_intrinsics.rs | 21 +++++++ .../src/transform/remove_noop_landing_pads.rs | 2 +- .../clippy_utils/src/qualify_min_const_fn.rs | 2 +- 10 files changed, 67 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 1150d4d734870..e148ed7ad3bce 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -643,23 +643,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { match intrinsic { None | Some(sym::drop_in_place) => {} - Some(sym::copy_nonoverlapping) => { - bx = self.codegen_statement( - bx, - &rustc_middle::mir::Statement { - source_info: rustc_middle::mir::SourceInfo::outermost(span), - kind: rustc_middle::mir::StatementKind::CopyNonOverlapping( - box rustc_middle::mir::CopyNonOverlapping { - src: args[0].clone(), - dst: args[1].clone(), - count: args[2].clone(), - }, - ), - }, - ); - helper.funclet_br(self, &mut bx, destination.unwrap().1); - return; - } + Some(sym::copy_nonoverlapping) => unreachable!(), Some(intrinsic) => { let dest = match ret_dest { _ if fn_abi.ret.is_indirect() => llargs[0], @@ -702,7 +686,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }) .collect(); - self.codegen_intrinsic_call( + Self::codegen_intrinsic_call( &mut bx, *instance.as_ref().unwrap(), &fn_abi, diff --git a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs index 00fc5b6606151..8502309b90e5a 100644 --- a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs +++ b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs @@ -49,7 +49,6 @@ fn memset_intrinsic<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { pub fn codegen_intrinsic_call( - &self, bx: &mut Bx, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, @@ -126,11 +125,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let offset = args[1].immediate(); bx.gep(ptr, &[offset]) } - - sym::copy_nonoverlapping => { - // handled explicitly in compiler/rustc_codegen_ssa/src/mir/block.rs - unreachable!(); - } sym::copy => { copy_intrinsic( bx, diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 20393ce46602c..ec8d7f8a55bbe 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1656,7 +1656,7 @@ pub struct Coverage { pub code_region: Option, } -#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)] +#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable)] pub struct CopyNonOverlapping<'tcx> { pub src: Operand<'tcx>, pub dst: Operand<'tcx>, diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index e7e936cb97e53..254ced49e2f3e 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -628,13 +628,19 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc } StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + .. + /* src, dst, count, + */ }) => { + unreachable!() + /* self.consume_operand(location, (src, span), flow_state); self.consume_operand(location, (dst, span), flow_state); self.consume_operand(location, (count, span), flow_state); + */ } StatementKind::Nop | StatementKind::Coverage(..) diff --git a/compiler/rustc_mir/src/interpret/intrinsics.rs b/compiler/rustc_mir/src/interpret/intrinsics.rs index d36b3a7d9b56e..b0a2d906519ac 100644 --- a/compiler/rustc_mir/src/interpret/intrinsics.rs +++ b/compiler/rustc_mir/src/interpret/intrinsics.rs @@ -323,7 +323,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let result = Scalar::from_uint(truncated_bits, layout.size); self.write_scalar(result, dest)?; } - sym::copy | sym::copy_nonoverlapping => { + sym::copy_nonoverlapping => { + self.copy_nonoverlapping(args[0], args[1], args[2])?; + } + sym::copy => { let elem_ty = instance.substs.type_at(0); let elem_layout = self.layout_of(elem_ty)?; let count = self.read_scalar(&args[2])?.to_machine_usize(self)?; @@ -338,12 +341,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let dest = self.memory.check_ptr_access(dest, size, elem_align)?; if let (Some(src), Some(dest)) = (src, dest) { - self.memory.copy( - src, - dest, - size, - intrinsic_name == sym::copy_nonoverlapping, - )?; + self.memory.copy(src, dest, size, false)?; } } sym::offset => { diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index eda9a4ac89026..64f6ec4832ec7 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -2,6 +2,7 @@ //! //! The main entry point is the `step` method. +use crate::interpret::OpTy; use rustc_middle::mir; use rustc_middle::mir::interpret::{InterpResult, Scalar}; use rustc_target::abi::LayoutOf; @@ -115,35 +116,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Call CopyNonOverlapping CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, count }) => { - let (src, size) = { - let src = self.eval_operand(src, None)?; - let size = src.layout.layout.size; - let mplace = *src.assert_mem_place(self); - let ptr = match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), - }; - (ptr, size) - }; - - let dst = { - let dst = self.eval_operand(dst, None)?; - let mplace = *dst.assert_mem_place(self); - match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), - } - }; - let count = self.eval_operand(count, None)?; - let count = self.read_immediate(count)?.to_scalar()?; - let count = if let Scalar::Int(i) = count { - core::convert::TryFrom::try_from(i).unwrap() - } else { - panic!(); - }; - self.memory.copy_repeatedly(src, dst, size, count, /*nonoverlapping*/ true)?; + let src = self.eval_operand(src, None)?; + let dst = self.eval_operand(dst, None)?; + self.copy_nonoverlapping(src, dst, count)?; } // Statements we do not track. @@ -173,6 +150,31 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } + pub(crate) fn copy_nonoverlapping( + &mut self, + src: OpTy<'tcx, >::PointerTag>, + dst: OpTy<'tcx, >::PointerTag>, + count: OpTy<'tcx, >::PointerTag>, + ) -> InterpResult<'tcx> { + let count = self.read_scalar(&count)?.to_machine_usize(self)?; + let layout = self.layout_of(src.layout.ty.builtin_deref(true).unwrap().ty)?; + let (size, align) = (layout.size, layout.align.abi); + let src = + self.memory.check_ptr_access(self.read_scalar(&src)?.check_init()?, size, align)?; + + let dst = + self.memory.check_ptr_access(self.read_scalar(&dst)?.check_init()?, size, align)?; + + let size = size.checked_mul(count, self).ok_or_else(|| { + err_ub_format!("overflow computing total size of `copy_nonoverlapping`") + })?; + + if let (Some(src), Some(dst)) = (src, dst) { + self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + } + Ok(()) + } + /// Evaluate an assignment statement. /// /// There is no separate `eval_rvalue` function. Instead, the code for handling each rvalue diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index e7fdd5496cb40..33848bc130581 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -115,7 +115,6 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) - | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // safe (at least as emitted during MIR construction) } @@ -124,6 +123,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { UnsafetyViolationKind::General, UnsafetyViolationDetails::UseOfInlineAssembly, ), + StatementKind::CopyNonOverlapping(..) => unreachable!(), } self.super_statement(statement, location); } diff --git a/compiler/rustc_mir/src/transform/lower_intrinsics.rs b/compiler/rustc_mir/src/transform/lower_intrinsics.rs index f5968532eb396..9749b31552d9a 100644 --- a/compiler/rustc_mir/src/transform/lower_intrinsics.rs +++ b/compiler/rustc_mir/src/transform/lower_intrinsics.rs @@ -40,6 +40,27 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics { terminator.kind = TerminatorKind::Goto { target }; } } + sym::copy_nonoverlapping => { + let target = destination.unwrap().1; + let mut args = args.drain(..); + block.statements.push(Statement { + source_info: terminator.source_info, + kind: StatementKind::CopyNonOverlapping( + box rustc_middle::mir::CopyNonOverlapping { + src: args.next().unwrap(), + dst: args.next().unwrap(), + count: args.next().unwrap(), + }, + ), + }); + assert_eq!( + args.next(), + None, + "Extra argument for copy_non_overlapping intrinsic" + ); + drop(args); + terminator.kind = TerminatorKind::Goto { target }; + } sym::wrapping_add | sym::wrapping_sub | sym::wrapping_mul => { if let Some((destination, target)) = *destination { let lhs; diff --git a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs index a37f5d4f329f3..5347846a4b334 100644 --- a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs +++ b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs @@ -39,7 +39,6 @@ impl RemoveNoopLandingPads { | StatementKind::StorageDead(_) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) - | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // These are all nops in a landing pad } @@ -56,6 +55,7 @@ impl RemoveNoopLandingPads { StatementKind::Assign { .. } | StatementKind::SetDiscriminant { .. } | StatementKind::LlvmInlineAsm { .. } + | StatementKind::CopyNonOverlapping(..) | StatementKind::Retag { .. } => { return false; } diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 03f1644386532..d401e25e316db 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -224,7 +224,7 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen check_operand(tcx, dst, span, body)?; check_operand(tcx, src, span, body)?; check_operand(tcx, count, span, body) - }, + } // These are all NOPs StatementKind::StorageLive(_) | StatementKind::StorageDead(_)