Skip to content

Commit

Permalink
Allow serializing all cranelift-module data structures (bytecodeallia…
Browse files Browse the repository at this point in the history
…nce#6172)

* Remove ModuleCompiledFunction

The same information can be retrieved using

ctx.compiled_code().unwrap().code_info().total_size

In addition for Module implementations that don't immediately compile the
given function there is no correct value that can be returned.

* Don't give anonymous functions and data objects an internal name

This internal name can conflict if a module is serialized and then
deserialized into another module. It also wasn't used by any of the
Module implementations anyway.

* Allow serializing all cranelift-module data structures

This allows a Module implementation to serialize it's internal state and
deserialize it in another compilation session. For example to implement
LTO or to load the module into cranelift-interpreter.

* Use expect
  • Loading branch information
bjorn3 authored and afonso360 committed Apr 24, 2023
1 parent c8fcf23 commit 1ce2d3c
Show file tree
Hide file tree
Showing 8 changed files with 359 additions and 100 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cranelift/codegen/src/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use super::{RelSourceLoc, SourceLoc, UserExternalName};

/// A version marker used to ensure that serialized clif ir is never deserialized with a
/// different version of Cranelift.
#[derive(Copy, Clone, Debug, PartialEq, Hash)]
#[derive(Default, Copy, Clone, Debug, PartialEq, Hash)]
pub struct VersionMarker;

#[cfg(feature = "enable-serde")]
Expand Down
65 changes: 39 additions & 26 deletions cranelift/jit/src/backend.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
//! Defines `JITModule`.
use crate::{compiled_blob::CompiledBlob, memory::BranchProtection, memory::Memory};
use cranelift_codegen::binemit::Reloc;
use cranelift_codegen::isa::{OwnedTargetIsa, TargetIsa};
use cranelift_codegen::settings::Configurable;
use cranelift_codegen::{self, ir, settings, MachReloc};
use cranelift_codegen::{binemit::Reloc, CodegenError};
use cranelift_control::ControlPlane;
use cranelift_entity::SecondaryMap;
use cranelift_module::{
DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleCompiledFunction,
ModuleDeclarations, ModuleError, ModuleExtName, ModuleReloc, ModuleResult,
DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleDeclarations, ModuleError,
ModuleExtName, ModuleReloc, ModuleResult,
};
use log::info;
use std::cell::RefCell;
Expand Down Expand Up @@ -272,7 +272,10 @@ impl JITModule {
self.record_function_for_perf(
plt_entry.as_ptr().cast(),
std::mem::size_of::<[u8; 16]>(),
&format!("{}@plt", self.declarations.get_function_decl(id).name),
&format!(
"{}@plt",
self.declarations.get_function_decl(id).linkage_name(id)
),
);
self.function_plt_entries[id] = Some(plt_entry);
}
Expand Down Expand Up @@ -323,6 +326,9 @@ impl JITModule {
}
}
};
let name = name
.as_ref()
.expect("anonymous symbol must be defined locally");
if let Some(ptr) = self.lookup_symbol(name) {
ptr
} else if linkage == Linkage::Preemptible {
Expand Down Expand Up @@ -554,13 +560,15 @@ impl JITModule {
assert!(self.hotswap_enabled, "Hotswap support is not enabled");
let decl = self.declarations.get_function_decl(func_id);
if !decl.linkage.is_definable() {
return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
return Err(ModuleError::InvalidImportDefinition(
decl.linkage_name(func_id).into_owned(),
));
}

if self.compiled_functions[func_id].is_none() {
return Err(ModuleError::Backend(anyhow::anyhow!(
"Tried to redefine not yet defined function {}",
decl.name
decl.linkage_name(func_id),
)));
}

Expand Down Expand Up @@ -647,15 +655,19 @@ impl Module for JITModule {
id: FuncId,
ctx: &mut cranelift_codegen::Context,
ctrl_plane: &mut ControlPlane,
) -> ModuleResult<ModuleCompiledFunction> {
) -> ModuleResult<()> {
info!("defining function {}: {}", id, ctx.func.display());
let decl = self.declarations.get_function_decl(id);
if !decl.linkage.is_definable() {
return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
return Err(ModuleError::InvalidImportDefinition(
decl.linkage_name(id).into_owned(),
));
}

if !self.compiled_functions[id].is_none() {
return Err(ModuleError::DuplicateDefinition(decl.name.to_owned()));
return Err(ModuleError::DuplicateDefinition(
decl.linkage_name(id).into_owned(),
));
}

if self.hotswap_enabled {
Expand All @@ -678,9 +690,7 @@ impl Module for JITModule {
let alignment = res.alignment as u64;
let compiled_code = ctx.compiled_code().unwrap();

let code_size = compiled_code.code_info().total_size;

let size = code_size as usize;
let size = compiled_code.code_info().total_size as usize;
let align = alignment
.max(self.isa.function_alignment() as u64)
.max(self.isa.symbol_alignment());
Expand All @@ -705,7 +715,7 @@ impl Module for JITModule {
.map(|reloc| ModuleReloc::from_mach_reloc(reloc, &ctx.func))
.collect();

self.record_function_for_perf(ptr, size, &decl.name);
self.record_function_for_perf(ptr, size, &decl.linkage_name(id));
self.compiled_functions[id] = Some(CompiledBlob { ptr, size, relocs });

if self.isa.flags().is_pic() {
Expand Down Expand Up @@ -739,7 +749,7 @@ impl Module for JITModule {
self.functions_to_finalize.push(id);
}

Ok(ModuleCompiledFunction { size: code_size })
Ok(())
}

fn define_function_bytes(
Expand All @@ -749,20 +759,19 @@ impl Module for JITModule {
alignment: u64,
bytes: &[u8],
relocs: &[MachReloc],
) -> ModuleResult<ModuleCompiledFunction> {
) -> ModuleResult<()> {
info!("defining function {} with bytes", id);
let total_size: u32 = match bytes.len().try_into() {
Ok(total_size) => total_size,
_ => Err(CodegenError::CodeTooLarge)?,
};

let decl = self.declarations.get_function_decl(id);
if !decl.linkage.is_definable() {
return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
return Err(ModuleError::InvalidImportDefinition(
decl.linkage_name(id).into_owned(),
));
}

if !self.compiled_functions[id].is_none() {
return Err(ModuleError::DuplicateDefinition(decl.name.to_owned()));
return Err(ModuleError::DuplicateDefinition(
decl.linkage_name(id).into_owned(),
));
}

let size = bytes.len();
Expand All @@ -782,7 +791,7 @@ impl Module for JITModule {
ptr::copy_nonoverlapping(bytes.as_ptr(), ptr, size);
}

self.record_function_for_perf(ptr, size, &decl.name);
self.record_function_for_perf(ptr, size, &decl.linkage_name(id));
self.compiled_functions[id] = Some(CompiledBlob {
ptr,
size,
Expand Down Expand Up @@ -812,17 +821,21 @@ impl Module for JITModule {
self.functions_to_finalize.push(id);
}

Ok(ModuleCompiledFunction { size: total_size })
Ok(())
}

fn define_data(&mut self, id: DataId, data: &DataDescription) -> ModuleResult<()> {
let decl = self.declarations.get_data_decl(id);
if !decl.linkage.is_definable() {
return Err(ModuleError::InvalidImportDefinition(decl.name.clone()));
return Err(ModuleError::InvalidImportDefinition(
decl.linkage_name(id).into_owned(),
));
}

if !self.compiled_data_objects[id].is_none() {
return Err(ModuleError::DuplicateDefinition(decl.name.to_owned()));
return Err(ModuleError::DuplicateDefinition(
decl.linkage_name(id).into_owned(),
));
}

assert!(!decl.tls, "JIT doesn't yet support TLS");
Expand Down
4 changes: 4 additions & 0 deletions cranelift/module/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@ cranelift-codegen = { workspace = true }
cranelift-control = { workspace = true }
hashbrown = { workspace = true, optional = true }
anyhow = { workspace = true }
serde = { version = "1.0.94", features = ["derive"], optional = true }

[features]
default = ["std"]
std = ["cranelift-codegen/std"]
core = ["hashbrown", "cranelift-codegen/core"]

# For dependent crates that want to serialize some parts of cranelift
enable-serde = ["serde", "cranelift-codegen/enable-serde"]

[badges]
maintenance = { status = "experimental" }
2 changes: 2 additions & 0 deletions cranelift/module/src/data_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::ModuleExtName;

/// This specifies how data is to be initialized.
#[derive(Clone, PartialEq, Eq, Debug)]
#[cfg_attr(feature = "enable-serde", derive(serde::Serialize, serde::Deserialize))]
pub enum Init {
/// This indicates that no initialization has been specified yet.
Uninitialized,
Expand Down Expand Up @@ -41,6 +42,7 @@ impl Init {

/// A description of a data object.
#[derive(Clone, Debug)]
#[cfg_attr(feature = "enable-serde", derive(serde::Serialize, serde::Deserialize))]
pub struct DataDescription {
/// How the data should be initialized.
pub init: Init,
Expand Down
3 changes: 1 addition & 2 deletions cranelift/module/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ mod traps;
pub use crate::data_context::{DataDescription, Init};
pub use crate::module::{
DataDeclaration, DataId, FuncId, FuncOrDataId, FunctionDeclaration, Linkage, Module,
ModuleCompiledFunction, ModuleDeclarations, ModuleError, ModuleExtName, ModuleReloc,
ModuleResult,
ModuleDeclarations, ModuleError, ModuleExtName, ModuleReloc, ModuleResult,
};
pub use crate::traps::TrapSite;

Expand Down
Loading

0 comments on commit 1ce2d3c

Please sign in to comment.