Skip to content

Commit

Permalink
Switch to changing cp_non_overlap in tform
Browse files Browse the repository at this point in the history
It was suggested to lower this in MIR instead of ssa, so do that instead.
  • Loading branch information
JulianKnodt committed Mar 9, 2021
1 parent d4ae9ff commit 217ff6b
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 62 deletions.
20 changes: 2 additions & 18 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_codegen_ssa/src/mir/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1682,7 +1682,7 @@ pub struct Coverage {
pub code_region: Option<CodeRegion>,
}

#[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>,
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_mir/src/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(..)
Expand Down
12 changes: 5 additions & 7 deletions compiler/rustc_mir/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand All @@ -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 => {
Expand Down
56 changes: 29 additions & 27 deletions compiler/rustc_mir/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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, <M as Machine<'mir, 'tcx>>::PointerTag>,
dst: OpTy<'tcx, <M as Machine<'mir, 'tcx>>::PointerTag>,
count: OpTy<'tcx, <M as Machine<'mir, '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
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/transform/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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);
}
Expand Down
21 changes: 21 additions & 0 deletions compiler/rustc_mir/src/transform/lower_intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ impl RemoveNoopLandingPads {
| StatementKind::StorageDead(_)
| StatementKind::AscribeUserType(..)
| StatementKind::Coverage(..)
| StatementKind::CopyNonOverlapping(..)
| StatementKind::Nop => {
// These are all nops in a landing pad
}
Expand All @@ -56,6 +55,7 @@ impl RemoveNoopLandingPads {
StatementKind::Assign { .. }
| StatementKind::SetDiscriminant { .. }
| StatementKind::LlvmInlineAsm { .. }
| StatementKind::CopyNonOverlapping(..)
| StatementKind::Retag { .. } => {
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(_)
Expand Down

0 comments on commit 217ff6b

Please sign in to comment.