From 2ceb5277284e7a485504d1f7d165a385cbfd797b Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 31 Mar 2021 12:42:30 +0200 Subject: [PATCH] Remove the cold block optimization It isn't effective with the new backend framework --- src/abi/mod.rs | 3 +-- src/base.rs | 11 +++-------- src/common.rs | 3 --- src/lib.rs | 1 - src/optimize/code_layout.rs | 34 ---------------------------------- src/optimize/mod.rs | 5 ----- 6 files changed, 4 insertions(+), 53 deletions(-) delete mode 100644 src/optimize/code_layout.rs diff --git a/src/abi/mod.rs b/src/abi/mod.rs index 0e7829eaa26a..f898348f97bc 100644 --- a/src/abi/mod.rs +++ b/src/abi/mod.rs @@ -295,7 +295,6 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_ pub(crate) fn codegen_terminator_call<'tcx>( fx: &mut FunctionCx<'_, '_, 'tcx>, span: Span, - current_block: Block, func: &Operand<'tcx>, args: &[Operand<'tcx>], destination: Option<(Place<'tcx>, BasicBlock)>, @@ -357,7 +356,7 @@ pub(crate) fn codegen_terminator_call<'tcx>( .map(|inst| fx.tcx.codegen_fn_attrs(inst.def_id()).flags.contains(CodegenFnAttrFlags::COLD)) .unwrap_or(false); if is_cold { - fx.cold_blocks.insert(current_block); + // FIXME Mark current_block block as cold once Cranelift supports it } // Unpack arguments tuple for closures diff --git a/src/base.rs b/src/base.rs index b34a29c25b92..c4e0c9676f69 100644 --- a/src/base.rs +++ b/src/base.rs @@ -55,7 +55,6 @@ pub(crate) fn codegen_fn<'tcx>(cx: &mut crate::CodegenCx<'_, 'tcx>, instance: In block_map, local_map: IndexVec::with_capacity(mir.local_decls.len()), caller_location: None, // set by `codegen_fn_prelude` - cold_blocks: EntitySet::new(), clif_comments, source_info_set: indexmap::IndexSet::new(), @@ -90,7 +89,6 @@ pub(crate) fn codegen_fn<'tcx>(cx: &mut crate::CodegenCx<'_, 'tcx>, instance: In let mut clif_comments = fx.clif_comments; let source_info_set = fx.source_info_set; let local_map = fx.local_map; - let cold_blocks = fx.cold_blocks; // Store function in context let context = &mut cx.cached_context; @@ -107,7 +105,6 @@ pub(crate) fn codegen_fn<'tcx>(cx: &mut crate::CodegenCx<'_, 'tcx>, instance: In tcx, instance, context, - &cold_blocks, &mut clif_comments, ); }); @@ -205,9 +202,8 @@ fn codegen_fn_content(fx: &mut FunctionCx<'_, '_, '_>) { // Unwinding after panicking is not supported continue; - // FIXME once unwinding is supported uncomment next lines - // // Unwinding is unlikely to happen, so mark cleanup block's as cold. - // fx.cold_blocks.insert(block); + // FIXME Once unwinding is supported and Cranelift supports marking blocks as cold, do + // so for cleanup blocks. } fx.bcx.ins().nop(); @@ -262,7 +258,7 @@ fn codegen_fn_content(fx: &mut FunctionCx<'_, '_, '_>) { let target = fx.get_block(*target); let failure = fx.bcx.create_block(); - fx.cold_blocks.insert(failure); + // FIXME Mark failure block as cold once Cranelift supports it if *expected { fx.bcx.ins().brz(cond, failure, &[]); @@ -358,7 +354,6 @@ fn codegen_fn_content(fx: &mut FunctionCx<'_, '_, '_>) { crate::abi::codegen_terminator_call( fx, *fn_span, - block, func, args, *destination, diff --git a/src/common.rs b/src/common.rs index b5874f62535c..fe6cf9a3b01e 100644 --- a/src/common.rs +++ b/src/common.rs @@ -242,9 +242,6 @@ pub(crate) struct FunctionCx<'m, 'clif, 'tcx> { /// When `#[track_caller]` is used, the implicit caller location is stored in this variable. pub(crate) caller_location: Option>, - /// See [`crate::optimize::code_layout`] for more information. - pub(crate) cold_blocks: EntitySet, - pub(crate) clif_comments: crate::pretty_clif::CommentWriter, pub(crate) source_info_set: indexmap::IndexSet, diff --git a/src/lib.rs b/src/lib.rs index 720d2a125344..8f4dede33ef4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -87,7 +87,6 @@ mod prelude { pub(crate) use rustc_index::vec::Idx; - pub(crate) use cranelift_codegen::entity::EntitySet; pub(crate) use cranelift_codegen::ir::condcodes::{FloatCC, IntCC}; pub(crate) use cranelift_codegen::ir::function::Function; pub(crate) use cranelift_codegen::ir::types; diff --git a/src/optimize/code_layout.rs b/src/optimize/code_layout.rs deleted file mode 100644 index ca9ff15ec10f..000000000000 --- a/src/optimize/code_layout.rs +++ /dev/null @@ -1,34 +0,0 @@ -//! This optimization moves cold code to the end of the function. -//! -//! Some code is executed much less often than other code. For example panicking or the -//! landingpads for unwinding. By moving this cold code to the end of the function the average -//! amount of jumps is reduced and the code locality is improved. -//! -//! # Undefined behaviour -//! -//! This optimization doesn't assume anything that isn't already assumed by Cranelift itself. - -use crate::prelude::*; - -pub(super) fn optimize_function(ctx: &mut Context, cold_blocks: &EntitySet) { - // FIXME Move the block in place instead of remove and append once - // bytecodealliance/cranelift#1339 is implemented. - - let mut block_insts = FxHashMap::default(); - for block in cold_blocks.keys().filter(|&block| cold_blocks.contains(block)) { - let insts = ctx.func.layout.block_insts(block).collect::>(); - for &inst in &insts { - ctx.func.layout.remove_inst(inst); - } - block_insts.insert(block, insts); - ctx.func.layout.remove_block(block); - } - - // And then append them at the back again. - for block in cold_blocks.keys().filter(|&block| cold_blocks.contains(block)) { - ctx.func.layout.append_block(block); - for inst in block_insts.remove(&block).unwrap() { - ctx.func.layout.append_inst(inst, block); - } - } -} diff --git a/src/optimize/mod.rs b/src/optimize/mod.rs index 95e8a449fa21..137fb5f77313 100644 --- a/src/optimize/mod.rs +++ b/src/optimize/mod.rs @@ -2,21 +2,16 @@ use crate::prelude::*; -mod code_layout; pub(crate) mod peephole; pub(crate) fn optimize_function<'tcx>( tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, ctx: &mut Context, - cold_blocks: &EntitySet, clif_comments: &mut crate::pretty_clif::CommentWriter, ) { // FIXME classify optimizations over opt levels once we have more - // The code_layout optimization is very cheap. - self::code_layout::optimize_function(ctx, cold_blocks); - crate::pretty_clif::write_clif_file(tcx, "preopt", None, instance, &ctx, &*clif_comments); crate::base::verify_func(tcx, &*clif_comments, &ctx.func); }