1
Fork 0

Add clif comments when in release mode

Fixes #1130
This commit is contained in:
bjorn3 2021-03-06 18:45:04 +01:00
parent 4ca3384db6
commit cecd7a9ae6
13 changed files with 117 additions and 102 deletions

View file

@ -11,9 +11,11 @@ use cranelift_codegen::entity::EntityRef;
use crate::prelude::*;
pub(super) fn add_args_header_comment(fx: &mut FunctionCx<'_, '_, '_>) {
if fx.clif_comments.enabled() {
fx.add_global_comment(
"kind loc.idx param pass mode ty".to_string(),
);
}
}
pub(super) fn add_arg_comment<'tcx>(
@ -25,6 +27,10 @@ pub(super) fn add_arg_comment<'tcx>(
arg_abi_mode: PassMode,
arg_layout: TyAndLayout<'tcx>,
) {
if !fx.clif_comments.enabled() {
return;
}
let local = if let Some(local) = local {
Cow::Owned(format!("{:?}", local))
} else {
@ -59,10 +65,12 @@ pub(super) fn add_arg_comment<'tcx>(
}
pub(super) fn add_locals_header_comment(fx: &mut FunctionCx<'_, '_, '_>) {
if fx.clif_comments.enabled() {
fx.add_global_comment(String::new());
fx.add_global_comment(
"kind local ty size align (abi,pref)".to_string(),
);
}
}
pub(super) fn add_local_place_comments<'tcx>(
@ -70,6 +78,9 @@ pub(super) fn add_local_place_comments<'tcx>(
place: CPlace<'tcx>,
local: Local,
) {
if !fx.clif_comments.enabled() {
return;
}
let TyAndLayout { ty, layout } = place.layout();
let rustc_target::abi::Layout { size, align, abi: _, variants: _, fields: _, largest_niche: _ } =
layout;
@ -90,7 +101,7 @@ pub(super) fn add_local_place_comments<'tcx>(
} else {
Cow::Borrowed("")
};
match ptr.base_and_offset() {
match ptr.debug_base_and_offset() {
(crate::pointer::PointerBase::Addr(addr), offset) => {
("reuse", format!("storage={}{}{}", addr, offset, meta).into())
}

View file

@ -1,6 +1,5 @@
//! Handling of everything related to the calling convention. Also fills `fx.local_map`.
#[cfg(debug_assertions)]
mod comments;
mod pass_mode;
mod returning;
@ -75,8 +74,9 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
let func_id = import_function(self.tcx, self.cx.module, inst);
let func_ref = self.cx.module.declare_func_in_func(func_id, &mut self.bcx.func);
#[cfg(debug_assertions)]
if self.clif_comments.enabled() {
self.add_comment(func_ref, format!("{:?}", inst));
}
func_ref
}
@ -92,8 +92,7 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
let func_id = self.cx.module.declare_function(&name, Linkage::Import, &sig).unwrap();
let func_ref = self.cx.module.declare_func_in_func(func_id, &mut self.bcx.func);
let call_inst = self.bcx.ins().call(func_ref, args);
#[cfg(debug_assertions)]
{
if self.clif_comments.enabled() {
self.add_comment(call_inst, format!("easy_call {}", name));
}
let results = self.bcx.inst_results(call_inst);
@ -149,7 +148,6 @@ fn make_local_place<'tcx>(
CPlace::new_stack_slot(fx, layout)
};
#[cfg(debug_assertions)]
self::comments::add_local_place_comments(fx, place, local);
place
@ -163,7 +161,6 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_
let ssa_analyzed = crate::analyze::analyze(fx);
#[cfg(debug_assertions)]
self::comments::add_args_header_comment(fx);
let mut block_params_iter = fx.bcx.func.dfg.block_params(start_block).to_vec().into_iter();
@ -228,7 +225,6 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_
fx.fn_abi = Some(fn_abi);
assert!(block_params_iter.next().is_none(), "arg_value left behind");
#[cfg(debug_assertions)]
self::comments::add_locals_header_comment(fx);
for (local, arg_kind, ty) in func_params {
@ -256,7 +252,6 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_
CPlace::for_ptr(addr, val.layout())
};
#[cfg(debug_assertions)]
self::comments::add_local_place_comments(fx, place, local);
assert_eq!(fx.local_map.push(place), local);
@ -392,8 +387,7 @@ pub(crate) fn codegen_terminator_call<'tcx>(
let (func_ref, first_arg) = match instance {
// Trait object call
Some(Instance { def: InstanceDef::Virtual(_, idx), .. }) => {
#[cfg(debug_assertions)]
{
if fx.clif_comments.enabled() {
let nop_inst = fx.bcx.ins().nop();
fx.add_comment(
nop_inst,
@ -414,8 +408,7 @@ pub(crate) fn codegen_terminator_call<'tcx>(
// Indirect call
None => {
#[cfg(debug_assertions)]
{
if fx.clif_comments.enabled() {
let nop_inst = fx.bcx.ins().nop();
fx.add_comment(nop_inst, "indirect call");
}

View file

@ -248,8 +248,8 @@ pub(super) fn adjust_arg_for_abi<'tcx>(
/// as necessary.
pub(super) fn cvalue_for_param<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
#[cfg_attr(not(debug_assertions), allow(unused_variables))] local: Option<mir::Local>,
#[cfg_attr(not(debug_assertions), allow(unused_variables))] local_field: Option<usize>,
local: Option<mir::Local>,
local_field: Option<usize>,
arg_abi: &ArgAbi<'tcx, Ty<'tcx>>,
block_params_iter: &mut impl Iterator<Item = Value>,
) -> Option<CValue<'tcx>> {
@ -263,7 +263,6 @@ pub(super) fn cvalue_for_param<'tcx>(
})
.collect::<SmallVec<[_; 2]>>();
#[cfg(debug_assertions)]
crate::abi::comments::add_arg_comment(
fx,
"arg",

View file

@ -84,10 +84,6 @@ pub(super) fn codegen_return_param<'tcx>(
}
};
#[cfg(not(debug_assertions))]
let _ = ret_param;
#[cfg(debug_assertions)]
crate::abi::comments::add_arg_comment(
fx,
"ret",

View file

@ -219,8 +219,7 @@ fn codegen_fn_content(fx: &mut FunctionCx<'_, '_, '_>) {
codegen_stmt(fx, block, stmt);
}
#[cfg(debug_assertions)]
{
if fx.clif_comments.enabled() {
let mut terminator_head = "\n".to_string();
bb_data.terminator().kind.fmt_head(&mut terminator_head).unwrap();
let inst = fx.bcx.func.layout.last_inst(block).unwrap();
@ -433,14 +432,16 @@ fn codegen_stmt<'tcx>(
fx.set_debug_loc(stmt.source_info);
#[cfg(false_debug_assertions)]
#[cfg(disabled)]
match &stmt.kind {
StatementKind::StorageLive(..) | StatementKind::StorageDead(..) => {} // Those are not very useful
_ => {
if fx.clif_comments.enabled() {
let inst = fx.bcx.func.layout.last_inst(cur_block).unwrap();
fx.add_comment(inst, format!("{:?}", stmt));
}
}
}
match &stmt.kind {
StatementKind::SetDiscriminant { place, variant_index } => {

View file

@ -361,8 +361,7 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
let _ = self.cx.module.define_data(msg_id, &data_ctx);
let local_msg_id = self.cx.module.declare_data_in_func(msg_id, self.bcx.func);
#[cfg(debug_assertions)]
{
if self.clif_comments.enabled() {
self.add_comment(local_msg_id, msg);
}
self.bcx.ins().global_value(self.pointer_type, local_msg_id)

View file

@ -82,8 +82,9 @@ pub(crate) fn codegen_tls_ref<'tcx>(
) -> CValue<'tcx> {
let data_id = data_id_for_static(fx.tcx, fx.cx.module, def_id, false);
let local_data_id = fx.cx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
#[cfg(debug_assertions)]
if fx.clif_comments.enabled() {
fx.add_comment(local_data_id, format!("tls {:?}", def_id));
}
let tls_ptr = fx.bcx.ins().tls_value(fx.pointer_type, local_data_id);
CValue::by_val(tls_ptr, layout)
}
@ -95,8 +96,9 @@ fn codegen_static_ref<'tcx>(
) -> CPlace<'tcx> {
let data_id = data_id_for_static(fx.tcx, fx.cx.module, def_id, false);
let local_data_id = fx.cx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
#[cfg(debug_assertions)]
if fx.clif_comments.enabled() {
fx.add_comment(local_data_id, format!("{:?}", def_id));
}
let global_ptr = fx.bcx.ins().global_value(fx.pointer_type, local_data_id);
assert!(!layout.is_unsized(), "unsized statics aren't supported");
assert!(
@ -182,8 +184,9 @@ pub(crate) fn codegen_const_value<'tcx>(
data_id_for_alloc_id(fx.cx.module, ptr.alloc_id, alloc.mutability);
let local_data_id =
fx.cx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
#[cfg(debug_assertions)]
if fx.clif_comments.enabled() {
fx.add_comment(local_data_id, format!("{:?}", ptr.alloc_id));
}
fx.bcx.ins().global_value(fx.pointer_type, local_data_id)
}
Some(GlobalAlloc::Function(instance)) => {
@ -198,8 +201,9 @@ pub(crate) fn codegen_const_value<'tcx>(
let data_id = data_id_for_static(fx.tcx, fx.cx.module, def_id, false);
let local_data_id =
fx.cx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
#[cfg(debug_assertions)]
if fx.clif_comments.enabled() {
fx.add_comment(local_data_id, format!("{:?}", def_id));
}
fx.bcx.ins().global_value(fx.pointer_type, local_data_id)
}
None => bug!("missing allocation {:?}", ptr.alloc_id),
@ -240,8 +244,9 @@ fn pointer_for_allocation<'tcx>(
let data_id = data_id_for_alloc_id(fx.cx.module, alloc_id, alloc.mutability);
let local_data_id = fx.cx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
#[cfg(debug_assertions)]
if fx.clif_comments.enabled() {
fx.add_comment(local_data_id, format!("{:?}", alloc_id));
}
let global_ptr = fx.bcx.ins().global_value(fx.pointer_type, local_data_id);
crate::pointer::Pointer::new(global_ptr)
}

View file

@ -197,8 +197,9 @@ fn call_inline_asm<'tcx>(
offset: None,
size: u32::try_from(slot_size.bytes()).unwrap(),
});
#[cfg(debug_assertions)]
if fx.clif_comments.enabled() {
fx.add_comment(stack_slot, "inline asm scratch slot");
}
let inline_asm_func = fx
.cx
@ -214,8 +215,9 @@ fn call_inline_asm<'tcx>(
)
.unwrap();
let inline_asm_func = fx.cx.module.declare_func_in_func(inline_asm_func, &mut fx.bcx.func);
#[cfg(debug_assertions)]
if fx.clif_comments.enabled() {
fx.add_comment(inline_asm_func, asm_name);
}
for (_reg, offset, value) in inputs {
fx.bcx.ins().stack_store(value, stack_slot, i32::try_from(offset.bytes()).unwrap());

View file

@ -181,7 +181,6 @@ impl<'a> OptimizeContext<'a> {
pub(super) fn optimize_function(
ctx: &mut Context,
#[cfg_attr(not(debug_assertions), allow(unused_variables))]
clif_comments: &mut crate::pretty_clif::CommentWriter,
) {
combine_stack_addr_with_load_store(&mut ctx.func);
@ -192,8 +191,7 @@ pub(super) fn optimize_function(
remove_unused_stack_addr_and_stack_load(&mut opt_ctx);
#[cfg(debug_assertions)]
{
if clif_comments.enabled() {
for (&OrdStackSlot(stack_slot), usage) in &opt_ctx.stack_slot_usage_map {
clif_comments.add_comment(stack_slot, format!("used by: {:?}", usage));
}
@ -209,7 +207,7 @@ pub(super) fn optimize_function(
for load in users.stack_load.clone().into_iter() {
let potential_stores = users.potential_stores_for_load(&opt_ctx.ctx, load);
#[cfg(debug_assertions)]
if clif_comments.enabled() {
for &store in &potential_stores {
clif_comments.add_comment(
load,
@ -222,13 +220,15 @@ pub(super) fn optimize_function(
),
);
}
}
match *potential_stores {
[] => {
#[cfg(debug_assertions)]
if clif_comments.enabled() {
clif_comments
.add_comment(load, "[BUG?] Reading uninitialized memory".to_string());
}
}
[store]
if spatial_overlap(&opt_ctx.ctx.func, store, load) == SpatialOverlap::Full
&& temporal_order(&opt_ctx.ctx, store, load)
@ -237,9 +237,12 @@ pub(super) fn optimize_function(
// Only one store could have been the origin of the value.
let stored_value = opt_ctx.ctx.func.dfg.inst_args(store)[0];
#[cfg(debug_assertions)]
clif_comments
.add_comment(load, format!("Store to load forward {} -> {}", store, load));
if clif_comments.enabled() {
clif_comments.add_comment(
load,
format!("Store to load forward {} -> {}", store, load),
);
}
users.change_load_to_alias(&mut opt_ctx.ctx.func, load, stored_value);
}
@ -250,7 +253,7 @@ pub(super) fn optimize_function(
for store in users.stack_store.clone().into_iter() {
let potential_loads = users.potential_loads_of_store(&opt_ctx.ctx, store);
#[cfg(debug_assertions)]
if clif_comments.enabled() {
for &load in &potential_loads {
clif_comments.add_comment(
store,
@ -263,12 +266,13 @@ pub(super) fn optimize_function(
),
);
}
}
if potential_loads.is_empty() {
// Never loaded; can safely remove all stores and the stack slot.
// FIXME also remove stores when there is always a next store before a load.
#[cfg(debug_assertions)]
if clif_comments.enabled() {
clif_comments.add_comment(
store,
format!(
@ -277,6 +281,7 @@ pub(super) fn optimize_function(
stack_slot.0
),
);
}
users.remove_dead_store(&mut opt_ctx.ctx.func, store);
}

View file

@ -39,8 +39,7 @@ impl Pointer {
Pointer { base: PointerBase::Dangling(align), offset: Offset32::new(0) }
}
#[cfg(debug_assertions)]
pub(crate) fn base_and_offset(self) -> (PointerBase, Offset32) {
pub(crate) fn debug_base_and_offset(self) -> (PointerBase, Offset32) {
(self.base, self.offset)
}

View file

@ -69,13 +69,15 @@ use crate::prelude::*;
#[derive(Debug)]
pub(crate) struct CommentWriter {
enabled: bool,
global_comments: Vec<String>,
entity_comments: FxHashMap<AnyEntity, String>,
}
impl CommentWriter {
pub(crate) fn new<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> Self {
let global_comments = if cfg!(debug_assertions) {
let enabled = should_write_ir(tcx);
let global_comments = if enabled {
vec![
format!("symbol {}", tcx.symbol_name(instance).name),
format!("instance {:?}", instance),
@ -86,13 +88,17 @@ impl CommentWriter {
vec![]
};
CommentWriter { global_comments, entity_comments: FxHashMap::default() }
CommentWriter { enabled, global_comments, entity_comments: FxHashMap::default() }
}
}
#[cfg(debug_assertions)]
impl CommentWriter {
pub(crate) fn enabled(&self) -> bool {
self.enabled
}
pub(crate) fn add_global_comment<S: Into<String>>(&mut self, comment: S) {
debug_assert!(self.enabled);
self.global_comments.push(comment.into());
}
@ -101,6 +107,8 @@ impl CommentWriter {
entity: E,
comment: S,
) {
debug_assert!(self.enabled);
use std::collections::hash_map::Entry;
match self.entity_comments.entry(entity.into()) {
Entry::Occupied(mut occ) => {
@ -179,7 +187,6 @@ impl FuncWriter for &'_ CommentWriter {
}
}
#[cfg(debug_assertions)]
impl FunctionCx<'_, '_, '_> {
pub(crate) fn add_global_comment<S: Into<String>>(&mut self, comment: S) {
self.clif_comments.add_global_comment(comment);

View file

@ -17,8 +17,7 @@ fn codegen_print(fx: &mut FunctionCx<'_, '_, '_>, msg: &str) {
)
.unwrap();
let puts = fx.cx.module.declare_func_in_func(puts, &mut fx.bcx.func);
#[cfg(debug_assertions)]
{
if fx.clif_comments.enabled() {
fx.add_comment(puts, "puts");
}

View file

@ -413,7 +413,7 @@ impl<'tcx> CPlace<'tcx> {
self,
fx: &mut FunctionCx<'_, '_, 'tcx>,
from: CValue<'tcx>,
#[cfg_attr(not(debug_assertions), allow(unused_variables))] method: &'static str,
method: &'static str,
) {
fn transmute_value<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
@ -461,8 +461,7 @@ impl<'tcx> CPlace<'tcx> {
assert_eq!(self.layout().size, from.layout().size);
#[cfg(debug_assertions)]
{
if fx.clif_comments.enabled() {
use cranelift_codegen::cursor::{Cursor, CursorPosition};
let cur_block = match fx.bcx.cursor().position() {
CursorPosition::After(block) => block,