From 20aba8f634c13fa2bb1b043b51a074769dc06f66 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Wed, 10 Jun 2020 09:54:02 -0700 Subject: [PATCH] added test, Operand::const_from_scalar, require_lang_item, & comments Addresses feedback from @oli-obk (Thanks!) --- src/librustc_middle/mir/mod.rs | 28 +++++++ src/librustc_mir/interpret/intrinsics.rs | 1 + .../transform/instrument_coverage.rs | 52 ++++-------- src/test/mir-opt/instrument_coverage.rs | 19 +++++ .../rustc.bar.InstrumentCoverage.diff | 41 ++++++++++ .../rustc.main.InstrumentCoverage.diff | 82 +++++++++++++++++++ 6 files changed, 186 insertions(+), 37 deletions(-) create mode 100644 src/test/mir-opt/instrument_coverage.rs create mode 100644 src/test/mir-opt/instrument_coverage/rustc.bar.InstrumentCoverage.diff create mode 100644 src/test/mir-opt/instrument_coverage/rustc.main.InstrumentCoverage.diff diff --git a/src/librustc_middle/mir/mod.rs b/src/librustc_middle/mir/mod.rs index 27848684706..11ae2cf72c4 100644 --- a/src/librustc_middle/mir/mod.rs +++ b/src/librustc_middle/mir/mod.rs @@ -29,6 +29,7 @@ use rustc_macros::HashStable; use rustc_serialize::{Decodable, Encodable}; use rustc_span::symbol::Symbol; use rustc_span::{Span, DUMMY_SP}; +use rustc_target::abi; use rustc_target::asm::InlineAsmRegOrRegClass; use std::borrow::Cow; use std::fmt::{self, Debug, Display, Formatter, Write}; @@ -2218,6 +2219,33 @@ impl<'tcx> Operand<'tcx> { }) } + /// Convenience helper to make a literal-like constant from a given scalar value. + /// Since this is used to synthesize MIR, assumes `user_ty` is None. + pub fn const_from_scalar( + tcx: TyCtxt<'tcx>, + ty: Ty<'tcx>, + val: Scalar, + span: Span, + ) -> Operand<'tcx> { + debug_assert!({ + let param_env_and_ty = ty::ParamEnv::empty().and(ty); + let type_size = tcx + .layout_of(param_env_and_ty) + .unwrap_or_else(|e| panic!("could not compute layout for {:?}: {:?}", ty, e)) + .size; + let scalar_size = abi::Size::from_bytes(match val { + Scalar::Raw { size, .. } => size, + _ => panic!("Invalid scalar type {:?}", val), + }); + scalar_size == type_size + }); + Operand::Constant(box Constant { + span, + user_ty: None, + literal: ty::Const::from_scalar(tcx, val, ty), + }) + } + pub fn to_copy(&self) -> Self { match *self { Operand::Copy(_) | Operand::Constant(_) => self.clone(), diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 4d8120794f8..ac28ccd1815 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -389,6 +389,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ); self.copy_op(self.operand_index(args[0], index)?, dest)?; } + // FIXME(#73156): Handle source code coverage in const eval sym::count_code_region => (), _ => return Ok(false), } diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs index 27abe813b06..fda7ad731fa 100644 --- a/src/librustc_mir/transform/instrument_coverage.rs +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -1,21 +1,17 @@ use crate::transform::{MirPass, MirSource}; use crate::util::patch::MirPatch; +use rustc_hir::lang_items; use rustc_middle::mir::interpret::Scalar; use rustc_middle::mir::*; use rustc_middle::ty; -use rustc_middle::ty::Ty; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::DefId; use rustc_span::Span; -use rustc_target::abi; +/// Inserts call to count_code_region() as a placeholder to be replaced during code generation with +/// the intrinsic llvm.instrprof.increment. pub struct InstrumentCoverage; -/** - * Inserts call to count_code_region() as a placeholder to be replaced during code generation with - * the intrinsic llvm.instrprof.increment. - */ - impl<'tcx> MirPass<'tcx> for InstrumentCoverage { fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut Body<'tcx>) { if tcx.sess.opts.debugging_opts.instrument_coverage { @@ -34,10 +30,17 @@ const INIT_FUNCTION_COUNTER: u32 = 0; pub fn instrument_coverage<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let span = body.span.shrink_to_lo(); - let count_code_region_fn = - function_handle(tcx, span, tcx.lang_items().count_code_region_fn().unwrap()); - let counter_index = - const_int_operand(tcx, span, tcx.types.u32, Scalar::from_u32(INIT_FUNCTION_COUNTER)); + let count_code_region_fn = function_handle( + tcx, + tcx.require_lang_item(lang_items::CountCodeRegionFnLangItem, None), + span, + ); + let counter_index = Operand::const_from_scalar( + tcx, + tcx.types.u32, + Scalar::from_u32(INIT_FUNCTION_COUNTER), + span, + ); let mut patch = MirPatch::new(body); @@ -68,38 +71,13 @@ pub fn instrument_coverage<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { body.basic_blocks_mut().swap(next_block, new_block); } -fn function_handle<'tcx>(tcx: TyCtxt<'tcx>, span: Span, fn_def_id: DefId) -> Operand<'tcx> { +fn function_handle<'tcx>(tcx: TyCtxt<'tcx>, fn_def_id: DefId, span: Span) -> Operand<'tcx> { let ret_ty = tcx.fn_sig(fn_def_id).output(); let ret_ty = ret_ty.no_bound_vars().unwrap(); let substs = tcx.mk_substs(::std::iter::once(ty::subst::GenericArg::from(ret_ty))); Operand::function_handle(tcx, fn_def_id, substs, span) } -fn const_int_operand<'tcx>( - tcx: TyCtxt<'tcx>, - span: Span, - ty: Ty<'tcx>, - val: Scalar, -) -> Operand<'tcx> { - debug_assert!({ - let param_env_and_ty = ty::ParamEnv::empty().and(ty); - let type_size = tcx - .layout_of(param_env_and_ty) - .unwrap_or_else(|e| panic!("could not compute layout for {:?}: {:?}", ty, e)) - .size; - let scalar_size = abi::Size::from_bytes(match val { - Scalar::Raw { size, .. } => size, - _ => panic!("Invalid scalar type {:?}", val), - }); - scalar_size == type_size - }); - Operand::Constant(box Constant { - span, - user_ty: None, - literal: ty::Const::from_scalar(tcx, val, ty), - }) -} - fn placeholder_block<'tcx>(source_info: SourceInfo) -> BasicBlockData<'tcx> { BasicBlockData { statements: vec![], diff --git a/src/test/mir-opt/instrument_coverage.rs b/src/test/mir-opt/instrument_coverage.rs new file mode 100644 index 00000000000..e8c723b528a --- /dev/null +++ b/src/test/mir-opt/instrument_coverage.rs @@ -0,0 +1,19 @@ +// Test that the initial version of Rust coverage injects count_code_region() placeholder calls, +// at the top of each function. The placeholders are later converted into LLVM instrprof.increment +// intrinsics, during codegen. + +// compile-flags: -Zinstrument-coverage +// EMIT_MIR rustc.main.InstrumentCoverage.diff +// EMIT_MIR rustc.bar.InstrumentCoverage.diff +fn main() { + loop { + if bar() { + break; + } + } +} + +#[inline(never)] +fn bar() -> bool { + true +} diff --git a/src/test/mir-opt/instrument_coverage/rustc.bar.InstrumentCoverage.diff b/src/test/mir-opt/instrument_coverage/rustc.bar.InstrumentCoverage.diff new file mode 100644 index 00000000000..d23bb93d951 --- /dev/null +++ b/src/test/mir-opt/instrument_coverage/rustc.bar.InstrumentCoverage.diff @@ -0,0 +1,41 @@ +- // MIR for `bar` before InstrumentCoverage ++ // MIR for `bar` after InstrumentCoverage + + fn bar() -> bool { + let mut _0: bool; // return place in scope 0 at $DIR/instrument_coverage.rs:17:13: 17:17 ++ let mut _1: (); // in scope 0 at $DIR/instrument_coverage.rs:17:1: 19:2 + + bb0: { ++ StorageLive(_1); // scope 0 at $DIR/instrument_coverage.rs:17:1: 19:2 ++ _1 = const std::intrinsics::count_code_region(const 0u32) -> bb2; // scope 0 at $DIR/instrument_coverage.rs:17:1: 19:2 ++ // ty::Const ++ // + ty: unsafe extern "rust-intrinsic" fn(u32) {std::intrinsics::count_code_region} ++ // + val: Value(Scalar()) ++ // mir::Constant ++ // + span: $DIR/instrument_coverage.rs:17:1: 17:1 ++ // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(u32) {std::intrinsics::count_code_region}, val: Value(Scalar()) } ++ // ty::Const ++ // + ty: u32 ++ // + val: Value(Scalar(0x00000000)) ++ // mir::Constant ++ // + span: $DIR/instrument_coverage.rs:17:1: 17:1 ++ // + literal: Const { ty: u32, val: Value(Scalar(0x00000000)) } ++ } ++ ++ bb1 (cleanup): { ++ resume; // scope 0 at $DIR/instrument_coverage.rs:17:1: 19:2 ++ } ++ ++ bb2: { ++ StorageDead(_1); // scope 0 at $DIR/instrument_coverage.rs:18:5: 18:9 + _0 = const true; // scope 0 at $DIR/instrument_coverage.rs:18:5: 18:9 + // ty::Const + // + ty: bool + // + val: Value(Scalar(0x01)) + // mir::Constant + // + span: $DIR/instrument_coverage.rs:18:5: 18:9 + // + literal: Const { ty: bool, val: Value(Scalar(0x01)) } + return; // scope 0 at $DIR/instrument_coverage.rs:19:2: 19:2 + } + } + diff --git a/src/test/mir-opt/instrument_coverage/rustc.main.InstrumentCoverage.diff b/src/test/mir-opt/instrument_coverage/rustc.main.InstrumentCoverage.diff new file mode 100644 index 00000000000..d5d0f82495d --- /dev/null +++ b/src/test/mir-opt/instrument_coverage/rustc.main.InstrumentCoverage.diff @@ -0,0 +1,82 @@ +- // MIR for `main` before InstrumentCoverage ++ // MIR for `main` after InstrumentCoverage + + fn main() -> () { + let mut _0: (); // return place in scope 0 at $DIR/instrument_coverage.rs:8:11: 8:11 + let mut _1: (); // in scope 0 at $DIR/instrument_coverage.rs:8:1: 14:2 + let mut _2: bool; // in scope 0 at $DIR/instrument_coverage.rs:10:12: 10:17 + let mut _3: !; // in scope 0 at $DIR/instrument_coverage.rs:10:18: 12:10 ++ let mut _4: (); // in scope 0 at $DIR/instrument_coverage.rs:8:1: 14:2 + + bb0: { +- falseUnwind -> [real: bb1, cleanup: bb6]; // scope 0 at $DIR/instrument_coverage.rs:9:5: 13:6 ++ StorageLive(_4); // scope 0 at $DIR/instrument_coverage.rs:8:1: 14:2 ++ _4 = const std::intrinsics::count_code_region(const 0u32) -> bb7; // scope 0 at $DIR/instrument_coverage.rs:8:1: 14:2 ++ // ty::Const ++ // + ty: unsafe extern "rust-intrinsic" fn(u32) {std::intrinsics::count_code_region} ++ // + val: Value(Scalar()) ++ // mir::Constant ++ // + span: $DIR/instrument_coverage.rs:8:1: 8:1 ++ // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(u32) {std::intrinsics::count_code_region}, val: Value(Scalar()) } ++ // ty::Const ++ // + ty: u32 ++ // + val: Value(Scalar(0x00000000)) ++ // mir::Constant ++ // + span: $DIR/instrument_coverage.rs:8:1: 8:1 ++ // + literal: Const { ty: u32, val: Value(Scalar(0x00000000)) } + } + + bb1: { + StorageLive(_2); // scope 0 at $DIR/instrument_coverage.rs:10:12: 10:17 + _2 = const bar() -> [return: bb2, unwind: bb6]; // scope 0 at $DIR/instrument_coverage.rs:10:12: 10:17 + // ty::Const + // + ty: fn() -> bool {bar} + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/instrument_coverage.rs:10:12: 10:15 + // + literal: Const { ty: fn() -> bool {bar}, val: Value(Scalar()) } + } + + bb2: { + FakeRead(ForMatchedPlace, _2); // scope 0 at $DIR/instrument_coverage.rs:10:12: 10:17 + switchInt(_2) -> [false: bb4, otherwise: bb3]; // scope 0 at $DIR/instrument_coverage.rs:10:9: 12:10 + } + + bb3: { + falseEdges -> [real: bb5, imaginary: bb4]; // scope 0 at $DIR/instrument_coverage.rs:10:9: 12:10 + } + + bb4: { + _1 = const (); // scope 0 at $DIR/instrument_coverage.rs:10:9: 12:10 + // ty::Const + // + ty: () + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/instrument_coverage.rs:10:9: 12:10 + // + literal: Const { ty: (), val: Value(Scalar()) } + StorageDead(_2); // scope 0 at $DIR/instrument_coverage.rs:13:5: 13:6 + goto -> bb0; // scope 0 at $DIR/instrument_coverage.rs:9:5: 13:6 + } + + bb5: { + _0 = const (); // scope 0 at $DIR/instrument_coverage.rs:11:13: 11:18 + // ty::Const + // + ty: () + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/instrument_coverage.rs:11:13: 11:18 + // + literal: Const { ty: (), val: Value(Scalar()) } + StorageDead(_2); // scope 0 at $DIR/instrument_coverage.rs:13:5: 13:6 + return; // scope 0 at $DIR/instrument_coverage.rs:14:2: 14:2 + } + + bb6 (cleanup): { + resume; // scope 0 at $DIR/instrument_coverage.rs:8:1: 14:2 ++ } ++ ++ bb7: { ++ StorageDead(_4); // scope 0 at $DIR/instrument_coverage.rs:9:5: 13:6 ++ falseUnwind -> [real: bb1, cleanup: bb6]; // scope 0 at $DIR/instrument_coverage.rs:9:5: 13:6 + } + } +