T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[cranelift] Rejigger the compile API (#4540)

Browse Source 
* Move `emit_to_memory` to `MachCompileResult`

This small refactoring makes it clearer to me that emitting to memory
doesn't require anything else from the compilation `Context`. While it's
a trivial change, it's a small public API change that shouldn't cause
too much trouble, and doesn't seem RFC-worthy. Happy to hear different
opinions about this, though!

* hide the MachCompileResult behind a method

* Add a `CompileError` wrapper type that references a `Function`

* Rename MachCompileResult to CompiledCode

* Additionally remove the last unsafe API in cranelift-codegen
This commit is contained in:
Benjamin Bouvier
2022-08-02 21:05:40 +02:00
committed by GitHub
parent 37cd96beff
commit ff37c9d8a4
17 changed files with 156 additions and 198 deletions
Download Patch File Download Diff File Expand all files Collapse all files

147
cranelift/codegen/src/context.rs
View File

@@ -10,7 +10,6 @@
//! single ISA instance.
use crate::alias_analysis::AliasAnalysis;
use crate::binemit::CodeInfo;
use crate::dce::do_dce;
use crate::dominator_tree::DominatorTree;
use crate::flowgraph::ControlFlowGraph;
@@ -19,16 +18,16 @@ use crate::isa::TargetIsa;
use crate::legalizer::simple_legalize;
use crate::licm::do_licm;
use crate::loop_analysis::LoopAnalysis;
use crate::machinst::MachCompileResult;
use crate::machinst::CompiledCode;
use crate::nan_canonicalization::do_nan_canonicalization;
use crate::remove_constant_phis::do_remove_constant_phis;
use crate::result::CodegenResult;
use crate::result::{CodegenResult, CompileResult};
use crate::settings::{FlagsOrIsa, OptLevel};
use crate::simple_gvn::do_simple_gvn;
use crate::simple_preopt::do_preopt;
use crate::timing;
use crate::unreachable_code::eliminate_unreachable_code;
use crate::verifier::{verify_context, VerifierErrors, VerifierResult};
use crate::{timing, CompileError};
#[cfg(feature = "souper-harvest")]
use alloc::string::String;
use alloc::vec::Vec;
@@ -51,9 +50,9 @@ pub struct Context {
pub loop_analysis: LoopAnalysis,
/// Result of MachBackend compilation, if computed.
pub mach_compile_result: Option<MachCompileResult>,
compiled_code: Option<CompiledCode>,
/// Flag: do we want a disassembly with the MachCompileResult?
/// Flag: do we want a disassembly with the CompiledCode?
pub want_disasm: bool,
}
@@ -76,7 +75,7 @@ impl Context {
cfg: ControlFlowGraph::new(),
domtree: DominatorTree::new(),
loop_analysis: LoopAnalysis::new(),
mach_compile_result: None,
compiled_code: None,
want_disasm: false,
}
}
@@ -87,10 +86,16 @@ impl Context {
self.cfg.clear();
self.domtree.clear();
self.loop_analysis.clear();
self.mach_compile_result = None;
self.compiled_code = None;
self.want_disasm = false;
}
/// Returns the compilation result for this function, available after any `compile` function
/// has been called.
pub fn compiled_code(&self) -> Option<&CompiledCode> {
self.compiled_code.as_ref()
}
/// Set the flag to request a disassembly when compiling with a
/// `MachBackend` backend.
pub fn set_disasm(&mut self, val: bool) {
@@ -102,7 +107,7 @@ impl Context {
/// Run the function through all the passes necessary to generate code for the target ISA
/// represented by `isa`, as well as the final step of emitting machine code into a
/// `Vec<u8>`. The machine code is not relocated. Instead, any relocations can be obtained
/// from `mach_compile_result`.
/// from `compiled_code()`.
///
/// This function calls `compile` and `emit_to_memory`, taking care to resize `mem` as
/// needed, so it provides a safe interface.
@@ -112,13 +117,13 @@ impl Context {
&mut self,
isa: &dyn TargetIsa,
mem: &mut Vec<u8>,
) -> CodegenResult<()> {
let info = self.compile(isa)?;
) -> CompileResult<&CompiledCode> {
let compiled_code = self.compile(isa)?;
let code_info = compiled_code.code_info();
let old_len = mem.len();
mem.resize(old_len + info.total_size as usize, 0);
let new_info = unsafe { self.emit_to_memory(mem.as_mut_ptr().add(old_len)) };
debug_assert!(new_info == info);
Ok(())
mem.resize(old_len + code_info.total_size as usize, 0);
mem[old_len..].copy_from_slice(compiled_code.code_buffer());
Ok(compiled_code)
}
/// Compile the function.
@@ -128,86 +133,66 @@ impl Context {
/// code sink.
///
/// Returns information about the function's code and read-only data.
pub fn compile(&mut self, isa: &dyn TargetIsa) -> CodegenResult<CodeInfo> {
pub fn compile(&mut self, isa: &dyn TargetIsa) -> CompileResult<&CompiledCode> {
let _tt = timing::compile();
self.verify_if(isa)?;
let opt_level = isa.flags().opt_level();
log::trace!(
"Compiling (opt level {:?}):\n{}",
opt_level,
self.func.display()
);
let mut inner = || {
self.verify_if(isa)?;
self.compute_cfg();
if opt_level != OptLevel::None {
self.preopt(isa)?;
}
if isa.flags().enable_nan_canonicalization() {
self.canonicalize_nans(isa)?;
}
let opt_level = isa.flags().opt_level();
log::trace!(
"Compiling (opt level {:?}):\n{}",
opt_level,
self.func.display()
);
self.compute_cfg();
if opt_level != OptLevel::None {
self.preopt(isa)?;
}
if isa.flags().enable_nan_canonicalization() {
self.canonicalize_nans(isa)?;
}
self.legalize(isa)?;
if opt_level != OptLevel::None {
self.compute_domtree();
self.compute_loop_analysis();
self.licm(isa)?;
self.simple_gvn(isa)?;
}
self.legalize(isa)?;
if opt_level != OptLevel::None {
self.compute_domtree();
self.compute_loop_analysis();
self.licm(isa)?;
self.simple_gvn(isa)?;
}
self.eliminate_unreachable_code(isa)?;
if opt_level != OptLevel::None {
self.dce(isa)?;
}
self.compute_domtree();
self.eliminate_unreachable_code(isa)?;
if opt_level != OptLevel::None {
self.dce(isa)?;
}
self.remove_constant_phis(isa)?;
self.remove_constant_phis(isa)?;
if opt_level != OptLevel::None && isa.flags().enable_alias_analysis() {
self.replace_redundant_loads()?;
self.simple_gvn(isa)?;
}
if opt_level != OptLevel::None && isa.flags().enable_alias_analysis() {
self.replace_redundant_loads()?;
self.simple_gvn(isa)?;
}
let result = isa.compile_function(&self.func, self.want_disasm)?;
self.compiled_code = Some(result);
Ok(())
};
let result = isa.compile_function(&self.func, self.want_disasm)?;
let info = result.code_info();
self.mach_compile_result = Some(result);
Ok(info)
}
/// Emit machine code directly into raw memory.
///
/// Write all of the function's machine code to the memory at `mem`. The size of the machine
/// code is returned by `compile` above.
///
/// The machine code is not relocated.
/// Instead, any relocations can be obtained from `mach_compile_result`.
///
/// # Safety
///
/// This function is unsafe since it does not perform bounds checking on the memory buffer,
/// and it can't guarantee that the `mem` pointer is valid.
///
/// Returns information about the emitted code and data.
#[deny(unsafe_op_in_unsafe_fn)]
pub unsafe fn emit_to_memory(&self, mem: *mut u8) -> CodeInfo {
let _tt = timing::binemit();
let result = self
.mach_compile_result
.as_ref()
.expect("only using mach backend now");
let info = result.code_info();
let mem = unsafe { std::slice::from_raw_parts_mut(mem, info.total_size as usize) };
mem.copy_from_slice(result.buffer.data());
info
inner()
.map(|_| self.compiled_code.as_ref().unwrap())
.map_err(|error| CompileError {
inner: error,
func: &self.func,
})
}
/// If available, return information about the code layout in the
/// final machine code: the offsets (in bytes) of each basic-block
/// start, and all basic-block edges.
pub fn get_code_bb_layout(&self) -> Option<(Vec<usize>, Vec<(usize, usize)>)> {
if let Some(result) = self.mach_compile_result.as_ref() {
if let Some(result) = self.compiled_code.as_ref() {
Some((
result.bb_starts.iter().map(|&off| off as usize).collect(),
result
@@ -230,7 +215,7 @@ impl Context {
isa: &dyn TargetIsa,
) -> CodegenResult<Option<crate::isa::unwind::UnwindInfo>> {
let unwind_info_kind = isa.unwind_info_kind();
let result = self.mach_compile_result.as_ref().unwrap();
let result = self.compiled_code.as_ref().unwrap();
isa.emit_unwind_info(result, unwind_info_kind)
}

12
cranelift/codegen/src/isa/aarch64/mod.rs
View File

@@ -7,7 +7,7 @@ use crate::isa::aarch64::settings as aarch64_settings;
use crate::isa::unwind::systemv;
use crate::isa::{Builder as IsaBuilder, TargetIsa};
use crate::machinst::{
compile, MachCompileResult, MachTextSectionBuilder, Reg, TextSectionBuilder, VCode,
compile, CompiledCode, MachTextSectionBuilder, Reg, TextSectionBuilder, VCode,
};
use crate::result::CodegenResult;
use crate::settings as shared_settings;
@@ -65,11 +65,7 @@ impl AArch64Backend {
}
impl TargetIsa for AArch64Backend {
fn compile_function(
&self,
func: &Function,
want_disasm: bool,
) -> CodegenResult<MachCompileResult> {
fn compile_function(&self, func: &Function, want_disasm: bool) -> CodegenResult<CompiledCode> {
let flags = self.flags();
let (vcode, regalloc_result) = self.compile_vcode(func, flags.clone())?;
@@ -84,7 +80,7 @@ impl TargetIsa for AArch64Backend {
log::debug!("disassembly:\n{}", disasm);
}
Ok(MachCompileResult {
Ok(CompiledCode {
buffer,
frame_size,
disasm: emit_result.disasm,
@@ -125,7 +121,7 @@ impl TargetIsa for AArch64Backend {
#[cfg(feature = "unwind")]
fn emit_unwind_info(
&self,
result: &MachCompileResult,
result: &CompiledCode,
kind: crate::machinst::UnwindInfoKind,
) -> CodegenResult<Option<crate::isa::unwind::UnwindInfo>> {
use crate::isa::unwind::UnwindInfo;

10
cranelift/codegen/src/isa/mod.rs
View File

@@ -49,7 +49,7 @@ use crate::flowgraph;
use crate::ir::{self, Function};
#[cfg(feature = "unwind")]
use crate::isa::unwind::systemv::RegisterMappingError;
use crate::machinst::{MachCompileResult, TextSectionBuilder, UnwindInfoKind};
use crate::machinst::{CompiledCode, TextSectionBuilder, UnwindInfoKind};
use crate::settings;
use crate::settings::SetResult;
use crate::CodegenResult;
@@ -230,11 +230,7 @@ pub trait TargetIsa: fmt::Display + Send + Sync {
fn dynamic_vector_bytes(&self, dynamic_ty: ir::Type) -> u32;
/// Compile the given function.
fn compile_function(
&self,
func: &Function,
want_disasm: bool,
) -> CodegenResult<MachCompileResult>;
fn compile_function(&self, func: &Function, want_disasm: bool) -> CodegenResult<CompiledCode>;
#[cfg(feature = "unwind")]
/// Map a regalloc::Reg to its corresponding DWARF register.
@@ -254,7 +250,7 @@ pub trait TargetIsa: fmt::Display + Send + Sync {
#[cfg(feature = "unwind")]
fn emit_unwind_info(
&self,
result: &MachCompileResult,
result: &CompiledCode,
kind: UnwindInfoKind,
) -> CodegenResult<Option<crate::isa::unwind::UnwindInfo>>;

12
cranelift/codegen/src/isa/s390x/mod.rs
View File

@@ -7,7 +7,7 @@ use crate::isa::s390x::settings as s390x_settings;
use crate::isa::unwind::systemv::RegisterMappingError;
use crate::isa::{Builder as IsaBuilder, TargetIsa};
use crate::machinst::{
compile, MachCompileResult, MachTextSectionBuilder, Reg, TextSectionBuilder, VCode,
compile, CompiledCode, MachTextSectionBuilder, Reg, TextSectionBuilder, VCode,
};
use crate::result::CodegenResult;
use crate::settings as shared_settings;
@@ -64,11 +64,7 @@ impl S390xBackend {
}
impl TargetIsa for S390xBackend {
fn compile_function(
&self,
func: &Function,
want_disasm: bool,
) -> CodegenResult<MachCompileResult> {
fn compile_function(&self, func: &Function, want_disasm: bool) -> CodegenResult<CompiledCode> {
let flags = self.flags();
let (vcode, regalloc_result) = self.compile_vcode(func, flags.clone())?;
@@ -83,7 +79,7 @@ impl TargetIsa for S390xBackend {
log::debug!("disassembly:\n{}", disasm);
}
Ok(MachCompileResult {
Ok(CompiledCode {
buffer,
frame_size,
disasm: emit_result.disasm,
@@ -127,7 +123,7 @@ impl TargetIsa for S390xBackend {
#[cfg(feature = "unwind")]
fn emit_unwind_info(
&self,
result: &MachCompileResult,
result: &CompiledCode,
kind: crate::machinst::UnwindInfoKind,
) -> CodegenResult<Option<crate::isa::unwind::UnwindInfo>> {
use crate::isa::unwind::UnwindInfo;

14
cranelift/codegen/src/isa/x64/mod.rs
View File

@@ -9,9 +9,7 @@ use crate::isa::unwind::systemv;
use crate::isa::x64::{inst::regs::create_reg_env_systemv, settings as x64_settings};
use crate::isa::Builder as IsaBuilder;
use crate::machinst::Reg;
use crate::machinst::{
compile, MachCompileResult, MachTextSectionBuilder, TextSectionBuilder, VCode,
};
use crate::machinst::{compile, CompiledCode, MachTextSectionBuilder, TextSectionBuilder, VCode};
use crate::result::{CodegenError, CodegenResult};
use crate::settings::{self as shared_settings, Flags};
use alloc::{boxed::Box, vec::Vec};
@@ -59,11 +57,7 @@ impl X64Backend {
}
impl TargetIsa for X64Backend {
fn compile_function(
&self,
func: &Function,
want_disasm: bool,
) -> CodegenResult<MachCompileResult> {
fn compile_function(&self, func: &Function, want_disasm: bool) -> CodegenResult<CompiledCode> {
let flags = self.flags();
let (vcode, regalloc_result) = self.compile_vcode(func, flags.clone())?;
@@ -78,7 +72,7 @@ impl TargetIsa for X64Backend {
log::trace!("disassembly:\n{}", disasm);
}
Ok(MachCompileResult {
Ok(CompiledCode {
buffer,
frame_size,
disasm: emit_result.disasm,
@@ -119,7 +113,7 @@ impl TargetIsa for X64Backend {
#[cfg(feature = "unwind")]
fn emit_unwind_info(
&self,
result: &MachCompileResult,
result: &CompiledCode,
kind: crate::machinst::UnwindInfoKind,
) -> CodegenResult<Option<crate::isa::unwind::UnwindInfo>> {
use crate::isa::unwind::UnwindInfo;

2
cranelift/codegen/src/lib.rs
View File

@@ -114,7 +114,7 @@ mod value_label;
#[cfg(feature = "souper-harvest")]
mod souper_harvest;
pub use crate::result::{CodegenError, CodegenResult};
pub use crate::result::{CodegenError, CodegenResult, CompileError};
/// Even when trace logging is disabled, the trace macro has a significant performance cost so we
/// disable it by default.

9
cranelift/codegen/src/machinst/mod.rs
View File

@@ -272,7 +272,7 @@ pub trait MachInstEmitState<I: MachInst>: Default + Clone + Debug {
/// The result of a `MachBackend::compile_function()` call. Contains machine
/// code (as bytes) and a disassembly, if requested.
pub struct MachCompileResult {
pub struct CompiledCode {
/// Machine code.
pub buffer: MachBufferFinalized,
/// Size of stack frame, in bytes.
@@ -299,13 +299,18 @@ pub struct MachCompileResult {
pub bb_edges: Vec<(CodeOffset, CodeOffset)>,
}
impl MachCompileResult {
impl CompiledCode {
/// Get a `CodeInfo` describing section sizes from this compilation result.
pub fn code_info(&self) -> CodeInfo {
CodeInfo {
total_size: self.buffer.total_size(),
}
}
/// Returns a reference to the machine code generated for this function compilation.
pub fn code_buffer(&self) -> &[u8] {
self.buffer.data()
}
}
/// An object that can be used to create the text section of an executable.

21
cranelift/codegen/src/result.rs
View File

@@ -2,7 +2,7 @@
use regalloc2::checker::CheckerErrors;
use crate::verifier::VerifierErrors;
use crate::{ir::Function, verifier::VerifierErrors};
use std::string::String;
/// A compilation error.
@@ -81,3 +81,22 @@ impl From<VerifierErrors> for CodegenError {
CodegenError::Verifier { 0: source }
}
}
/// Compilation error, with the accompanying function to help printing it.
pub struct CompileError<'a> {
/// Underlying `CodegenError` that triggered the error.
pub inner: CodegenError,
/// Function we tried to compile, for display purposes.
pub func: &'a Function,
}
// By default, have `CompileError` be displayed as the internal error, and let consumers care if
// they want to use the func field for adding details.
impl<'a> core::fmt::Debug for CompileError<'a> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
self.inner.fmt(f)
}
}
/// A convenient alias for a `Result` that uses `CompileError` as the error type.
pub type CompileResult<'a, T> = Result<T, CompileError<'a>>;

1
cranelift/codegen/src/timing.rs
View File

@@ -68,7 +68,6 @@ define_passes! {
regalloc: "Register allocation",
regalloc_checker: "Register allocation symbolic verification",
binemit: "Binary machine code emission",
layout_renumber: "Layout full renumbering",
canonicalize_nans: "Canonicalization of NaNs",