Add clif ir comments for stack2reg opt

This commit is contained in:
bjorn3 2020-03-20 12:18:40 +01:00
parent 02d85dd590
commit a59479bd37
7 changed files with 61 additions and 59 deletions

View File

@ -200,7 +200,7 @@ impl<'tcx, B: Backend + 'static> FunctionCx<'_, 'tcx, B> {
.declare_func_in_func(func_id, &mut self.bcx.func);
#[cfg(debug_assertions)]
self.add_entity_comment(func_ref, format!("{:?}", inst));
self.add_comment(func_ref, format!("{:?}", inst));
func_ref
}

View File

@ -52,7 +52,7 @@ fn codegen_static_ref<'tcx>(
let data_id = data_id_for_static(fx.tcx, fx.module, def_id, linkage);
let local_data_id = fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
#[cfg(debug_assertions)]
fx.add_entity_comment(local_data_id, format!("{:?}", def_id));
fx.add_comment(local_data_id, format!("{:?}", def_id));
cplace_for_dataid(fx, layout, local_data_id)
}
@ -114,7 +114,7 @@ pub fn trans_const_value<'tcx>(
let data_id = data_id_for_alloc_id(fx.module, ptr.alloc_id, alloc.align);
let local_data_id = fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
#[cfg(debug_assertions)]
fx.add_entity_comment(local_data_id, format!("{:?}", ptr.alloc_id));
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)) => {
@ -128,7 +128,7 @@ pub fn trans_const_value<'tcx>(
let data_id = data_id_for_static(fx.tcx, fx.module, def_id, linkage);
let local_data_id = fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
#[cfg(debug_assertions)]
fx.add_entity_comment(local_data_id, format!("{:?}", def_id));
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),
@ -200,7 +200,7 @@ fn trans_const_place<'tcx>(
let data_id = data_id_for_alloc_id(fx.module, alloc_id, alloc.align);
let local_data_id = fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
#[cfg(debug_assertions)]
fx.add_entity_comment(local_data_id, format!("{:?}", alloc_id));
fx.add_comment(local_data_id, format!("{:?}", alloc_id));
cplace_for_dataid(fx, fx.layout_of(const_.ty), local_data_id)
}

View File

@ -16,7 +16,7 @@ pub fn optimize_function<'tcx>(
if tcx.sess.opts.optimize == rustc_session::config::OptLevel::No {
return; // FIXME classify optimizations over opt levels
}
self::stack2reg::optimize_function(ctx, clif_comments, instance);
self::stack2reg::optimize_function(ctx, clif_comments);
#[cfg(debug_assertions)]
crate::pretty_clif::write_clif_file(tcx, "stack2reg", instance, &ctx.func, &*clif_comments, None);
crate::base::verify_func(tcx, &*clif_comments, &ctx.func);

View File

@ -10,6 +10,7 @@
//! being loaded by a `stack_load`.
use std::collections::{BTreeMap, HashSet};
use std::fmt;
use std::ops::Not;
use cranelift_codegen::cursor::{Cursor, FuncCursor};
@ -19,9 +20,15 @@ use cranelift_codegen::ir::immediates::Offset32;
use crate::prelude::*;
/// Workaround for `StackSlot` not implementing `Ord`.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
#[derive(Copy, Clone, PartialEq, Eq)]
struct OrdStackSlot(StackSlot);
impl fmt::Debug for OrdStackSlot {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{:?}", self.0)
}
}
impl PartialOrd for OrdStackSlot {
fn partial_cmp(&self, rhs: &Self) -> Option<std::cmp::Ordering> {
self.0.as_u32().partial_cmp(&rhs.0.as_u32())
@ -153,10 +160,9 @@ impl<'a> OptimizeContext<'a> {
}
}
pub(super) fn optimize_function<T: std::fmt::Debug>(
pub(super) fn optimize_function(
ctx: &mut Context,
_clif_comments: &mut crate::pretty_clif::CommentWriter,
name: T,
clif_comments: &mut crate::pretty_clif::CommentWriter,
) {
combine_stack_addr_with_load_store(&mut ctx.func);
@ -167,7 +173,9 @@ pub(super) fn optimize_function<T: std::fmt::Debug>(
remove_unused_stack_addr_and_stack_load(&mut opt_ctx);
#[cfg(debug_assertions)] {
println!("stack slot usage: {:?}", opt_ctx.stack_slot_usage_map);
for (&OrdStackSlot(stack_slot), usage) in &opt_ctx.stack_slot_usage_map {
clif_comments.add_comment(stack_slot, format!("used by: {:?}", usage));
}
}
for (stack_slot, users) in opt_ctx.stack_slot_usage_map.iter_mut() {
@ -182,28 +190,26 @@ pub(super) fn optimize_function<T: std::fmt::Debug>(
#[cfg(debug_assertions)]
for &store in &potential_stores {
println!(
clif_comments.add_comment(load, format!(
"Potential store -> load forwarding {} -> {} ({:?}, {:?})",
opt_ctx.ctx.func.dfg.display_inst(store, None),
opt_ctx.ctx.func.dfg.display_inst(load, None),
spatial_overlap(&opt_ctx.ctx.func, store, load),
temporal_order(&opt_ctx.ctx, store, load),
);
));
}
match *potential_stores {
[] => {
#[cfg(debug_assertions)] {
println!("[{:?}] [BUG?] Reading uninitialized memory", name);
}
#[cfg(debug_assertions)]
clif_comments.add_comment(load, format!("[BUG?] Reading uninitialized memory"));
}
[store] if spatial_overlap(&opt_ctx.ctx.func, store, load) == SpatialOverlap::Full && temporal_order(&opt_ctx.ctx, store, load) == TemporalOrder::DefinitivelyBefore => {
// 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)] {
println!("Store to load forward {} -> {}", store, load);
}
#[cfg(debug_assertions)]
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);
}
@ -216,22 +222,22 @@ pub(super) fn optimize_function<T: std::fmt::Debug>(
#[cfg(debug_assertions)]
for &load in &potential_loads {
println!(
clif_comments.add_comment(store, format!(
"Potential load from store {} <- {} ({:?}, {:?})",
opt_ctx.ctx.func.dfg.display_inst(load, None),
opt_ctx.ctx.func.dfg.display_inst(store, None),
spatial_overlap(&opt_ctx.ctx.func, store, load),
temporal_order(&opt_ctx.ctx, store, load),
);
));
}
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)] {
println!("[{:?}] Remove dead stack store {} of {}", name, opt_ctx.ctx.func.dfg.display_inst(store, None), stack_slot.0);
}
#[cfg(debug_assertions)]
clif_comments.add_comment(store, format!("Remove dead stack store {} of {}", opt_ctx.ctx.func.dfg.display_inst(store, None), stack_slot.0));
users.remove_dead_store(&mut opt_ctx.ctx.func, store);
}
}
@ -240,10 +246,6 @@ pub(super) fn optimize_function<T: std::fmt::Debug>(
opt_ctx.ctx.func.stack_slots[stack_slot.0].size = 0;
}
}
#[cfg(debug_assertions)] {
println!();
}
}
fn combine_stack_addr_with_load_store(func: &mut Function) {

View File

@ -69,7 +69,6 @@ use crate::prelude::*;
pub struct CommentWriter {
global_comments: Vec<String>,
entity_comments: HashMap<AnyEntity, String>,
inst_comments: HashMap<Inst, String>,
}
impl CommentWriter {
@ -94,7 +93,30 @@ impl CommentWriter {
CommentWriter {
global_comments,
entity_comments: HashMap::new(),
inst_comments: HashMap::new(),
}
}
}
#[cfg(debug_assertions)]
impl CommentWriter {
pub fn add_global_comment<S: Into<String>>(&mut self, comment: S) {
self.global_comments.push(comment.into());
}
pub fn add_comment<'s, S: Into<Cow<'s, str>>, E: Into<AnyEntity>>(
&mut self,
entity: E,
comment: S,
) {
use std::collections::hash_map::Entry;
match self.entity_comments.entry(entity.into()) {
Entry::Occupied(mut occ) => {
occ.get_mut().push('\n');
occ.get_mut().push_str(comment.into().as_ref());
}
Entry::Vacant(vac) => {
vac.insert(comment.into().into_owned());
}
}
}
}
@ -157,7 +179,7 @@ impl FuncWriter for &'_ CommentWriter {
indent: usize,
) -> fmt::Result {
PlainWriter.write_instruction(w, func, aliases, isa, inst, indent)?;
if let Some(comment) = self.inst_comments.get(&inst) {
if let Some(comment) = self.entity_comments.get(&inst.into()) {
writeln!(w, "; {}", comment.replace('\n', "\n; "))?;
}
Ok(())
@ -167,37 +189,15 @@ impl FuncWriter for &'_ CommentWriter {
#[cfg(debug_assertions)]
impl<'a, 'tcx, B: Backend + 'static> FunctionCx<'_, 'tcx, B> {
pub fn add_global_comment<S: Into<String>>(&mut self, comment: S) {
self.clif_comments.global_comments.push(comment.into());
self.clif_comments.add_global_comment(comment);
}
pub fn add_entity_comment<'s, S: Into<Cow<'s, str>>, E: Into<AnyEntity>>(
pub fn add_comment<'s, S: Into<Cow<'s, str>>, E: Into<AnyEntity>>(
&mut self,
entity: E,
comment: S,
) {
use std::collections::hash_map::Entry;
match self.clif_comments.entity_comments.entry(entity.into()) {
Entry::Occupied(mut occ) => {
occ.get_mut().push('\n');
occ.get_mut().push_str(comment.into().as_ref());
}
Entry::Vacant(vac) => {
vac.insert(comment.into().into_owned());
}
}
}
pub fn add_comment<'s, S: Into<Cow<'s, str>>>(&mut self, inst: Inst, comment: S) {
use std::collections::hash_map::Entry;
match self.clif_comments.inst_comments.entry(inst) {
Entry::Occupied(mut occ) => {
occ.get_mut().push('\n');
occ.get_mut().push_str(comment.into().as_ref());
}
Entry::Vacant(vac) => {
vac.insert(comment.into().into_owned());
}
}
self.clif_comments.add_comment(entity, comment);
}
}

View File

@ -19,7 +19,7 @@ fn codegen_print(fx: &mut FunctionCx<'_, '_, impl cranelift_module::Backend>, ms
let puts = fx.module.declare_func_in_func(puts, &mut fx.bcx.func);
#[cfg(debug_assertions)]
{
fx.add_entity_comment(puts, "puts");
fx.add_comment(puts, "puts");
}
let symbol_name = fx.tcx.symbol_name(fx.instance);
@ -46,7 +46,7 @@ fn codegen_print(fx: &mut FunctionCx<'_, '_, impl cranelift_module::Backend>, ms
let local_msg_id = fx.module.declare_data_in_func(msg_id, fx.bcx.func);
#[cfg(debug_assertions)]
{
fx.add_entity_comment(local_msg_id, msg);
fx.add_comment(local_msg_id, msg);
}
let msg_ptr = fx.bcx.ins().global_value(pointer_ty(fx.tcx), local_msg_id);
fx.bcx.ins().call(puts, &[msg_ptr]);

View File

@ -381,7 +381,7 @@ impl<'tcx> CPlace<'tcx> {
};
fx.add_comment(
fx.bcx.func.layout.last_inst(cur_block).unwrap(),
format!("write_cvalue: {:?} <- {:?}",self, from),
format!("write_cvalue: {:?}: {:?} <- {:?}: {:?}", self.inner(), self.layout().ty, from.0, from.layout().ty),
);
}