From b1187b5507a68af67444f5b0c08e719ee4b91320 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 12:29:22 +0200 Subject: [PATCH 01/17] Merge ModuleNamespace and ModuleContents --- cranelift/module/src/backend.rs | 14 ++--- cranelift/module/src/lib.rs | 2 +- cranelift/module/src/module.rs | 83 +++++++----------------------- cranelift/object/src/backend.rs | 24 ++++----- cranelift/simplejit/src/backend.rs | 26 +++++----- 5 files changed, 52 insertions(+), 97 deletions(-) diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs index ff1372208b..6602c4cbb7 100644 --- a/cranelift/module/src/backend.rs +++ b/cranelift/module/src/backend.rs @@ -4,7 +4,7 @@ use crate::DataContext; use crate::DataId; use crate::FuncId; use crate::Linkage; -use crate::ModuleNamespace; +use crate::ModuleContents; use crate::ModuleResult; use core::marker; use cranelift_codegen::isa::TargetIsa; @@ -79,7 +79,7 @@ where id: FuncId, name: &str, ctx: &Context, - namespace: &ModuleNamespace, + contents: &ModuleContents, code_size: u32, trap_sink: &mut TS, ) -> ModuleResult @@ -94,7 +94,7 @@ where id: FuncId, name: &str, bytes: &[u8], - namespace: &ModuleNamespace, + contents: &ModuleContents, ) -> ModuleResult; /// Define a zero-initialized data object of the given size. @@ -108,7 +108,7 @@ where tls: bool, align: Option, data_ctx: &DataContext, - namespace: &ModuleNamespace, + contents: &ModuleContents, ) -> ModuleResult; /// Write the address of `what` into the data for `data` at `offset`. `data` must refer to a @@ -139,7 +139,7 @@ where &mut self, id: FuncId, func: &Self::CompiledFunction, - namespace: &ModuleNamespace, + contents: &ModuleContents, ) -> Self::FinalizedFunction; /// Return the finalized artifact from the backend, if relevant. @@ -154,7 +154,7 @@ where &mut self, id: DataId, data: &Self::CompiledData, - namespace: &ModuleNamespace, + contents: &ModuleContents, ) -> Self::FinalizedData; /// Return the finalized artifact from the backend, if relevant. @@ -168,7 +168,7 @@ where /// Consume this `Backend` and return a result. Some implementations may /// provide additional functionality through this result. - fn finish(self, namespace: &ModuleNamespace) -> Self::Product; + fn finish(self, contents: &ModuleContents) -> Self::Product; } /// Default names for `ir::LibCall`s. A function by this name is imported into the object as diff --git a/cranelift/module/src/lib.rs b/cranelift/module/src/lib.rs index 25a2759be4..6331cfac1f 100644 --- a/cranelift/module/src/lib.rs +++ b/cranelift/module/src/lib.rs @@ -40,7 +40,7 @@ mod traps; pub use crate::backend::{default_libcall_names, Backend}; pub use crate::data_context::{DataContext, DataDescription, Init}; pub use crate::module::{ - DataId, FuncId, FuncOrDataId, Linkage, Module, ModuleError, ModuleFunction, ModuleNamespace, + DataId, FuncId, FuncOrDataId, Linkage, Module, ModuleContents, ModuleError, ModuleFunction, ModuleResult, }; pub use crate::traps::TrapSite; diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 3a6f90fc88..f27cac2012 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -228,8 +228,9 @@ where } } -/// The functions and data objects belonging to a module. -struct ModuleContents +/// This provides a view to the state of a module which allows `ir::ExternalName`s to be translated +/// into `FunctionDeclaration`s and `DataDeclaration`s. +pub struct ModuleContents where B: Backend, { @@ -241,7 +242,8 @@ impl ModuleContents where B: Backend, { - fn get_function_id(&self, name: &ir::ExternalName) -> FuncId { + /// Get the `FuncId` for the function named by `name`. + pub fn get_function_id(&self, name: &ir::ExternalName) -> FuncId { if let ir::ExternalName::User { namespace, index } = *name { debug_assert_eq!(namespace, 0); FuncId::from_u32(index) @@ -250,7 +252,8 @@ where } } - fn get_data_id(&self, name: &ir::ExternalName) -> DataId { + /// Get the `DataId` for the data object named by `name`. + pub fn get_data_id(&self, name: &ir::ExternalName) -> DataId { if let ir::ExternalName::User { namespace, index } = *name { debug_assert_eq!(namespace, 1); DataId::from_u32(index) @@ -259,47 +262,14 @@ where } } - fn get_function_info(&self, name: &ir::ExternalName) -> &ModuleFunction { - &self.functions[self.get_function_id(name)] - } - - /// Get the `DataDeclaration` for the function named by `name`. - fn get_data_info(&self, name: &ir::ExternalName) -> &ModuleData { - &self.data_objects[self.get_data_id(name)] - } -} - -/// This provides a view to the state of a module which allows `ir::ExternalName`s to be translated -/// into `FunctionDeclaration`s and `DataDeclaration`s. -pub struct ModuleNamespace<'a, B: 'a> -where - B: Backend, -{ - contents: &'a ModuleContents, -} - -impl<'a, B> ModuleNamespace<'a, B> -where - B: Backend, -{ - /// Get the `FuncId` for the function named by `name`. - pub fn get_function_id(&self, name: &ir::ExternalName) -> FuncId { - self.contents.get_function_id(name) - } - - /// Get the `DataId` for the data object named by `name`. - pub fn get_data_id(&self, name: &ir::ExternalName) -> DataId { - self.contents.get_data_id(name) - } - /// Get the `FunctionDeclaration` for the function named by `name`. pub fn get_function_decl(&self, name: &ir::ExternalName) -> &FunctionDeclaration { - &self.contents.get_function_info(name).decl + &self.functions[self.get_function_id(name)].decl } /// Get the `DataDeclaration` for the data object named by `name`. pub fn get_data_decl(&self, name: &ir::ExternalName) -> &DataDeclaration { - &self.contents.get_data_info(name).decl + &self.data_objects[self.get_data_id(name)].decl } /// Get the definition for the function named by `name`, along with its name @@ -308,7 +278,7 @@ where &self, name: &ir::ExternalName, ) -> (Option<&B::CompiledFunction>, &str, &ir::Signature) { - let info = self.contents.get_function_info(name); + let info = &self.functions[self.get_function_id(name)]; debug_assert!( !info.decl.linkage.is_definable() || info.compiled.is_some(), "Finalization requires a definition for function {}.", @@ -329,7 +299,7 @@ where &self, name: &ir::ExternalName, ) -> (Option<&B::CompiledData>, &str, bool) { - let info = self.contents.get_data_info(name); + let info = &self.data_objects[self.get_data_id(name)]; debug_assert!( !info.decl.linkage.is_definable() || info.compiled.is_some(), "Finalization requires a definition for data object {}.", @@ -594,9 +564,7 @@ where func, &info.decl.name, ctx, - &ModuleNamespace:: { - contents: &self.contents, - }, + &self.contents, total_size, trap_sink, )?; @@ -632,14 +600,9 @@ where _ => Err(ModuleError::FunctionTooLarge(info.decl.name.clone()))?, }; - let compiled = self.backend.define_function_bytes( - func, - &info.decl.name, - bytes, - &ModuleNamespace:: { - contents: &self.contents, - }, - )?; + let compiled = + self.backend + .define_function_bytes(func, &info.decl.name, bytes, &self.contents)?; self.contents.functions[func].compiled = Some(compiled); self.functions_to_finalize.push(func); @@ -663,9 +626,7 @@ where info.decl.tls, info.decl.align, data_ctx, - &ModuleNamespace:: { - contents: &self.contents, - }, + &self.contents, )?) }; self.contents.data_objects[data].compiled = compiled; @@ -734,9 +695,7 @@ where info.compiled .as_ref() .expect("function must be compiled before it can be finalized"), - &ModuleNamespace:: { - contents: &self.contents, - }, + &self.contents, ); } for data in self.data_objects_to_finalize.drain(..) { @@ -747,9 +706,7 @@ where info.compiled .as_ref() .expect("data object must be compiled before it can be finalized"), - &ModuleNamespace:: { - contents: &self.contents, - }, + &self.contents, ); } self.backend.publish(); @@ -792,8 +749,6 @@ where /// implementations may provide additional functionality available after /// a `Module` is complete. pub fn finish(self) -> B::Product { - self.backend.finish(&ModuleNamespace:: { - contents: &self.contents, - }) + self.backend.finish(&self.contents) } } diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index 3cceda4d88..31ec135d24 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -9,7 +9,7 @@ use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::{self, binemit, ir}; use cranelift_module::{ Backend, DataContext, DataDescription, DataId, FuncId, Init, Linkage, ModuleError, - ModuleNamespace, ModuleResult, + ModuleContents, ModuleResult, }; use object::write::{ Object, Relocation, SectionId, StandardSection, Symbol, SymbolId, SymbolSection, @@ -218,7 +218,7 @@ impl Backend for ObjectBackend { func_id: FuncId, _name: &str, ctx: &cranelift_codegen::Context, - _namespace: &ModuleNamespace, + _contents: &ModuleContents, code_size: u32, trap_sink: &mut TS, ) -> ModuleResult @@ -275,7 +275,7 @@ impl Backend for ObjectBackend { func_id: FuncId, _name: &str, bytes: &[u8], - _namespace: &ModuleNamespace, + _contents: &ModuleContents, ) -> ModuleResult { let symbol = self.functions[func_id].unwrap(); @@ -307,7 +307,7 @@ impl Backend for ObjectBackend { tls: bool, align: Option, data_ctx: &DataContext, - _namespace: &ModuleNamespace, + _contents: &ModuleContents, ) -> ModuleResult { let &DataDescription { ref init, @@ -428,7 +428,7 @@ impl Backend for ObjectBackend { &mut self, _id: FuncId, _func: &ObjectCompiledFunction, - _namespace: &ModuleNamespace, + _contents: &ModuleContents, ) { // Nothing to do. } @@ -441,7 +441,7 @@ impl Backend for ObjectBackend { &mut self, _id: DataId, _data: &ObjectCompiledData, - _namespace: &ModuleNamespace, + _contents: &ModuleContents, ) { // Nothing to do. } @@ -454,7 +454,7 @@ impl Backend for ObjectBackend { // Nothing to do. } - fn finish(mut self, namespace: &ModuleNamespace) -> ObjectProduct { + fn finish(mut self, contents: &ModuleContents) -> ObjectProduct { let mut symbol_relocs = Vec::new(); mem::swap(&mut symbol_relocs, &mut self.relocs); for symbol in symbol_relocs { @@ -467,7 +467,7 @@ impl Backend for ObjectBackend { addend, } in &symbol.relocs { - let target_symbol = self.get_symbol(namespace, name); + let target_symbol = self.get_symbol(contents, name); self.object .add_relocation( symbol.section, @@ -506,16 +506,16 @@ impl ObjectBackend { // symbols for missing libcalls. fn get_symbol( &mut self, - namespace: &ModuleNamespace, + contents: &ModuleContents, name: &ir::ExternalName, ) -> SymbolId { match *name { ir::ExternalName::User { .. } => { - if namespace.is_function(name) { - let id = namespace.get_function_id(name); + if contents.is_function(name) { + let id = contents.get_function_id(name); self.functions[id].unwrap() } else { - let id = namespace.get_data_id(name); + let id = contents.get_data_id(name); self.data_objects[id].unwrap() } } diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 6e21e7f154..c790a6bf01 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -8,7 +8,7 @@ use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::settings::Configurable; use cranelift_codegen::{self, ir, settings}; use cranelift_module::{ - Backend, DataContext, DataDescription, DataId, FuncId, Init, Linkage, ModuleNamespace, + Backend, DataContext, DataDescription, DataId, FuncId, Init, Linkage, ModuleContents, ModuleResult, }; use cranelift_native; @@ -170,19 +170,19 @@ impl SimpleJITBackend { fn get_definition( &self, - namespace: &ModuleNamespace, + contents: &ModuleContents, name: &ir::ExternalName, ) -> *const u8 { match *name { ir::ExternalName::User { .. } => { - if namespace.is_function(name) { - let (def, name_str, _signature) = namespace.get_function_definition(&name); + if contents.is_function(name) { + let (def, name_str, _signature) = contents.get_function_definition(&name); match def { Some(compiled) => compiled.code, None => self.lookup_symbol(name_str), } } else { - let (def, name_str, _writable) = namespace.get_data_definition(&name); + let (def, name_str, _writable) = contents.get_data_definition(&name); match def { Some(compiled) => compiled.storage, None => self.lookup_symbol(name_str), @@ -281,7 +281,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { _id: FuncId, name: &str, ctx: &cranelift_codegen::Context, - _namespace: &ModuleNamespace, + _contents: &ModuleContents, code_size: u32, trap_sink: &mut TS, ) -> ModuleResult @@ -321,7 +321,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { _id: FuncId, name: &str, bytes: &[u8], - _namespace: &ModuleNamespace, + _contents: &ModuleContents, ) -> ModuleResult { let size = bytes.len(); let ptr = self @@ -351,7 +351,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { tls: bool, align: Option, data: &DataContext, - _namespace: &ModuleNamespace, + _contents: &ModuleContents, ) -> ModuleResult { assert!(!tls, "SimpleJIT doesn't yet support TLS"); @@ -443,7 +443,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { &mut self, _id: FuncId, func: &Self::CompiledFunction, - namespace: &ModuleNamespace, + contents: &ModuleContents, ) -> Self::FinalizedFunction { use std::ptr::write_unaligned; @@ -457,7 +457,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { let ptr = func.code; debug_assert!((offset as usize) < func.size); let at = unsafe { ptr.offset(offset as isize) }; - let base = self.get_definition(namespace, name); + let base = self.get_definition(contents, name); // TODO: Handle overflow. let what = unsafe { base.offset(addend as isize) }; match reloc { @@ -497,7 +497,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { &mut self, _id: DataId, data: &Self::CompiledData, - namespace: &ModuleNamespace, + contents: &ModuleContents, ) -> Self::FinalizedData { use std::ptr::write_unaligned; @@ -511,7 +511,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { let ptr = data.storage; debug_assert!((offset as usize) < data.size); let at = unsafe { ptr.offset(offset as isize) }; - let base = self.get_definition(namespace, name); + let base = self.get_definition(contents, name); // TODO: Handle overflow. let what = unsafe { base.offset(addend as isize) }; match reloc { @@ -555,7 +555,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { /// /// This method does not need to be called when access to the memory /// handle is not required. - fn finish(self, _namespace: &ModuleNamespace) -> Self::Product { + fn finish(self, _contents: &ModuleContents) -> Self::Product { self.memory } } From 6161acfba50d2c3d748f32d375282772f41c808a Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 12:33:12 +0200 Subject: [PATCH 02/17] Minor simplification --- cranelift/object/src/backend.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index 31ec135d24..be247f9cdd 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -455,8 +455,7 @@ impl Backend for ObjectBackend { } fn finish(mut self, contents: &ModuleContents) -> ObjectProduct { - let mut symbol_relocs = Vec::new(); - mem::swap(&mut symbol_relocs, &mut self.relocs); + let symbol_relocs = mem::take(&mut self.relocs); for symbol in symbol_relocs { for &RelocRecord { offset, From 7dcfb1b47bbeb93927c98740a76603479f0b3621 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 12:40:26 +0200 Subject: [PATCH 03/17] Move some error checking out of the define_* functions --- cranelift/module/src/module.rs | 41 +++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index f27cac2012..4e08afe37e 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -191,6 +191,16 @@ where } Ok(()) } + + fn validate_for_define(&self) -> ModuleResult<()> { + if self.compiled.is_some() { + return Err(ModuleError::DuplicateDefinition(self.decl.name.clone())); + } + if !self.decl.linkage.is_definable() { + return Err(ModuleError::InvalidImportDefinition(self.decl.name.clone())); + } + Ok(()) + } } /// Information about a data object which can be accessed. @@ -226,6 +236,16 @@ where "Can't change TLS data object to normal or in the opposite way", ); } + + fn validate_for_define(&self) -> ModuleResult<()> { + if self.compiled.is_some() { + return Err(ModuleError::DuplicateDefinition(self.decl.name.clone())); + } + if !self.decl.linkage.is_definable() { + return Err(ModuleError::InvalidImportDefinition(self.decl.name.clone())); + } + Ok(()) + } } /// This provides a view to the state of a module which allows `ir::ExternalName`s to be translated @@ -553,12 +573,7 @@ where ); let CodeInfo { total_size, .. } = ctx.compile(self.backend.isa())?; let info = &self.contents.functions[func]; - if info.compiled.is_some() { - return Err(ModuleError::DuplicateDefinition(info.decl.name.clone())); - } - if !info.decl.linkage.is_definable() { - return Err(ModuleError::InvalidImportDefinition(info.decl.name.clone())); - } + info.validate_for_define()?; let compiled = self.backend.define_function( func, @@ -588,12 +603,7 @@ where ) -> ModuleResult { info!("defining function {} with bytes", func); let info = &self.contents.functions[func]; - if info.compiled.is_some() { - return Err(ModuleError::DuplicateDefinition(info.decl.name.clone())); - } - if !info.decl.linkage.is_definable() { - return Err(ModuleError::InvalidImportDefinition(info.decl.name.clone())); - } + info.validate_for_define()?; let total_size: u32 = match bytes.len().try_into() { Ok(total_size) => total_size, @@ -613,12 +623,7 @@ where pub fn define_data(&mut self, data: DataId, data_ctx: &DataContext) -> ModuleResult<()> { let compiled = { let info = &self.contents.data_objects[data]; - if info.compiled.is_some() { - return Err(ModuleError::DuplicateDefinition(info.decl.name.clone())); - } - if !info.decl.linkage.is_definable() { - return Err(ModuleError::InvalidImportDefinition(info.decl.name.clone())); - } + info.validate_for_define()?; Some(self.backend.define_data( data, &info.decl.name, From 4483c3740a7f045b59648b5e37249c8e1fc66653 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 13:53:01 +0200 Subject: [PATCH 04/17] Remove get_finalized_* --- cranelift/module/src/backend.rs | 28 +++----- cranelift/module/src/module.rs | 30 +------- cranelift/object/src/backend.rs | 31 +++------ cranelift/simplejit/src/backend.rs | 106 +++++++++++++++++------------ cranelift/simplejit/src/lib.rs | 2 +- 5 files changed, 84 insertions(+), 113 deletions(-) diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs index 6602c4cbb7..e20416b920 100644 --- a/cranelift/module/src/backend.rs +++ b/cranelift/module/src/backend.rs @@ -1,6 +1,6 @@ //! Defines the `Backend` trait. -use crate::DataContext; +use crate::{DataContext, FuncOrDataId}; use crate::DataId; use crate::FuncId; use crate::Linkage; @@ -11,7 +11,7 @@ use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::Context; use cranelift_codegen::{binemit, ir}; -use std::borrow::ToOwned; +use std::{borrow::ToOwned, collections::HashMap}; use std::boxed::Box; use std::string::String; @@ -38,14 +38,6 @@ where /// The results of "compiling" a data object. type CompiledData; - /// The completed output artifact for a function, if this is meaningful for - /// the `Backend`. - type FinalizedFunction; - - /// The completed output artifact for a data object, if this is meaningful for - /// the `Backend`. - type FinalizedData; - /// This is an object returned by `Module`'s /// [`finish`](struct.Module.html#method.finish) function, /// if the `Backend` has a purpose for this. @@ -140,10 +132,7 @@ where id: FuncId, func: &Self::CompiledFunction, contents: &ModuleContents, - ) -> Self::FinalizedFunction; - - /// Return the finalized artifact from the backend, if relevant. - fn get_finalized_function(&self, func: &Self::CompiledFunction) -> Self::FinalizedFunction; + ); /// Perform all outstanding relocations on the given data object. This requires all /// `Local` and `Export` entities referenced to be defined. @@ -155,10 +144,7 @@ where id: DataId, data: &Self::CompiledData, contents: &ModuleContents, - ) -> Self::FinalizedData; - - /// Return the finalized artifact from the backend, if relevant. - fn get_finalized_data(&self, data: &Self::CompiledData) -> Self::FinalizedData; + ); /// "Publish" all finalized functions and data objects to their ultimate destinations. /// @@ -168,7 +154,11 @@ where /// Consume this `Backend` and return a result. Some implementations may /// provide additional functionality through this result. - fn finish(self, contents: &ModuleContents) -> Self::Product; + fn finish( + self, + names: HashMap, + contents: ModuleContents, + ) -> Self::Product; } /// Default names for `ir::LibCall`s. A function by this name is imported into the object as diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 4e08afe37e..78a6ae542d 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -717,34 +717,6 @@ where self.backend.publish(); } - /// Return the finalized artifact from the backend, if it provides one. - pub fn get_finalized_function(&mut self, func: FuncId) -> B::FinalizedFunction { - let info = &self.contents.functions[func]; - debug_assert!( - !self.functions_to_finalize.iter().any(|x| *x == func), - "function not yet finalized" - ); - self.backend.get_finalized_function( - info.compiled - .as_ref() - .expect("function must be compiled before it can be finalized"), - ) - } - - /// Return the finalized artifact from the backend, if it provides one. - pub fn get_finalized_data(&mut self, data: DataId) -> B::FinalizedData { - let info = &self.contents.data_objects[data]; - debug_assert!( - !self.data_objects_to_finalize.iter().any(|x| *x == data), - "data object not yet finalized" - ); - self.backend.get_finalized_data( - info.compiled - .as_ref() - .expect("data object must be compiled before it can be finalized"), - ) - } - /// Return the target isa pub fn isa(&self) -> &dyn isa::TargetIsa { self.backend.isa() @@ -754,6 +726,6 @@ where /// implementations may provide additional functionality available after /// a `Module` is complete. pub fn finish(self) -> B::Product { - self.backend.finish(&self.contents) + self.backend.finish(self.names, self.contents) } } diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index be247f9cdd..a2db3f99c7 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -8,8 +8,8 @@ use cranelift_codegen::entity::SecondaryMap; use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::{self, binemit, ir}; use cranelift_module::{ - Backend, DataContext, DataDescription, DataId, FuncId, Init, Linkage, ModuleError, - ModuleContents, ModuleResult, + Backend, DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, + ModuleContents, ModuleError, ModuleResult, }; use object::write::{ Object, Relocation, SectionId, StandardSection, Symbol, SymbolId, SymbolSection, @@ -127,11 +127,6 @@ impl Backend for ObjectBackend { type CompiledFunction = ObjectCompiledFunction; type CompiledData = ObjectCompiledData; - // There's no need to return individual artifacts; we're writing them into - // the output file instead. - type FinalizedFunction = (); - type FinalizedData = (); - type Product = ObjectProduct; /// Create a new `ObjectBackend` using the given Cranelift target. @@ -433,10 +428,6 @@ impl Backend for ObjectBackend { // Nothing to do. } - fn get_finalized_function(&self, _func: &ObjectCompiledFunction) { - // Nothing to do. - } - fn finalize_data( &mut self, _id: DataId, @@ -446,15 +437,15 @@ impl Backend for ObjectBackend { // Nothing to do. } - fn get_finalized_data(&self, _data: &ObjectCompiledData) { - // Nothing to do. - } - fn publish(&mut self) { // Nothing to do. } - fn finish(mut self, contents: &ModuleContents) -> ObjectProduct { + fn finish( + mut self, + _names: HashMap, + contents: ModuleContents, + ) -> ObjectProduct { let symbol_relocs = mem::take(&mut self.relocs); for symbol in symbol_relocs { for &RelocRecord { @@ -466,7 +457,7 @@ impl Backend for ObjectBackend { addend, } in &symbol.relocs { - let target_symbol = self.get_symbol(contents, name); + let target_symbol = self.get_symbol(&contents, name); self.object .add_relocation( symbol.section, @@ -503,11 +494,7 @@ impl Backend for ObjectBackend { impl ObjectBackend { // This should only be called during finish because it creates // symbols for missing libcalls. - fn get_symbol( - &mut self, - contents: &ModuleContents, - name: &ir::ExternalName, - ) -> SymbolId { + fn get_symbol(&mut self, contents: &ModuleContents, name: &ir::ExternalName) -> SymbolId { match *name { ir::ExternalName::User { .. } => { if contents.is_function(name) { diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index c790a6bf01..3a999ef221 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -8,8 +8,8 @@ use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::settings::Configurable; use cranelift_codegen::{self, ir, settings}; use cranelift_module::{ - Backend, DataContext, DataDescription, DataId, FuncId, Init, Linkage, ModuleContents, - ModuleResult, + Backend, DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, + ModuleContents, ModuleResult, }; use cranelift_native; #[cfg(not(windows))] @@ -154,12 +154,60 @@ pub struct SimpleJITCompiledData { } /// A handle to allow freeing memory allocated by the `Backend`. -pub struct SimpleJITMemoryHandle { +struct SimpleJITMemoryHandle { code: Memory, readonly: Memory, writable: Memory, } +/// A `SimpleJITProduct` allows looking up the addresses of all functions and data objects +/// defined in the original module. +pub struct SimpleJITProduct { + memory: SimpleJITMemoryHandle, + names: HashMap, + contents: ModuleContents, +} + +impl SimpleJITProduct { + /// Free memory allocated for code and data segments of compiled functions. + /// + /// # Safety + /// + /// Because this function invalidates any pointers retrived from the + /// corresponding module, it should only be used when none of the functions + /// from that module are currently executing and none of the `fn` pointers + /// are called afterwards. + pub unsafe fn free_memory(&mut self) { + self.memory.code.free_memory(); + self.memory.readonly.free_memory(); + self.memory.writable.free_memory(); + } + + /// Get the `FuncOrDataId` associated with the given name. + pub fn func_or_data_for_func(&self, name: &str) -> Option { + self.names.get(name).copied() + } + + /// Return the address of a function. + pub fn lookup_func(&self, func_id: FuncId) -> *const u8 { + self.contents + .get_function_definition(&func_id.into()) + .0 + .unwrap_or_else(|| panic!("{} is not defined", func_id)) + .code + } + + /// Return the address and size of a data object. + pub fn lookup_data(&self, data_id: DataId) -> (*const u8, usize) { + let data = self + .contents + .get_data_definition(&data_id.into()) + .0 + .unwrap_or_else(|| panic!("{} is not defined", data_id)); + (data.storage, data.size) + } +} + impl SimpleJITBackend { fn lookup_symbol(&self, name: &str) -> *const u8 { match self.symbols.get(name) { @@ -225,19 +273,11 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { type CompiledFunction = SimpleJITCompiledFunction; type CompiledData = SimpleJITCompiledData; - /// SimpleJIT emits code and data into memory, and provides raw pointers - /// to them. They are valid for the remainder of the program's life, unless - /// [`free_memory`] is used. - /// - /// [`free_memory`]: #method.free_memory - type FinalizedFunction = *const u8; - type FinalizedData = (*mut u8, usize); - /// SimpleJIT emits code and data into memory as it processes them, so it /// doesn't need to provide anything after the `Module` is complete. /// The handle object that is returned can optionally be used to free /// allocated memory if required. - type Product = SimpleJITMemoryHandle; + type Product = SimpleJITProduct; /// Create a new `SimpleJITBackend`. fn new(builder: SimpleJITBuilder) -> Self { @@ -444,7 +484,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { _id: FuncId, func: &Self::CompiledFunction, contents: &ModuleContents, - ) -> Self::FinalizedFunction { + ) { use std::ptr::write_unaligned; for &RelocRecord { @@ -486,11 +526,6 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { _ => unimplemented!(), } } - func.code - } - - fn get_finalized_function(&self, func: &Self::CompiledFunction) -> Self::FinalizedFunction { - func.code } fn finalize_data( @@ -498,7 +533,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { _id: DataId, data: &Self::CompiledData, contents: &ModuleContents, - ) -> Self::FinalizedData { + ) { use std::ptr::write_unaligned; for &RelocRecord { @@ -535,11 +570,6 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { _ => unimplemented!(), } } - (data.storage, data.size) - } - - fn get_finalized_data(&self, data: &Self::CompiledData) -> Self::FinalizedData { - (data.storage, data.size) } fn publish(&mut self) { @@ -555,8 +585,16 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { /// /// This method does not need to be called when access to the memory /// handle is not required. - fn finish(self, _contents: &ModuleContents) -> Self::Product { - self.memory + fn finish( + self, + names: HashMap, + contents: ModuleContents, + ) -> Self::Product { + SimpleJITProduct { + memory: self.memory, + names, + contents, + } } } @@ -603,22 +641,6 @@ fn lookup_with_dlsym(name: &str) -> *const u8 { } } -impl SimpleJITMemoryHandle { - /// Free memory allocated for code and data segments of compiled functions. - /// - /// # Safety - /// - /// Because this function invalidates any pointers retrived from the - /// corresponding module, it should only be used when none of the functions - /// from that module are currently executing and none of the`fn` pointers - /// are called afterwards. - pub unsafe fn free_memory(&mut self) { - self.code.free_memory(); - self.readonly.free_memory(); - self.writable.free_memory(); - } -} - struct SimpleJITRelocSink { pub relocs: Vec, } diff --git a/cranelift/simplejit/src/lib.rs b/cranelift/simplejit/src/lib.rs index d907964cff..705b8decfc 100644 --- a/cranelift/simplejit/src/lib.rs +++ b/cranelift/simplejit/src/lib.rs @@ -26,7 +26,7 @@ mod backend; mod memory; -pub use crate::backend::{SimpleJITBackend, SimpleJITBuilder}; +pub use crate::backend::{SimpleJITBackend, SimpleJITBuilder, SimpleJITProduct}; /// Version number of this crate. pub const VERSION: &str = env!("CARGO_PKG_VERSION"); From 7608749647d3fd4adbce405cdfdb1ebbca0ecb51 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 13:58:13 +0200 Subject: [PATCH 05/17] Merge finalize_definitions into finish --- cranelift/module/src/backend.rs | 6 ------ cranelift/module/src/module.rs | 31 +++++++++--------------------- cranelift/object/src/backend.rs | 4 ---- cranelift/simplejit/src/backend.rs | 12 +++++------- 4 files changed, 14 insertions(+), 39 deletions(-) diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs index e20416b920..74b2342e13 100644 --- a/cranelift/module/src/backend.rs +++ b/cranelift/module/src/backend.rs @@ -146,12 +146,6 @@ where contents: &ModuleContents, ); - /// "Publish" all finalized functions and data objects to their ultimate destinations. - /// - /// This method is not relevant for `Backend` implementations that do not provide - /// `Backend::FinalizedFunction` or `Backend::FinalizedData`. - fn publish(&mut self); - /// Consume this `Backend` and return a result. Some implementations may /// provide additional functionality through this result. fn finish( diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 78a6ae542d..18e1dc7347 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -682,16 +682,15 @@ where ); } - /// Finalize all functions and data objects that are defined but not yet finalized. - /// All symbols referenced in their bodies that are declared as needing a definition - /// must be defined by this point. - /// - /// Use `get_finalized_function` and `get_finalized_data` to obtain the final - /// artifacts. - /// - /// This method is not relevant for `Backend` implementations that do not provide - /// `Backend::FinalizedFunction` or `Backend::FinalizedData`. - pub fn finalize_definitions(&mut self) { + /// Return the target isa + pub fn isa(&self) -> &dyn isa::TargetIsa { + self.backend.isa() + } + + /// Consume the module and return the resulting `Product`. Some `Backend` + /// implementations may provide additional functionality available after + /// a `Module` is complete. + pub fn finish(mut self) -> B::Product { for func in self.functions_to_finalize.drain(..) { let info = &self.contents.functions[func]; debug_assert!(info.decl.linkage.is_definable()); @@ -714,18 +713,6 @@ where &self.contents, ); } - self.backend.publish(); - } - - /// Return the target isa - pub fn isa(&self) -> &dyn isa::TargetIsa { - self.backend.isa() - } - - /// Consume the module and return the resulting `Product`. Some `Backend` - /// implementations may provide additional functionality available after - /// a `Module` is complete. - pub fn finish(self) -> B::Product { self.backend.finish(self.names, self.contents) } } diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index a2db3f99c7..15ba44be7d 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -437,10 +437,6 @@ impl Backend for ObjectBackend { // Nothing to do. } - fn publish(&mut self) { - // Nothing to do. - } - fn finish( mut self, _names: HashMap, diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 3a999ef221..c78c283564 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -572,12 +572,6 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { } } - fn publish(&mut self) { - // Now that we're done patching, prepare the memory for execution! - self.memory.readonly.set_readonly(); - self.memory.code.set_readable_and_executable(); - } - /// SimpleJIT emits code and data into memory as it processes them. This /// method performs no additional processing, but returns a handle which /// allows freeing the allocated memory. Otherwise said memory is leaked @@ -586,10 +580,14 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { /// This method does not need to be called when access to the memory /// handle is not required. fn finish( - self, + mut self, names: HashMap, contents: ModuleContents, ) -> Self::Product { + // Now that we're done patching, prepare the memory for execution! + self.memory.readonly.set_readonly(); + self.memory.code.set_readable_and_executable(); + SimpleJITProduct { memory: self.memory, names, From 59f95083b14f045745e4a4991239af9dbd38bfb3 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 13:58:58 +0200 Subject: [PATCH 06/17] Remove write_data_funcaddr and write_data_dataaddr They are unimplemented by all backends --- cranelift/module/src/backend.rs | 19 ------------- cranelift/module/src/module.rs | 43 ------------------------------ cranelift/object/src/backend.rs | 21 +-------------- cranelift/simplejit/src/backend.rs | 19 ------------- 4 files changed, 1 insertion(+), 101 deletions(-) diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs index 74b2342e13..bf4f593a42 100644 --- a/cranelift/module/src/backend.rs +++ b/cranelift/module/src/backend.rs @@ -103,25 +103,6 @@ where contents: &ModuleContents, ) -> ModuleResult; - /// Write the address of `what` into the data for `data` at `offset`. `data` must refer to a - /// defined data object. - fn write_data_funcaddr( - &mut self, - data: &mut Self::CompiledData, - offset: usize, - what: ir::FuncRef, - ); - - /// Write the address of `what` plus `addend` into the data for `data` at `offset`. `data` must - /// refer to a defined data object. - fn write_data_dataaddr( - &mut self, - data: &mut Self::CompiledData, - offset: usize, - what: ir::GlobalValue, - addend: binemit::Addend, - ); - /// Perform all outstanding relocations on the given function. This requires all `Local` /// and `Export` entities referenced to be defined. /// diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 18e1dc7347..600cff9ec8 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -639,49 +639,6 @@ where Ok(()) } - /// Write the address of `what` into the data for `data` at `offset`. `data` must refer to a - /// defined data object. - pub fn write_data_funcaddr(&mut self, data: DataId, offset: usize, what: ir::FuncRef) { - let info = &mut self.contents.data_objects[data]; - debug_assert!( - info.decl.linkage.is_definable(), - "imported data cannot contain references" - ); - self.backend.write_data_funcaddr( - &mut info - .compiled - .as_mut() - .expect("`data` must refer to a defined data object"), - offset, - what, - ); - } - - /// Write the address of `what` plus `addend` into the data for `data` at `offset`. `data` must - /// refer to a defined data object. - pub fn write_data_dataaddr( - &mut self, - data: DataId, - offset: usize, - what: ir::GlobalValue, - addend: binemit::Addend, - ) { - let info = &mut self.contents.data_objects[data]; - debug_assert!( - info.decl.linkage.is_definable(), - "imported data cannot contain references" - ); - self.backend.write_data_dataaddr( - &mut info - .compiled - .as_mut() - .expect("`data` must refer to a defined data object"), - offset, - what, - addend, - ); - } - /// Return the target isa pub fn isa(&self) -> &dyn isa::TargetIsa { self.backend.isa() diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index 15ba44be7d..2177f81c27 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -6,7 +6,7 @@ use cranelift_codegen::binemit::{ }; use cranelift_codegen::entity::SecondaryMap; use cranelift_codegen::isa::TargetIsa; -use cranelift_codegen::{self, binemit, ir}; +use cranelift_codegen::{self, ir}; use cranelift_module::{ Backend, DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, ModuleContents, ModuleError, ModuleResult, @@ -400,25 +400,6 @@ impl Backend for ObjectBackend { Ok(ObjectCompiledData) } - fn write_data_funcaddr( - &mut self, - _data: &mut ObjectCompiledData, - _offset: usize, - _what: ir::FuncRef, - ) { - unimplemented!() - } - - fn write_data_dataaddr( - &mut self, - _data: &mut ObjectCompiledData, - _offset: usize, - _what: ir::GlobalValue, - _usize: binemit::Addend, - ) { - unimplemented!() - } - fn finalize_function( &mut self, _id: FuncId, diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index c78c283564..222cb81494 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -460,25 +460,6 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { }) } - fn write_data_funcaddr( - &mut self, - _data: &mut Self::CompiledData, - _offset: usize, - _what: ir::FuncRef, - ) { - unimplemented!(); - } - - fn write_data_dataaddr( - &mut self, - _data: &mut Self::CompiledData, - _offset: usize, - _what: ir::GlobalValue, - _usize: Addend, - ) { - unimplemented!(); - } - fn finalize_function( &mut self, _id: FuncId, From 405b9e28753db5d5681ddf6fb9099612ff64621e Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 14:20:39 +0200 Subject: [PATCH 07/17] Remove finalize_* from the Backend trait Instead let the `finish` method perform finalization --- cranelift/module/src/backend.rs | 24 ---- cranelift/module/src/module.rs | 40 ++---- cranelift/object/src/backend.rs | 18 --- cranelift/simplejit/src/backend.rs | 224 ++++++++++++++++------------- 4 files changed, 142 insertions(+), 164 deletions(-) diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs index bf4f593a42..0d4b5e8e5f 100644 --- a/cranelift/module/src/backend.rs +++ b/cranelift/module/src/backend.rs @@ -103,30 +103,6 @@ where contents: &ModuleContents, ) -> ModuleResult; - /// Perform all outstanding relocations on the given function. This requires all `Local` - /// and `Export` entities referenced to be defined. - /// - /// This method is not relevant for `Backend` implementations that do not provide - /// `Backend::FinalizedFunction`. - fn finalize_function( - &mut self, - id: FuncId, - func: &Self::CompiledFunction, - contents: &ModuleContents, - ); - - /// Perform all outstanding relocations on the given data object. This requires all - /// `Local` and `Export` entities referenced to be defined. - /// - /// This method is not relevant for `Backend` implementations that do not provide - /// `Backend::FinalizedData`. - fn finalize_data( - &mut self, - id: DataId, - data: &Self::CompiledData, - contents: &ModuleContents, - ); - /// Consume this `Backend` and return a result. Some implementations may /// provide additional functionality through this result. fn finish( diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 600cff9ec8..f95ee5e27e 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -213,14 +213,14 @@ pub struct DataDeclaration { } /// A data object belonging to a `Module`. -struct ModuleData +pub struct ModuleData where B: Backend, { /// The data object declaration. - decl: DataDeclaration, + pub decl: DataDeclaration, /// The "compiled" artifact, once it's available. - compiled: Option, + pub compiled: Option, } impl ModuleData @@ -282,11 +282,21 @@ where } } + /// Get the `ModuleFunction` for the given function. + pub fn get_function_info(&self, func_id: FuncId) -> &ModuleFunction { + &self.functions[func_id] + } + /// Get the `FunctionDeclaration` for the function named by `name`. pub fn get_function_decl(&self, name: &ir::ExternalName) -> &FunctionDeclaration { &self.functions[self.get_function_id(name)].decl } + /// Get the `ModuleData` for the given data object. + pub fn get_data_info(&self, data_id: DataId) -> &ModuleData { + &self.data_objects[data_id] + } + /// Get the `DataDeclaration` for the data object named by `name`. pub fn get_data_decl(&self, name: &ir::ExternalName) -> &DataDeclaration { &self.data_objects[self.get_data_id(name)].decl @@ -647,29 +657,7 @@ where /// Consume the module and return the resulting `Product`. Some `Backend` /// implementations may provide additional functionality available after /// a `Module` is complete. - pub fn finish(mut self) -> B::Product { - for func in self.functions_to_finalize.drain(..) { - let info = &self.contents.functions[func]; - debug_assert!(info.decl.linkage.is_definable()); - self.backend.finalize_function( - func, - info.compiled - .as_ref() - .expect("function must be compiled before it can be finalized"), - &self.contents, - ); - } - for data in self.data_objects_to_finalize.drain(..) { - let info = &self.contents.data_objects[data]; - debug_assert!(info.decl.linkage.is_definable()); - self.backend.finalize_data( - data, - info.compiled - .as_ref() - .expect("data object must be compiled before it can be finalized"), - &self.contents, - ); - } + pub fn finish(self) -> B::Product { self.backend.finish(self.names, self.contents) } } diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index 2177f81c27..64feb15b45 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -400,24 +400,6 @@ impl Backend for ObjectBackend { Ok(ObjectCompiledData) } - fn finalize_function( - &mut self, - _id: FuncId, - _func: &ObjectCompiledFunction, - _contents: &ModuleContents, - ) { - // Nothing to do. - } - - fn finalize_data( - &mut self, - _id: DataId, - _data: &ObjectCompiledData, - _contents: &ModuleContents, - ) { - // Nothing to do. - } - fn finish( mut self, _names: HashMap, diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 222cb81494..5a97ae8b5e 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -124,6 +124,8 @@ pub struct SimpleJITBackend { symbols: HashMap, libcall_names: Box String>, memory: SimpleJITMemoryHandle, + functions_to_finalize: Vec, + data_objects_to_finalize: Vec, } /// A record of a relocation to perform. @@ -261,6 +263,100 @@ impl SimpleJITBackend { let _ = writeln!(map_file, "{:x} {:x} {}", ptr as usize, size, name); } } + + + fn finalize_function( + &mut self, + _id: FuncId, + func: &SimpleJITCompiledFunction, + contents: &ModuleContents, + ) { + use std::ptr::write_unaligned; + + for &RelocRecord { + reloc, + offset, + ref name, + addend, + } in &func.relocs + { + let ptr = func.code; + debug_assert!((offset as usize) < func.size); + let at = unsafe { ptr.offset(offset as isize) }; + let base = self.get_definition(contents, name); + // TODO: Handle overflow. + let what = unsafe { base.offset(addend as isize) }; + match reloc { + Reloc::Abs4 => { + // TODO: Handle overflow. + #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] + unsafe { + write_unaligned(at as *mut u32, what as u32) + }; + } + Reloc::Abs8 => { + #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] + unsafe { + write_unaligned(at as *mut u64, what as u64) + }; + } + Reloc::X86PCRel4 | Reloc::X86CallPCRel4 => { + // TODO: Handle overflow. + let pcrel = ((what as isize) - (at as isize)) as i32; + #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] + unsafe { + write_unaligned(at as *mut i32, pcrel) + }; + } + Reloc::X86GOTPCRel4 | Reloc::X86CallPLTRel4 => panic!("unexpected PIC relocation"), + _ => unimplemented!(), + } + } + } + + fn finalize_data( + &mut self, + _id: DataId, + data: &SimpleJITCompiledData, + contents: &ModuleContents, + ) { + use std::ptr::write_unaligned; + + for &RelocRecord { + reloc, + offset, + ref name, + addend, + } in &data.relocs + { + let ptr = data.storage; + debug_assert!((offset as usize) < data.size); + let at = unsafe { ptr.offset(offset as isize) }; + let base = self.get_definition(contents, name); + // TODO: Handle overflow. + let what = unsafe { base.offset(addend as isize) }; + match reloc { + Reloc::Abs4 => { + // TODO: Handle overflow. + #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] + unsafe { + write_unaligned(at as *mut u32, what as u32) + }; + } + Reloc::Abs8 => { + #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] + unsafe { + write_unaligned(at as *mut u64, what as u64) + }; + } + Reloc::X86PCRel4 + | Reloc::X86CallPCRel4 + | Reloc::X86GOTPCRel4 + | Reloc::X86CallPLTRel4 => panic!("unexpected text relocation in data"), + _ => unimplemented!(), + } + } + } } impl<'simple_jit_backend> Backend for SimpleJITBackend { @@ -292,6 +388,8 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { symbols: builder.symbols, libcall_names: builder.libcall_names, memory, + functions_to_finalize: Vec::new(), + data_objects_to_finalize: Vec::new(), } } @@ -318,7 +416,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { fn define_function( &mut self, - _id: FuncId, + id: FuncId, name: &str, ctx: &cranelift_codegen::Context, _contents: &ModuleContents, @@ -328,6 +426,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { where TS: TrapSink, { + self.functions_to_finalize.push(id); let size = code_size as usize; let ptr = self .memory @@ -358,11 +457,12 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { fn define_function_bytes( &mut self, - _id: FuncId, + id: FuncId, name: &str, bytes: &[u8], _contents: &ModuleContents, ) -> ModuleResult { + self.functions_to_finalize.push(id); let size = bytes.len(); let ptr = self .memory @@ -385,7 +485,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { fn define_data( &mut self, - _id: DataId, + id: DataId, _name: &str, writable: bool, tls: bool, @@ -395,6 +495,8 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { ) -> ModuleResult { assert!(!tls, "SimpleJIT doesn't yet support TLS"); + self.data_objects_to_finalize.push(id); + let &DataDescription { ref init, ref function_decls, @@ -460,99 +562,6 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { }) } - fn finalize_function( - &mut self, - _id: FuncId, - func: &Self::CompiledFunction, - contents: &ModuleContents, - ) { - use std::ptr::write_unaligned; - - for &RelocRecord { - reloc, - offset, - ref name, - addend, - } in &func.relocs - { - let ptr = func.code; - debug_assert!((offset as usize) < func.size); - let at = unsafe { ptr.offset(offset as isize) }; - let base = self.get_definition(contents, name); - // TODO: Handle overflow. - let what = unsafe { base.offset(addend as isize) }; - match reloc { - Reloc::Abs4 => { - // TODO: Handle overflow. - #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] - unsafe { - write_unaligned(at as *mut u32, what as u32) - }; - } - Reloc::Abs8 => { - #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] - unsafe { - write_unaligned(at as *mut u64, what as u64) - }; - } - Reloc::X86PCRel4 | Reloc::X86CallPCRel4 => { - // TODO: Handle overflow. - let pcrel = ((what as isize) - (at as isize)) as i32; - #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] - unsafe { - write_unaligned(at as *mut i32, pcrel) - }; - } - Reloc::X86GOTPCRel4 | Reloc::X86CallPLTRel4 => panic!("unexpected PIC relocation"), - _ => unimplemented!(), - } - } - } - - fn finalize_data( - &mut self, - _id: DataId, - data: &Self::CompiledData, - contents: &ModuleContents, - ) { - use std::ptr::write_unaligned; - - for &RelocRecord { - reloc, - offset, - ref name, - addend, - } in &data.relocs - { - let ptr = data.storage; - debug_assert!((offset as usize) < data.size); - let at = unsafe { ptr.offset(offset as isize) }; - let base = self.get_definition(contents, name); - // TODO: Handle overflow. - let what = unsafe { base.offset(addend as isize) }; - match reloc { - Reloc::Abs4 => { - // TODO: Handle overflow. - #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] - unsafe { - write_unaligned(at as *mut u32, what as u32) - }; - } - Reloc::Abs8 => { - #[cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))] - unsafe { - write_unaligned(at as *mut u64, what as u64) - }; - } - Reloc::X86PCRel4 - | Reloc::X86CallPCRel4 - | Reloc::X86GOTPCRel4 - | Reloc::X86CallPLTRel4 => panic!("unexpected text relocation in data"), - _ => unimplemented!(), - } - } - } - /// SimpleJIT emits code and data into memory as it processes them. This /// method performs no additional processing, but returns a handle which /// allows freeing the allocated memory. Otherwise said memory is leaked @@ -565,6 +574,29 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { names: HashMap, contents: ModuleContents, ) -> Self::Product { + for func in std::mem::take(&mut self.functions_to_finalize) { + let info = contents.get_function_info(func); + debug_assert!(info.decl.linkage.is_definable()); + self.finalize_function( + func, + info.compiled + .as_ref() + .expect("function must be compiled before it can be finalized"), + &contents, + ); + } + for data in std::mem::take(&mut self.data_objects_to_finalize) { + let info = contents.get_data_info(data); + debug_assert!(info.decl.linkage.is_definable()); + self.finalize_data( + data, + info.compiled + .as_ref() + .expect("data object must be compiled before it can be finalized"), + &contents, + ); + } + // Now that we're done patching, prepare the memory for execution! self.memory.readonly.set_readonly(); self.memory.code.set_readable_and_executable(); From 588a4be0b31bf679ae153e929853623894184c63 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 16:18:57 +0200 Subject: [PATCH 08/17] Store Compiled* in the backend instead of Module --- cranelift/module/src/backend.rs | 26 ++- cranelift/module/src/lib.rs | 3 +- cranelift/module/src/module.rs | 274 +++++++++-------------------- cranelift/object/src/backend.rs | 93 +++++----- cranelift/simplejit/Cargo.toml | 1 + cranelift/simplejit/src/backend.rs | 147 ++++++++-------- 6 files changed, 218 insertions(+), 326 deletions(-) diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs index 0d4b5e8e5f..32fcd6cf7c 100644 --- a/cranelift/module/src/backend.rs +++ b/cranelift/module/src/backend.rs @@ -1,19 +1,19 @@ //! Defines the `Backend` trait. -use crate::{DataContext, FuncOrDataId}; use crate::DataId; use crate::FuncId; use crate::Linkage; -use crate::ModuleContents; +use crate::ModuleDeclarations; use crate::ModuleResult; +use crate::{DataContext, FuncOrDataId}; use core::marker; use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::Context; use cranelift_codegen::{binemit, ir}; -use std::{borrow::ToOwned, collections::HashMap}; use std::boxed::Box; use std::string::String; +use std::{borrow::ToOwned, collections::HashMap}; /// A `Backend` implements the functionality needed to support a `Module`. /// @@ -32,12 +32,6 @@ where /// A builder for constructing `Backend` instances. type Builder; - /// The results of compiling a function. - type CompiledFunction; - - /// The results of "compiling" a data object. - type CompiledData; - /// This is an object returned by `Module`'s /// [`finish`](struct.Module.html#method.finish) function, /// if the `Backend` has a purpose for this. @@ -71,10 +65,10 @@ where id: FuncId, name: &str, ctx: &Context, - contents: &ModuleContents, + declarations: &ModuleDeclarations, code_size: u32, trap_sink: &mut TS, - ) -> ModuleResult + ) -> ModuleResult<()> where TS: binemit::TrapSink; @@ -86,8 +80,8 @@ where id: FuncId, name: &str, bytes: &[u8], - contents: &ModuleContents, - ) -> ModuleResult; + declarations: &ModuleDeclarations, + ) -> ModuleResult<()>; /// Define a zero-initialized data object of the given size. /// @@ -100,15 +94,15 @@ where tls: bool, align: Option, data_ctx: &DataContext, - contents: &ModuleContents, - ) -> ModuleResult; + declarations: &ModuleDeclarations, + ) -> ModuleResult<()>; /// Consume this `Backend` and return a result. Some implementations may /// provide additional functionality through this result. fn finish( self, names: HashMap, - contents: ModuleContents, + declarations: ModuleDeclarations, ) -> Self::Product; } diff --git a/cranelift/module/src/lib.rs b/cranelift/module/src/lib.rs index 6331cfac1f..713739f893 100644 --- a/cranelift/module/src/lib.rs +++ b/cranelift/module/src/lib.rs @@ -40,8 +40,7 @@ mod traps; pub use crate::backend::{default_libcall_names, Backend}; pub use crate::data_context::{DataContext, DataDescription, Init}; pub use crate::module::{ - DataId, FuncId, FuncOrDataId, Linkage, Module, ModuleContents, ModuleError, ModuleFunction, - ModuleResult, + DataId, FuncId, FuncOrDataId, Linkage, Module, ModuleDeclarations, ModuleError, ModuleResult, }; pub use crate::traps::TrapSite; diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index f95ee5e27e..f83d8f4ef5 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -15,7 +15,6 @@ use log::info; use std::borrow::ToOwned; use std::convert::TryInto; use std::string::String; -use std::vec::Vec; use thiserror::Error; /// A function identifier for use in the `Module` interface. @@ -132,6 +131,20 @@ pub struct FunctionDeclaration { pub signature: ir::Signature, } +impl FunctionDeclaration { + fn merge(&mut self, linkage: Linkage, sig: &ir::Signature) -> Result<(), ModuleError> { + self.linkage = Linkage::merge(self.linkage, linkage); + if &self.signature != sig { + return Err(ModuleError::IncompatibleSignature( + self.name.clone(), + self.signature.clone(), + sig.clone(), + )); + } + Ok(()) + } +} + /// Error messages for all `Module` and `Backend` methods #[derive(Error, Debug)] pub enum ModuleError { @@ -165,44 +178,6 @@ pub enum ModuleError { /// A convenient alias for a `Result` that uses `ModuleError` as the error type. pub type ModuleResult = Result; -/// A function belonging to a `Module`. -pub struct ModuleFunction -where - B: Backend, -{ - /// The function declaration. - pub decl: FunctionDeclaration, - /// The compiled artifact, once it's available. - pub compiled: Option, -} - -impl ModuleFunction -where - B: Backend, -{ - fn merge(&mut self, linkage: Linkage, sig: &ir::Signature) -> Result<(), ModuleError> { - self.decl.linkage = Linkage::merge(self.decl.linkage, linkage); - if &self.decl.signature != sig { - return Err(ModuleError::IncompatibleSignature( - self.decl.name.clone(), - self.decl.signature.clone(), - sig.clone(), - )); - } - Ok(()) - } - - fn validate_for_define(&self) -> ModuleResult<()> { - if self.compiled.is_some() { - return Err(ModuleError::DuplicateDefinition(self.decl.name.clone())); - } - if !self.decl.linkage.is_definable() { - return Err(ModuleError::InvalidImportDefinition(self.decl.name.clone())); - } - Ok(()) - } -} - /// Information about a data object which can be accessed. pub struct DataDeclaration { pub name: String, @@ -212,56 +187,26 @@ pub struct DataDeclaration { pub align: Option, } -/// A data object belonging to a `Module`. -pub struct ModuleData -where - B: Backend, -{ - /// The data object declaration. - pub decl: DataDeclaration, - /// The "compiled" artifact, once it's available. - pub compiled: Option, -} - -impl ModuleData -where - B: Backend, -{ +impl DataDeclaration { fn merge(&mut self, linkage: Linkage, writable: bool, tls: bool, align: Option) { - self.decl.linkage = Linkage::merge(self.decl.linkage, linkage); - self.decl.writable = self.decl.writable || writable; - self.decl.align = self.decl.align.max(align); + self.linkage = Linkage::merge(self.linkage, linkage); + self.writable = self.writable || writable; + self.align = self.align.max(align); assert_eq!( - self.decl.tls, tls, + self.tls, tls, "Can't change TLS data object to normal or in the opposite way", ); } - - fn validate_for_define(&self) -> ModuleResult<()> { - if self.compiled.is_some() { - return Err(ModuleError::DuplicateDefinition(self.decl.name.clone())); - } - if !self.decl.linkage.is_definable() { - return Err(ModuleError::InvalidImportDefinition(self.decl.name.clone())); - } - Ok(()) - } } /// This provides a view to the state of a module which allows `ir::ExternalName`s to be translated /// into `FunctionDeclaration`s and `DataDeclaration`s. -pub struct ModuleContents -where - B: Backend, -{ - functions: PrimaryMap>, - data_objects: PrimaryMap>, +pub struct ModuleDeclarations { + functions: PrimaryMap, + data_objects: PrimaryMap, } -impl ModuleContents -where - B: Backend, -{ +impl ModuleDeclarations { /// Get the `FuncId` for the function named by `name`. pub fn get_function_id(&self, name: &ir::ExternalName) -> FuncId { if let ir::ExternalName::User { namespace, index } = *name { @@ -282,62 +227,14 @@ where } } - /// Get the `ModuleFunction` for the given function. - pub fn get_function_info(&self, func_id: FuncId) -> &ModuleFunction { + /// Get the `FunctionDeclaration` for the function named by `name`. + pub fn get_function_decl(&self, func_id: FuncId) -> &FunctionDeclaration { &self.functions[func_id] } - /// Get the `FunctionDeclaration` for the function named by `name`. - pub fn get_function_decl(&self, name: &ir::ExternalName) -> &FunctionDeclaration { - &self.functions[self.get_function_id(name)].decl - } - - /// Get the `ModuleData` for the given data object. - pub fn get_data_info(&self, data_id: DataId) -> &ModuleData { - &self.data_objects[data_id] - } - /// Get the `DataDeclaration` for the data object named by `name`. - pub fn get_data_decl(&self, name: &ir::ExternalName) -> &DataDeclaration { - &self.data_objects[self.get_data_id(name)].decl - } - - /// Get the definition for the function named by `name`, along with its name - /// and signature. - pub fn get_function_definition( - &self, - name: &ir::ExternalName, - ) -> (Option<&B::CompiledFunction>, &str, &ir::Signature) { - let info = &self.functions[self.get_function_id(name)]; - debug_assert!( - !info.decl.linkage.is_definable() || info.compiled.is_some(), - "Finalization requires a definition for function {}.", - name, - ); - debug_assert_eq!(info.decl.linkage.is_definable(), info.compiled.is_some()); - - ( - info.compiled.as_ref(), - &info.decl.name, - &info.decl.signature, - ) - } - - /// Get the definition for the data object named by `name`, along with its name - /// and writable flag - pub fn get_data_definition( - &self, - name: &ir::ExternalName, - ) -> (Option<&B::CompiledData>, &str, bool) { - let info = &self.data_objects[self.get_data_id(name)]; - debug_assert!( - !info.decl.linkage.is_definable() || info.compiled.is_some(), - "Finalization requires a definition for data object {}.", - name, - ); - debug_assert_eq!(info.decl.linkage.is_definable(), info.compiled.is_some()); - - (info.compiled.as_ref(), &info.decl.name, info.decl.writable) + pub fn get_data_decl(&self, data_id: DataId) -> &DataDeclaration { + &self.data_objects[data_id] } /// Return whether `name` names a function, rather than a data object. @@ -356,9 +253,7 @@ where B: Backend, { names: HashMap, - contents: ModuleContents, - functions_to_finalize: Vec, - data_objects_to_finalize: Vec, + declarations: ModuleDeclarations, backend: B, } @@ -374,12 +269,10 @@ where pub fn new(backend_builder: B::Builder) -> Self { Self { names: HashMap::new(), - contents: ModuleContents { + declarations: ModuleDeclarations { functions: PrimaryMap::new(), data_objects: PrimaryMap::new(), }, - functions_to_finalize: Vec::new(), - data_objects_to_finalize: Vec::new(), backend: B::new(backend_builder), } } @@ -442,10 +335,9 @@ where match self.names.entry(name.to_owned()) { Occupied(entry) => match *entry.get() { FuncOrDataId::Func(id) => { - let existing = &mut self.contents.functions[id]; + let existing = &mut self.declarations.functions[id]; existing.merge(linkage, signature)?; - self.backend - .declare_function(id, name, existing.decl.linkage); + self.backend.declare_function(id, name, existing.linkage); Ok(id) } FuncOrDataId::Data(..) => { @@ -453,13 +345,10 @@ where } }, Vacant(entry) => { - let id = self.contents.functions.push(ModuleFunction { - decl: FunctionDeclaration { - name: name.to_owned(), - linkage, - signature: signature.clone(), - }, - compiled: None, + let id = self.declarations.functions.push(FunctionDeclaration { + name: name.to_owned(), + linkage, + signature: signature.clone(), }); entry.insert(FuncOrDataId::Func(id)); self.backend.declare_function(id, name, linkage); @@ -469,8 +358,8 @@ where } /// An iterator over functions that have been declared in this module. - pub fn declared_functions(&self) -> core::slice::Iter<'_, ModuleFunction> { - self.contents.functions.values() + pub fn declared_functions(&self) -> core::slice::Iter<'_, FunctionDeclaration> { + self.declarations.functions.values() } /// Declare a data object in this module. @@ -487,15 +376,15 @@ where match self.names.entry(name.to_owned()) { Occupied(entry) => match *entry.get() { FuncOrDataId::Data(id) => { - let existing = &mut self.contents.data_objects[id]; + let existing = &mut self.declarations.data_objects[id]; existing.merge(linkage, writable, tls, align); self.backend.declare_data( id, name, - existing.decl.linkage, - existing.decl.writable, - existing.decl.tls, - existing.decl.align, + existing.linkage, + existing.writable, + existing.tls, + existing.align, ); Ok(id) } @@ -505,15 +394,12 @@ where } }, Vacant(entry) => { - let id = self.contents.data_objects.push(ModuleData { - decl: DataDeclaration { - name: name.to_owned(), - linkage, - writable, - tls, - align, - }, - compiled: None, + let id = self.declarations.data_objects.push(DataDeclaration { + name: name.to_owned(), + linkage, + writable, + tls, + align, }); entry.insert(FuncOrDataId::Data(id)); self.backend @@ -528,7 +414,7 @@ where /// TODO: Coalesce redundant decls and signatures. /// TODO: Look into ways to reduce the risk of using a FuncRef in the wrong function. pub fn declare_func_in_func(&self, func: FuncId, in_func: &mut ir::Function) -> ir::FuncRef { - let decl = &self.contents.functions[func].decl; + let decl = &self.declarations.functions[func]; let signature = in_func.import_signature(decl.signature.clone()); let colocated = decl.linkage.is_final(); in_func.import_function(ir::ExtFuncData { @@ -542,7 +428,7 @@ where /// /// TODO: Same as above. pub fn declare_data_in_func(&self, data: DataId, func: &mut ir::Function) -> ir::GlobalValue { - let decl = &self.contents.data_objects[data].decl; + let decl = &self.declarations.data_objects[data]; let colocated = decl.linkage.is_final(); func.create_global_value(ir::GlobalValueData::Symbol { name: ir::ExternalName::user(1, data.as_u32()), @@ -582,20 +468,20 @@ where ctx.func.display(self.backend.isa()) ); let CodeInfo { total_size, .. } = ctx.compile(self.backend.isa())?; - let info = &self.contents.functions[func]; - info.validate_for_define()?; + let decl = &self.declarations.functions[func]; + if !decl.linkage.is_definable() { + return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); + } - let compiled = self.backend.define_function( + self.backend.define_function( func, - &info.decl.name, + &decl.name, ctx, - &self.contents, + &self.declarations, total_size, trap_sink, )?; - self.contents.functions[func].compiled = Some(compiled); - self.functions_to_finalize.push(func); Ok(ModuleCompiledFunction { size: total_size }) } @@ -612,40 +498,38 @@ where bytes: &[u8], ) -> ModuleResult { info!("defining function {} with bytes", func); - let info = &self.contents.functions[func]; - info.validate_for_define()?; + let decl = &self.declarations.functions[func]; + if !decl.linkage.is_definable() { + return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); + } let total_size: u32 = match bytes.len().try_into() { Ok(total_size) => total_size, - _ => Err(ModuleError::FunctionTooLarge(info.decl.name.clone()))?, + _ => Err(ModuleError::FunctionTooLarge(decl.name.clone()))?, }; - let compiled = - self.backend - .define_function_bytes(func, &info.decl.name, bytes, &self.contents)?; + self.backend + .define_function_bytes(func, &decl.name, bytes, &self.declarations)?; - self.contents.functions[func].compiled = Some(compiled); - self.functions_to_finalize.push(func); Ok(ModuleCompiledFunction { size: total_size }) } /// Define a data object, producing the data contents from the given `DataContext`. pub fn define_data(&mut self, data: DataId, data_ctx: &DataContext) -> ModuleResult<()> { - let compiled = { - let info = &self.contents.data_objects[data]; - info.validate_for_define()?; - Some(self.backend.define_data( - data, - &info.decl.name, - info.decl.writable, - info.decl.tls, - info.decl.align, - data_ctx, - &self.contents, - )?) - }; - self.contents.data_objects[data].compiled = compiled; - self.data_objects_to_finalize.push(data); + let decl = &self.declarations.data_objects[data]; + if !decl.linkage.is_definable() { + return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); + } + + self.backend.define_data( + data, + &decl.name, + decl.writable, + decl.tls, + decl.align, + data_ctx, + &self.declarations, + )?; Ok(()) } @@ -658,6 +542,6 @@ where /// implementations may provide additional functionality available after /// a `Module` is complete. pub fn finish(self) -> B::Product { - self.backend.finish(self.names, self.contents) + self.backend.finish(self.names, self.declarations) } } diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index 64feb15b45..42060a8722 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -9,7 +9,7 @@ use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::{self, ir}; use cranelift_module::{ Backend, DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, - ModuleContents, ModuleError, ModuleResult, + ModuleDeclarations, ModuleError, ModuleResult, }; use object::write::{ Object, Relocation, SectionId, StandardSection, Symbol, SymbolId, SymbolSection, @@ -112,8 +112,8 @@ impl ObjectBuilder { pub struct ObjectBackend { isa: Box, object: Object, - functions: SecondaryMap>, - data_objects: SecondaryMap>, + functions: SecondaryMap>, + data_objects: SecondaryMap>, relocs: Vec, libcalls: HashMap, libcall_names: Box String>, @@ -124,9 +124,6 @@ pub struct ObjectBackend { impl Backend for ObjectBackend { type Builder = ObjectBuilder; - type CompiledFunction = ObjectCompiledFunction; - type CompiledData = ObjectCompiledData; - type Product = ObjectProduct; /// Create a new `ObjectBackend` using the given Cranelift target. @@ -153,7 +150,7 @@ impl Backend for ObjectBackend { fn declare_function(&mut self, id: FuncId, name: &str, linkage: Linkage) { let (scope, weak) = translate_linkage(linkage); - if let Some(function) = self.functions[id] { + if let Some((function, _defined)) = self.functions[id] { let symbol = self.object.symbol_mut(function); symbol.scope = scope; symbol.weak = weak; @@ -168,7 +165,7 @@ impl Backend for ObjectBackend { section: SymbolSection::Undefined, flags: SymbolFlags::None, }); - self.functions[id] = Some(symbol_id); + self.functions[id] = Some((symbol_id, false)); } } @@ -188,7 +185,7 @@ impl Backend for ObjectBackend { }; let (scope, weak) = translate_linkage(linkage); - if let Some(data) = self.data_objects[id] { + if let Some((data, _defined)) = self.data_objects[id] { let symbol = self.object.symbol_mut(data); symbol.kind = kind; symbol.scope = scope; @@ -204,22 +201,28 @@ impl Backend for ObjectBackend { section: SymbolSection::Undefined, flags: SymbolFlags::None, }); - self.data_objects[id] = Some(symbol_id); + self.data_objects[id] = Some((symbol_id, false)); } } fn define_function( &mut self, func_id: FuncId, - _name: &str, + name: &str, ctx: &cranelift_codegen::Context, - _contents: &ModuleContents, + _declarations: &ModuleDeclarations, code_size: u32, trap_sink: &mut TS, - ) -> ModuleResult + ) -> ModuleResult<()> where TS: TrapSink, { + let &mut (symbol, ref mut defined) = self.functions[func_id].as_mut().unwrap(); + if *defined { + return Err(ModuleError::DuplicateDefinition(name.to_owned())); + } + *defined = true; + let mut code: Vec = vec![0; code_size as usize]; let mut reloc_sink = ObjectRelocSink::new(self.object.format()); let mut stack_map_sink = NullStackMapSink {}; @@ -234,8 +237,6 @@ impl Backend for ObjectBackend { ) }; - let symbol = self.functions[func_id].unwrap(); - let (section, offset) = if self.per_function_section { let symbol_name = self.object.symbol(symbol).name.clone(); let (section, offset) = self.object.add_subsection( @@ -262,17 +263,21 @@ impl Backend for ObjectBackend { relocs: reloc_sink.relocs, }); } - Ok(ObjectCompiledFunction) + Ok(()) } fn define_function_bytes( &mut self, func_id: FuncId, - _name: &str, + name: &str, bytes: &[u8], - _contents: &ModuleContents, - ) -> ModuleResult { - let symbol = self.functions[func_id].unwrap(); + _declarations: &ModuleDeclarations, + ) -> ModuleResult<()> { + let &mut (symbol, ref mut defined) = self.functions[func_id].as_mut().unwrap(); + if *defined { + return Err(ModuleError::DuplicateDefinition(name.to_owned())); + } + *defined = true; if self.per_function_section { let symbol_name = self.object.symbol(symbol).name.clone(); @@ -291,19 +296,25 @@ impl Backend for ObjectBackend { .add_symbol_data(symbol, section, bytes, self.function_alignment); } - Ok(ObjectCompiledFunction) + Ok(()) } fn define_data( &mut self, data_id: DataId, - _name: &str, + name: &str, writable: bool, tls: bool, align: Option, data_ctx: &DataContext, - _contents: &ModuleContents, - ) -> ModuleResult { + _declarations: &ModuleDeclarations, + ) -> ModuleResult<()> { + let &mut (symbol, ref mut defined) = self.data_objects[data_id].as_mut().unwrap(); + if *defined { + return Err(ModuleError::DuplicateDefinition(name.to_owned())); + } + *defined = true; + let &DataDescription { ref init, ref function_decls, @@ -340,7 +351,6 @@ impl Backend for ObjectBackend { }); } - let symbol = self.data_objects[data_id].unwrap(); let section = if custom_segment_section.is_none() { let section_kind = if let Init::Zeros { .. } = *init { if tls { @@ -397,13 +407,13 @@ impl Backend for ObjectBackend { relocs, }); } - Ok(ObjectCompiledData) + Ok(()) } fn finish( mut self, _names: HashMap, - contents: ModuleContents, + declarations: ModuleDeclarations, ) -> ObjectProduct { let symbol_relocs = mem::take(&mut self.relocs); for symbol in symbol_relocs { @@ -416,7 +426,7 @@ impl Backend for ObjectBackend { addend, } in &symbol.relocs { - let target_symbol = self.get_symbol(&contents, name); + let target_symbol = self.get_symbol(&declarations, name); self.object .add_relocation( symbol.section, @@ -453,15 +463,19 @@ impl Backend for ObjectBackend { impl ObjectBackend { // This should only be called during finish because it creates // symbols for missing libcalls. - fn get_symbol(&mut self, contents: &ModuleContents, name: &ir::ExternalName) -> SymbolId { + fn get_symbol( + &mut self, + declarations: &ModuleDeclarations, + name: &ir::ExternalName, + ) -> SymbolId { match *name { ir::ExternalName::User { .. } => { - if contents.is_function(name) { - let id = contents.get_function_id(name); - self.functions[id].unwrap() + if declarations.is_function(name) { + let id = declarations.get_function_id(name); + self.functions[id].unwrap().0 } else { - let id = contents.get_data_id(name); - self.data_objects[id].unwrap() + let id = declarations.get_data_id(name); + self.data_objects[id].unwrap().0 } } ir::ExternalName::LibCall(ref libcall) => { @@ -502,9 +516,6 @@ fn translate_linkage(linkage: Linkage) -> (SymbolScope, bool) { (scope, weak) } -pub struct ObjectCompiledFunction; -pub struct ObjectCompiledData; - /// This is the output of `Module`'s /// [`finish`](../cranelift_module/struct.Module.html#method.finish) function. /// It contains the generated `Object` and other information produced during @@ -513,22 +524,22 @@ pub struct ObjectProduct { /// Object artifact with all functions and data from the module defined. pub object: Object, /// Symbol IDs for functions (both declared and defined). - pub functions: SecondaryMap>, + pub functions: SecondaryMap>, /// Symbol IDs for data objects (both declared and defined). - pub data_objects: SecondaryMap>, + pub data_objects: SecondaryMap>, } impl ObjectProduct { /// Return the `SymbolId` for the given function. #[inline] pub fn function_symbol(&self, id: FuncId) -> SymbolId { - self.functions[id].unwrap() + self.functions[id].unwrap().0 } /// Return the `SymbolId` for the given data object. #[inline] pub fn data_symbol(&self, id: DataId) -> SymbolId { - self.data_objects[id].unwrap() + self.data_objects[id].unwrap().0 } /// Write the object bytes in memory. diff --git a/cranelift/simplejit/Cargo.toml b/cranelift/simplejit/Cargo.toml index 9fe617bd14..dcbdbd7300 100644 --- a/cranelift/simplejit/Cargo.toml +++ b/cranelift/simplejit/Cargo.toml @@ -13,6 +13,7 @@ edition = "2018" cranelift-module = { path = "../module", version = "0.67.0" } cranelift-native = { path = "../native", version = "0.67.0" } cranelift-codegen = { path = "../codegen", version = "0.67.0", default-features = false, features = ["std"] } +cranelift-entity = { path = "../entity", version = "0.67.0" } region = "2.2.0" libc = { version = "0.2.42" } errno = "0.2.4" diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 5a97ae8b5e..c0c6dee757 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -7,9 +7,10 @@ use cranelift_codegen::binemit::{ use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::settings::Configurable; use cranelift_codegen::{self, ir, settings}; +use cranelift_entity::SecondaryMap; use cranelift_module::{ Backend, DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, - ModuleContents, ModuleResult, + ModuleDeclarations, ModuleError, ModuleResult, }; use cranelift_native; #[cfg(not(windows))] @@ -124,11 +125,14 @@ pub struct SimpleJITBackend { symbols: HashMap, libcall_names: Box String>, memory: SimpleJITMemoryHandle, + functions: SecondaryMap>, + data_objects: SecondaryMap>, functions_to_finalize: Vec, data_objects_to_finalize: Vec, } /// A record of a relocation to perform. +#[derive(Clone)] struct RelocRecord { offset: CodeOffset, reloc: Reloc, @@ -143,12 +147,14 @@ struct StackMapRecord { stack_map: StackMap, } +#[derive(Clone)] pub struct SimpleJITCompiledFunction { code: *mut u8, size: usize, relocs: Vec, } +#[derive(Clone)] pub struct SimpleJITCompiledData { storage: *mut u8, size: usize, @@ -167,7 +173,8 @@ struct SimpleJITMemoryHandle { pub struct SimpleJITProduct { memory: SimpleJITMemoryHandle, names: HashMap, - contents: ModuleContents, + functions: SecondaryMap>, + data_objects: SecondaryMap>, } impl SimpleJITProduct { @@ -192,19 +199,16 @@ impl SimpleJITProduct { /// Return the address of a function. pub fn lookup_func(&self, func_id: FuncId) -> *const u8 { - self.contents - .get_function_definition(&func_id.into()) - .0 + self.functions[func_id] + .as_ref() .unwrap_or_else(|| panic!("{} is not defined", func_id)) .code } /// Return the address and size of a data object. pub fn lookup_data(&self, data_id: DataId) -> (*const u8, usize) { - let data = self - .contents - .get_data_definition(&data_id.into()) - .0 + let data = self.data_objects[data_id] + .as_ref() .unwrap_or_else(|| panic!("{} is not defined", data_id)); (data.storage, data.size) } @@ -220,22 +224,22 @@ impl SimpleJITBackend { fn get_definition( &self, - contents: &ModuleContents, + declarations: &ModuleDeclarations, name: &ir::ExternalName, ) -> *const u8 { match *name { ir::ExternalName::User { .. } => { - if contents.is_function(name) { - let (def, name_str, _signature) = contents.get_function_definition(&name); - match def { + if declarations.is_function(name) { + let func_id = declarations.get_function_id(name); + match &self.functions[func_id] { Some(compiled) => compiled.code, - None => self.lookup_symbol(name_str), + None => self.lookup_symbol(&declarations.get_function_decl(func_id).name), } } else { - let (def, name_str, _writable) = contents.get_data_definition(&name); - match def { + let data_id = declarations.get_data_id(name); + match &self.data_objects[data_id] { Some(compiled) => compiled.storage, - None => self.lookup_symbol(name_str), + None => self.lookup_symbol(&declarations.get_data_decl(data_id).name), } } } @@ -264,15 +268,13 @@ impl SimpleJITBackend { } } - - fn finalize_function( - &mut self, - _id: FuncId, - func: &SimpleJITCompiledFunction, - contents: &ModuleContents, - ) { + fn finalize_function(&mut self, id: FuncId, declarations: &ModuleDeclarations) { use std::ptr::write_unaligned; + let func = self.functions[id] + .as_ref() + .expect("function must be compiled before it can be finalized"); + for &RelocRecord { reloc, offset, @@ -283,7 +285,7 @@ impl SimpleJITBackend { let ptr = func.code; debug_assert!((offset as usize) < func.size); let at = unsafe { ptr.offset(offset as isize) }; - let base = self.get_definition(contents, name); + let base = self.get_definition(declarations, name); // TODO: Handle overflow. let what = unsafe { base.offset(addend as isize) }; match reloc { @@ -314,14 +316,13 @@ impl SimpleJITBackend { } } - fn finalize_data( - &mut self, - _id: DataId, - data: &SimpleJITCompiledData, - contents: &ModuleContents, - ) { + fn finalize_data(&mut self, id: DataId, declarations: &ModuleDeclarations) { use std::ptr::write_unaligned; + let data = self.data_objects[id] + .as_ref() + .expect("data object must be compiled before it can be finalized"); + for &RelocRecord { reloc, offset, @@ -332,7 +333,7 @@ impl SimpleJITBackend { let ptr = data.storage; debug_assert!((offset as usize) < data.size); let at = unsafe { ptr.offset(offset as isize) }; - let base = self.get_definition(contents, name); + let base = self.get_definition(declarations, name); // TODO: Handle overflow. let what = unsafe { base.offset(addend as isize) }; match reloc { @@ -362,13 +363,6 @@ impl SimpleJITBackend { impl<'simple_jit_backend> Backend for SimpleJITBackend { type Builder = SimpleJITBuilder; - /// SimpleJIT compiled function and data objects may have outstanding - /// relocations that need to be performed before the memory can be used. - /// These relocations are performed within `finalize_function` and - /// `finalize_data`. - type CompiledFunction = SimpleJITCompiledFunction; - type CompiledData = SimpleJITCompiledData; - /// SimpleJIT emits code and data into memory as it processes them, so it /// doesn't need to provide anything after the `Module` is complete. /// The handle object that is returned can optionally be used to free @@ -388,6 +382,8 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { symbols: builder.symbols, libcall_names: builder.libcall_names, memory, + functions: SecondaryMap::new(), + data_objects: SecondaryMap::new(), functions_to_finalize: Vec::new(), data_objects_to_finalize: Vec::new(), } @@ -419,13 +415,17 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { id: FuncId, name: &str, ctx: &cranelift_codegen::Context, - _contents: &ModuleContents, + _declarations: &ModuleDeclarations, code_size: u32, trap_sink: &mut TS, - ) -> ModuleResult + ) -> ModuleResult<()> where TS: TrapSink, { + if !self.functions[id].is_none() { + return Err(ModuleError::DuplicateDefinition(name.to_owned())); + } + self.functions_to_finalize.push(id); let size = code_size as usize; let ptr = self @@ -448,11 +448,13 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { ) }; - Ok(Self::CompiledFunction { + self.functions[id] = Some(SimpleJITCompiledFunction { code: ptr, size, relocs: reloc_sink.relocs, - }) + }); + + Ok(()) } fn define_function_bytes( @@ -460,8 +462,12 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { id: FuncId, name: &str, bytes: &[u8], - _contents: &ModuleContents, - ) -> ModuleResult { + _declarations: &ModuleDeclarations, + ) -> ModuleResult<()> { + if !self.functions[id].is_none() { + return Err(ModuleError::DuplicateDefinition(name.to_owned())); + } + self.functions_to_finalize.push(id); let size = bytes.len(); let ptr = self @@ -476,23 +482,29 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { ptr::copy_nonoverlapping(bytes.as_ptr(), ptr, size); } - Ok(Self::CompiledFunction { + self.functions[id] = Some(SimpleJITCompiledFunction { code: ptr, size, relocs: vec![], - }) + }); + + Ok(()) } fn define_data( &mut self, id: DataId, - _name: &str, + name: &str, writable: bool, tls: bool, align: Option, data: &DataContext, - _contents: &ModuleContents, - ) -> ModuleResult { + _declarations: &ModuleDeclarations, + ) -> ModuleResult<()> { + if !self.data_objects[id].is_none() { + return Err(ModuleError::DuplicateDefinition(name.to_owned())); + } + assert!(!tls, "SimpleJIT doesn't yet support TLS"); self.data_objects_to_finalize.push(id); @@ -555,11 +567,13 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { }); } - Ok(Self::CompiledData { + self.data_objects[id] = Some(SimpleJITCompiledData { storage, size, relocs, - }) + }); + + Ok(()) } /// SimpleJIT emits code and data into memory as it processes them. This @@ -572,29 +586,17 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { fn finish( mut self, names: HashMap, - contents: ModuleContents, + declarations: ModuleDeclarations, ) -> Self::Product { for func in std::mem::take(&mut self.functions_to_finalize) { - let info = contents.get_function_info(func); - debug_assert!(info.decl.linkage.is_definable()); - self.finalize_function( - func, - info.compiled - .as_ref() - .expect("function must be compiled before it can be finalized"), - &contents, - ); + let decl = declarations.get_function_decl(func); + debug_assert!(decl.linkage.is_definable()); + self.finalize_function(func, &declarations); } for data in std::mem::take(&mut self.data_objects_to_finalize) { - let info = contents.get_data_info(data); - debug_assert!(info.decl.linkage.is_definable()); - self.finalize_data( - data, - info.compiled - .as_ref() - .expect("data object must be compiled before it can be finalized"), - &contents, - ); + let decl = declarations.get_data_decl(data); + debug_assert!(decl.linkage.is_definable()); + self.finalize_data(data, &declarations); } // Now that we're done patching, prepare the memory for execution! @@ -604,7 +606,8 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { SimpleJITProduct { memory: self.memory, names, - contents, + functions: self.functions, + data_objects: self.data_objects, } } } From 80f4ecf9b511c3767c65e5657775e154ae5fe1fa Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 16:50:24 +0200 Subject: [PATCH 09/17] Move almost all logic out of Module --- cranelift/module/src/backend.rs | 10 +- cranelift/module/src/module.rs | 161 +++++++++++++++++------------ cranelift/object/src/backend.rs | 10 +- cranelift/simplejit/src/backend.rs | 12 +-- 4 files changed, 106 insertions(+), 87 deletions(-) diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs index 32fcd6cf7c..69137d7144 100644 --- a/cranelift/module/src/backend.rs +++ b/cranelift/module/src/backend.rs @@ -5,7 +5,7 @@ use crate::FuncId; use crate::Linkage; use crate::ModuleDeclarations; use crate::ModuleResult; -use crate::{DataContext, FuncOrDataId}; +use crate::DataContext; use core::marker; use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::Context; @@ -13,7 +13,7 @@ use cranelift_codegen::{binemit, ir}; use std::boxed::Box; use std::string::String; -use std::{borrow::ToOwned, collections::HashMap}; +use std::borrow::ToOwned; /// A `Backend` implements the functionality needed to support a `Module`. /// @@ -99,11 +99,7 @@ where /// Consume this `Backend` and return a result. Some implementations may /// provide additional functionality through this result. - fn finish( - self, - names: HashMap, - declarations: ModuleDeclarations, - ) -> Self::Product; + fn finish(self, declarations: ModuleDeclarations) -> Self::Product; } /// Default names for `ir::LibCall`s. A function by this name is imported into the object as diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index f83d8f4ef5..aa30ce0daf 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -202,11 +202,18 @@ impl DataDeclaration { /// This provides a view to the state of a module which allows `ir::ExternalName`s to be translated /// into `FunctionDeclaration`s and `DataDeclaration`s. pub struct ModuleDeclarations { + names: HashMap, functions: PrimaryMap, data_objects: PrimaryMap, } impl ModuleDeclarations { + /// Get the module identifier for a given name, if that name + /// has been declared. + pub fn get_name(&self, name: &str) -> Option { + self.names.get(name).copied() + } + /// Get the `FuncId` for the function named by `name`. pub fn get_function_id(&self, name: &ir::ExternalName) -> FuncId { if let ir::ExternalName::User { namespace, index } = *name { @@ -245,6 +252,75 @@ impl ModuleDeclarations { panic!("unexpected ExternalName kind {}", name) } } + + /// Declare a function in this module. + pub fn declare_function( + &mut self, + name: &str, + linkage: Linkage, + signature: &ir::Signature, + ) -> ModuleResult<(FuncId, &FunctionDeclaration)> { + // TODO: Can we avoid allocating names so often? + use super::hash_map::Entry::*; + match self.names.entry(name.to_owned()) { + Occupied(entry) => match *entry.get() { + FuncOrDataId::Func(id) => { + let existing = &mut self.functions[id]; + existing.merge(linkage, signature)?; + Ok((id, existing)) + } + FuncOrDataId::Data(..) => { + Err(ModuleError::IncompatibleDeclaration(name.to_owned())) + } + }, + Vacant(entry) => { + let id = self.functions.push(FunctionDeclaration { + name: name.to_owned(), + linkage, + signature: signature.clone(), + }); + entry.insert(FuncOrDataId::Func(id)); + Ok((id, &self.functions[id])) + } + } + } + + /// Declare a data object in this module. + pub fn declare_data( + &mut self, + name: &str, + linkage: Linkage, + writable: bool, + tls: bool, + align: Option, // An alignment bigger than 128 is unlikely + ) -> ModuleResult<(DataId, &DataDeclaration)> { + // TODO: Can we avoid allocating names so often? + use super::hash_map::Entry::*; + match self.names.entry(name.to_owned()) { + Occupied(entry) => match *entry.get() { + FuncOrDataId::Data(id) => { + let existing = &mut self.data_objects[id]; + existing.merge(linkage, writable, tls, align); + Ok((id, existing)) + } + + FuncOrDataId::Func(..) => { + Err(ModuleError::IncompatibleDeclaration(name.to_owned())) + } + }, + Vacant(entry) => { + let id = self.data_objects.push(DataDeclaration { + name: name.to_owned(), + linkage, + writable, + tls, + align, + }); + entry.insert(FuncOrDataId::Data(id)); + Ok((id, &self.data_objects[id])) + } + } + } } /// A `Module` is a utility for collecting functions and data objects, and linking them together. @@ -252,7 +328,6 @@ pub struct Module where B: Backend, { - names: HashMap, declarations: ModuleDeclarations, backend: B, } @@ -268,8 +343,8 @@ where /// Create a new `Module`. pub fn new(backend_builder: B::Builder) -> Self { Self { - names: HashMap::new(), declarations: ModuleDeclarations { + names: HashMap::new(), functions: PrimaryMap::new(), data_objects: PrimaryMap::new(), }, @@ -280,7 +355,7 @@ where /// Get the module identifier for a given name, if that name /// has been declared. pub fn get_name(&self, name: &str) -> Option { - self.names.get(name).cloned() + self.declarations.names.get(name).cloned() } /// Return the target information needed by frontends to produce Cranelift IR @@ -330,31 +405,11 @@ where linkage: Linkage, signature: &ir::Signature, ) -> ModuleResult { - // TODO: Can we avoid allocating names so often? - use super::hash_map::Entry::*; - match self.names.entry(name.to_owned()) { - Occupied(entry) => match *entry.get() { - FuncOrDataId::Func(id) => { - let existing = &mut self.declarations.functions[id]; - existing.merge(linkage, signature)?; - self.backend.declare_function(id, name, existing.linkage); - Ok(id) - } - FuncOrDataId::Data(..) => { - Err(ModuleError::IncompatibleDeclaration(name.to_owned())) - } - }, - Vacant(entry) => { - let id = self.declarations.functions.push(FunctionDeclaration { - name: name.to_owned(), - linkage, - signature: signature.clone(), - }); - entry.insert(FuncOrDataId::Func(id)); - self.backend.declare_function(id, name, linkage); - Ok(id) - } - } + let (id, decl) = self + .declarations + .declare_function(name, linkage, signature)?; + self.backend.declare_function(id, name, decl.linkage); + Ok(id) } /// An iterator over functions that have been declared in this module. @@ -371,42 +426,18 @@ where tls: bool, align: Option, // An alignment bigger than 128 is unlikely ) -> ModuleResult { - // TODO: Can we avoid allocating names so often? - use super::hash_map::Entry::*; - match self.names.entry(name.to_owned()) { - Occupied(entry) => match *entry.get() { - FuncOrDataId::Data(id) => { - let existing = &mut self.declarations.data_objects[id]; - existing.merge(linkage, writable, tls, align); - self.backend.declare_data( - id, - name, - existing.linkage, - existing.writable, - existing.tls, - existing.align, - ); - Ok(id) - } - - FuncOrDataId::Func(..) => { - Err(ModuleError::IncompatibleDeclaration(name.to_owned())) - } - }, - Vacant(entry) => { - let id = self.declarations.data_objects.push(DataDeclaration { - name: name.to_owned(), - linkage, - writable, - tls, - align, - }); - entry.insert(FuncOrDataId::Data(id)); - self.backend - .declare_data(id, name, linkage, writable, tls, align); - Ok(id) - } - } + let (id, decl) = self + .declarations + .declare_data(name, linkage, writable, tls, align)?; + self.backend.declare_data( + id, + name, + decl.linkage, + decl.writable, + decl.tls, + decl.align, + ); + Ok(id) } /// Use this when you're building the IR of a function to reference a function. @@ -542,6 +573,6 @@ where /// implementations may provide additional functionality available after /// a `Module` is complete. pub fn finish(self) -> B::Product { - self.backend.finish(self.names, self.declarations) + self.backend.finish(self.declarations) } } diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index 42060a8722..2d6528f535 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -8,8 +8,8 @@ use cranelift_codegen::entity::SecondaryMap; use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::{self, ir}; use cranelift_module::{ - Backend, DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, - ModuleDeclarations, ModuleError, ModuleResult, + Backend, DataContext, DataDescription, DataId, FuncId, Init, Linkage, ModuleDeclarations, + ModuleError, ModuleResult, }; use object::write::{ Object, Relocation, SectionId, StandardSection, Symbol, SymbolId, SymbolSection, @@ -410,11 +410,7 @@ impl Backend for ObjectBackend { Ok(()) } - fn finish( - mut self, - _names: HashMap, - declarations: ModuleDeclarations, - ) -> ObjectProduct { + fn finish(mut self, declarations: ModuleDeclarations) -> ObjectProduct { let symbol_relocs = mem::take(&mut self.relocs); for symbol in symbol_relocs { for &RelocRecord { diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index c0c6dee757..572b9849ee 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -172,7 +172,7 @@ struct SimpleJITMemoryHandle { /// defined in the original module. pub struct SimpleJITProduct { memory: SimpleJITMemoryHandle, - names: HashMap, + declarations: ModuleDeclarations, functions: SecondaryMap>, data_objects: SecondaryMap>, } @@ -194,7 +194,7 @@ impl SimpleJITProduct { /// Get the `FuncOrDataId` associated with the given name. pub fn func_or_data_for_func(&self, name: &str) -> Option { - self.names.get(name).copied() + self.declarations.get_name(name) } /// Return the address of a function. @@ -583,11 +583,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { /// /// This method does not need to be called when access to the memory /// handle is not required. - fn finish( - mut self, - names: HashMap, - declarations: ModuleDeclarations, - ) -> Self::Product { + fn finish(mut self, declarations: ModuleDeclarations) -> Self::Product { for func in std::mem::take(&mut self.functions_to_finalize) { let decl = declarations.get_function_decl(func); debug_assert!(decl.linkage.is_definable()); @@ -605,7 +601,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { SimpleJITProduct { memory: self.memory, - names, + declarations, functions: self.functions, data_objects: self.data_objects, } From c2ffcdc6d098642a7a697d5c01207c8bad73c892 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 17:12:27 +0200 Subject: [PATCH 10/17] Move logic out of more Module methods --- cranelift/module/src/backend.rs | 6 ---- cranelift/module/src/module.rs | 52 +++++++----------------------- cranelift/object/src/backend.rs | 45 +++++++++++++++----------- cranelift/simplejit/src/backend.rs | 47 ++++++++++++++++----------- 4 files changed, 67 insertions(+), 83 deletions(-) diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs index 69137d7144..61cab5521b 100644 --- a/cranelift/module/src/backend.rs +++ b/cranelift/module/src/backend.rs @@ -63,7 +63,6 @@ where fn define_function( &mut self, id: FuncId, - name: &str, ctx: &Context, declarations: &ModuleDeclarations, code_size: u32, @@ -78,7 +77,6 @@ where fn define_function_bytes( &mut self, id: FuncId, - name: &str, bytes: &[u8], declarations: &ModuleDeclarations, ) -> ModuleResult<()>; @@ -89,10 +87,6 @@ where fn define_data( &mut self, id: DataId, - name: &str, - writable: bool, - tls: bool, - align: Option, data_ctx: &DataContext, declarations: &ModuleDeclarations, ) -> ModuleResult<()>; diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index aa30ce0daf..e4f04842b9 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -201,6 +201,7 @@ impl DataDeclaration { /// This provides a view to the state of a module which allows `ir::ExternalName`s to be translated /// into `FunctionDeclaration`s and `DataDeclaration`s. +#[derive(Default)] pub struct ModuleDeclarations { names: HashMap, functions: PrimaryMap, @@ -361,7 +362,7 @@ where /// Return the target information needed by frontends to produce Cranelift IR /// for the current target. pub fn target_config(&self) -> isa::TargetFrontendConfig { - self.backend.isa().frontend_config() + self.isa().frontend_config() } /// Create a new `Context` initialized for use with this `Module`. @@ -370,7 +371,7 @@ where /// convention for the `TargetIsa`. pub fn make_context(&self) -> Context { let mut ctx = Context::new(); - ctx.func.signature.call_conv = self.backend.isa().default_call_conv(); + ctx.func.signature.call_conv = self.isa().default_call_conv(); ctx } @@ -380,14 +381,14 @@ where /// convention for the `TargetIsa`. pub fn clear_context(&self, ctx: &mut Context) { ctx.clear(); - ctx.func.signature.call_conv = self.backend.isa().default_call_conv(); + ctx.func.signature.call_conv = self.isa().default_call_conv(); } /// Create a new empty `Signature` with the default calling convention for /// the `TargetIsa`, to which parameter and return types can be added for /// declaring a function to be called by this `Module`. pub fn make_signature(&self) -> ir::Signature { - ir::Signature::new(self.backend.isa().default_call_conv()) + ir::Signature::new(self.isa().default_call_conv()) } /// Clear the given `Signature` and reset for use with a new function. @@ -395,7 +396,7 @@ where /// This ensures that the `Signature` is initialized with the default /// calling convention for the `TargetIsa`. pub fn clear_signature(&self, sig: &mut ir::Signature) { - sig.clear(self.backend.isa().default_call_conv()); + sig.clear(self.isa().default_call_conv()); } /// Declare a function in this module. @@ -429,14 +430,8 @@ where let (id, decl) = self .declarations .declare_data(name, linkage, writable, tls, align)?; - self.backend.declare_data( - id, - name, - decl.linkage, - decl.writable, - decl.tls, - decl.align, - ); + self.backend + .declare_data(id, name, decl.linkage, decl.writable, decl.tls, decl.align); Ok(id) } @@ -499,19 +494,9 @@ where ctx.func.display(self.backend.isa()) ); let CodeInfo { total_size, .. } = ctx.compile(self.backend.isa())?; - let decl = &self.declarations.functions[func]; - if !decl.linkage.is_definable() { - return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); - } - self.backend.define_function( - func, - &decl.name, - ctx, - &self.declarations, - total_size, - trap_sink, - )?; + self.backend + .define_function(func, ctx, &self.declarations, total_size, trap_sink)?; Ok(ModuleCompiledFunction { size: total_size }) } @@ -530,9 +515,6 @@ where ) -> ModuleResult { info!("defining function {} with bytes", func); let decl = &self.declarations.functions[func]; - if !decl.linkage.is_definable() { - return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); - } let total_size: u32 = match bytes.len().try_into() { Ok(total_size) => total_size, @@ -540,28 +522,18 @@ where }; self.backend - .define_function_bytes(func, &decl.name, bytes, &self.declarations)?; + .define_function_bytes(func, bytes, &self.declarations)?; Ok(ModuleCompiledFunction { size: total_size }) } /// Define a data object, producing the data contents from the given `DataContext`. pub fn define_data(&mut self, data: DataId, data_ctx: &DataContext) -> ModuleResult<()> { - let decl = &self.declarations.data_objects[data]; - if !decl.linkage.is_definable() { - return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); - } - self.backend.define_data( data, - &decl.name, - decl.writable, - decl.tls, - decl.align, data_ctx, &self.declarations, - )?; - Ok(()) + ) } /// Return the target isa diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index 2d6528f535..f00a878379 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -208,18 +208,22 @@ impl Backend for ObjectBackend { fn define_function( &mut self, func_id: FuncId, - name: &str, ctx: &cranelift_codegen::Context, - _declarations: &ModuleDeclarations, + declarations: &ModuleDeclarations, code_size: u32, trap_sink: &mut TS, ) -> ModuleResult<()> where TS: TrapSink, { + let decl = declarations.get_function_decl(func_id); + if !decl.linkage.is_definable() { + return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); + } + let &mut (symbol, ref mut defined) = self.functions[func_id].as_mut().unwrap(); if *defined { - return Err(ModuleError::DuplicateDefinition(name.to_owned())); + return Err(ModuleError::DuplicateDefinition(decl.name.clone())); } *defined = true; @@ -269,13 +273,17 @@ impl Backend for ObjectBackend { fn define_function_bytes( &mut self, func_id: FuncId, - name: &str, bytes: &[u8], - _declarations: &ModuleDeclarations, + declarations: &ModuleDeclarations, ) -> ModuleResult<()> { + let decl = declarations.get_function_decl(func_id); + if !decl.linkage.is_definable() { + return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); + } + let &mut (symbol, ref mut defined) = self.functions[func_id].as_mut().unwrap(); if *defined { - return Err(ModuleError::DuplicateDefinition(name.to_owned())); + return Err(ModuleError::DuplicateDefinition(decl.name.clone())); } *defined = true; @@ -302,16 +310,17 @@ impl Backend for ObjectBackend { fn define_data( &mut self, data_id: DataId, - name: &str, - writable: bool, - tls: bool, - align: Option, data_ctx: &DataContext, - _declarations: &ModuleDeclarations, + declarations: &ModuleDeclarations, ) -> ModuleResult<()> { + let decl = declarations.get_data_decl(data_id); + if !decl.linkage.is_definable() { + return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); + } + let &mut (symbol, ref mut defined) = self.data_objects[data_id].as_mut().unwrap(); if *defined { - return Err(ModuleError::DuplicateDefinition(name.to_owned())); + return Err(ModuleError::DuplicateDefinition(decl.name.clone())); } *defined = true; @@ -353,14 +362,14 @@ impl Backend for ObjectBackend { let section = if custom_segment_section.is_none() { let section_kind = if let Init::Zeros { .. } = *init { - if tls { + if decl.tls { StandardSection::UninitializedTls } else { StandardSection::UninitializedData } - } else if tls { + } else if decl.tls { StandardSection::Tls - } else if writable { + } else if decl.writable { StandardSection::Data } else if relocs.is_empty() { StandardSection::ReadOnlyData @@ -369,7 +378,7 @@ impl Backend for ObjectBackend { }; self.object.section_id(section_kind) } else { - if tls { + if decl.tls { return Err(cranelift_module::ModuleError::Backend(anyhow::anyhow!( "Custom section not supported for TLS" ))); @@ -378,7 +387,7 @@ impl Backend for ObjectBackend { self.object.add_section( seg.clone().into_bytes(), sec.clone().into_bytes(), - if writable { + if decl.writable { SectionKind::Data } else if relocs.is_empty() { SectionKind::ReadOnlyData @@ -388,7 +397,7 @@ impl Backend for ObjectBackend { ) }; - let align = u64::from(align.unwrap_or(1)); + let align = u64::from(decl.align.unwrap_or(1)); let offset = match *init { Init::Uninitialized => { panic!("data is not initialized yet"); diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 572b9849ee..391eaa517e 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -413,17 +413,21 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { fn define_function( &mut self, id: FuncId, - name: &str, ctx: &cranelift_codegen::Context, - _declarations: &ModuleDeclarations, + declarations: &ModuleDeclarations, code_size: u32, trap_sink: &mut TS, ) -> ModuleResult<()> where TS: TrapSink, { + let decl = declarations.get_function_decl(id); + if !decl.linkage.is_definable() { + return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); + } + if !self.functions[id].is_none() { - return Err(ModuleError::DuplicateDefinition(name.to_owned())); + return Err(ModuleError::DuplicateDefinition(decl.name.to_owned())); } self.functions_to_finalize.push(id); @@ -434,7 +438,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { .allocate(size, EXECUTABLE_DATA_ALIGNMENT) .expect("TODO: handle OOM etc."); - self.record_function_for_perf(ptr, size, name); + self.record_function_for_perf(ptr, size, &decl.name); let mut reloc_sink = SimpleJITRelocSink::new(); let mut stack_map_sink = SimpleJITStackMapSink::new(); @@ -460,12 +464,16 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { fn define_function_bytes( &mut self, id: FuncId, - name: &str, bytes: &[u8], - _declarations: &ModuleDeclarations, + declarations: &ModuleDeclarations, ) -> ModuleResult<()> { + let decl = declarations.get_function_decl(id); + if !decl.linkage.is_definable() { + return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); + } + if !self.functions[id].is_none() { - return Err(ModuleError::DuplicateDefinition(name.to_owned())); + return Err(ModuleError::DuplicateDefinition(decl.name.to_owned())); } self.functions_to_finalize.push(id); @@ -476,7 +484,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { .allocate(size, EXECUTABLE_DATA_ALIGNMENT) .expect("TODO: handle OOM etc."); - self.record_function_for_perf(ptr, size, name); + self.record_function_for_perf(ptr, size, &decl.name); unsafe { ptr::copy_nonoverlapping(bytes.as_ptr(), ptr, size); @@ -494,18 +502,19 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { fn define_data( &mut self, id: DataId, - name: &str, - writable: bool, - tls: bool, - align: Option, data: &DataContext, - _declarations: &ModuleDeclarations, + declarations: &ModuleDeclarations, ) -> ModuleResult<()> { - if !self.data_objects[id].is_none() { - return Err(ModuleError::DuplicateDefinition(name.to_owned())); + let decl = declarations.get_data_decl(id); + if !decl.linkage.is_definable() { + return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); } - assert!(!tls, "SimpleJIT doesn't yet support TLS"); + if !self.data_objects[id].is_none() { + return Err(ModuleError::DuplicateDefinition(decl.name.to_owned())); + } + + assert!(!decl.tls, "SimpleJIT doesn't yet support TLS"); self.data_objects_to_finalize.push(id); @@ -519,15 +528,15 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { } = data.description(); let size = init.size(); - let storage = if writable { + let storage = if decl.writable { self.memory .writable - .allocate(size, align.unwrap_or(WRITABLE_DATA_ALIGNMENT)) + .allocate(size, decl.align.unwrap_or(WRITABLE_DATA_ALIGNMENT)) .expect("TODO: handle OOM etc.") } else { self.memory .readonly - .allocate(size, align.unwrap_or(READONLY_DATA_ALIGNMENT)) + .allocate(size, decl.align.unwrap_or(READONLY_DATA_ALIGNMENT)) .expect("TODO: handle OOM etc.") }; From 7a6e909efe9a72f94a9aac3acceb0325e25a1236 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 17:31:08 +0200 Subject: [PATCH 11/17] Move a bit more logic out of Module --- Cargo.lock | 2 ++ cranelift/module/src/backend.rs | 12 +++++----- cranelift/module/src/lib.rs | 3 ++- cranelift/module/src/module.rs | 35 +++++----------------------- cranelift/object/Cargo.toml | 1 + cranelift/object/src/backend.rs | 37 ++++++++++++++++++++++-------- cranelift/simplejit/Cargo.toml | 1 + cranelift/simplejit/src/backend.rs | 28 +++++++++++++++------- 8 files changed, 66 insertions(+), 53 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4fc0480e99..ee7bc821ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -489,6 +489,7 @@ dependencies = [ "anyhow", "cranelift-codegen", "cranelift-module", + "log", "object 0.21.1", "target-lexicon", ] @@ -535,6 +536,7 @@ dependencies = [ "cranelift-native", "errno", "libc", + "log", "memmap", "region", "target-lexicon", diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs index 61cab5521b..a72adf00cd 100644 --- a/cranelift/module/src/backend.rs +++ b/cranelift/module/src/backend.rs @@ -1,19 +1,20 @@ //! Defines the `Backend` trait. +use crate::module::ModuleCompiledFunction; +use crate::DataContext; use crate::DataId; use crate::FuncId; use crate::Linkage; use crate::ModuleDeclarations; use crate::ModuleResult; -use crate::DataContext; use core::marker; use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::Context; use cranelift_codegen::{binemit, ir}; +use std::borrow::ToOwned; use std::boxed::Box; use std::string::String; -use std::borrow::ToOwned; /// A `Backend` implements the functionality needed to support a `Module`. /// @@ -63,11 +64,10 @@ where fn define_function( &mut self, id: FuncId, - ctx: &Context, + ctx: &mut Context, declarations: &ModuleDeclarations, - code_size: u32, trap_sink: &mut TS, - ) -> ModuleResult<()> + ) -> ModuleResult where TS: binemit::TrapSink; @@ -79,7 +79,7 @@ where id: FuncId, bytes: &[u8], declarations: &ModuleDeclarations, - ) -> ModuleResult<()>; + ) -> ModuleResult; /// Define a zero-initialized data object of the given size. /// diff --git a/cranelift/module/src/lib.rs b/cranelift/module/src/lib.rs index 713739f893..b04d29b87c 100644 --- a/cranelift/module/src/lib.rs +++ b/cranelift/module/src/lib.rs @@ -40,7 +40,8 @@ mod traps; pub use crate::backend::{default_libcall_names, Backend}; pub use crate::data_context::{DataContext, DataDescription, Init}; pub use crate::module::{ - DataId, FuncId, FuncOrDataId, Linkage, Module, ModuleDeclarations, ModuleError, ModuleResult, + DataId, FuncId, FuncOrDataId, Linkage, Module, ModuleCompiledFunction, ModuleDeclarations, + ModuleError, ModuleResult, }; pub use crate::traps::TrapSite; diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index e4f04842b9..62050142bb 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -8,12 +8,10 @@ use super::HashMap; use crate::data_context::DataContext; use crate::Backend; -use cranelift_codegen::binemit::{self, CodeInfo}; +use cranelift_codegen::binemit; use cranelift_codegen::entity::{entity_impl, PrimaryMap}; use cranelift_codegen::{ir, isa, CodegenError, Context}; -use log::info; use std::borrow::ToOwned; -use std::convert::TryInto; use std::string::String; use thiserror::Error; @@ -333,7 +331,9 @@ where backend: B, } +/// Information about the compiled function. pub struct ModuleCompiledFunction { + /// The size of the compiled function. pub size: binemit::CodeOffset, } @@ -488,17 +488,8 @@ where where TS: binemit::TrapSink, { - info!( - "defining function {}: {}", - func, - ctx.func.display(self.backend.isa()) - ); - let CodeInfo { total_size, .. } = ctx.compile(self.backend.isa())?; - self.backend - .define_function(func, ctx, &self.declarations, total_size, trap_sink)?; - - Ok(ModuleCompiledFunction { size: total_size }) + .define_function(func, ctx, &self.declarations, trap_sink) } /// Define a function, taking the function body from the given `bytes`. @@ -513,27 +504,13 @@ where func: FuncId, bytes: &[u8], ) -> ModuleResult { - info!("defining function {} with bytes", func); - let decl = &self.declarations.functions[func]; - - let total_size: u32 = match bytes.len().try_into() { - Ok(total_size) => total_size, - _ => Err(ModuleError::FunctionTooLarge(decl.name.clone()))?, - }; - self.backend - .define_function_bytes(func, bytes, &self.declarations)?; - - Ok(ModuleCompiledFunction { size: total_size }) + .define_function_bytes(func, bytes, &self.declarations) } /// Define a data object, producing the data contents from the given `DataContext`. pub fn define_data(&mut self, data: DataId, data_ctx: &DataContext) -> ModuleResult<()> { - self.backend.define_data( - data, - data_ctx, - &self.declarations, - ) + self.backend.define_data(data, data_ctx, &self.declarations) } /// Return the target isa diff --git a/cranelift/object/Cargo.toml b/cranelift/object/Cargo.toml index c39332ce2f..7257652a4b 100644 --- a/cranelift/object/Cargo.toml +++ b/cranelift/object/Cargo.toml @@ -15,6 +15,7 @@ cranelift-codegen = { path = "../codegen", version = "0.67.0", default-features object = { version = "0.21.1", default-features = false, features = ["write"] } target-lexicon = "0.11" anyhow = "1.0" +log = { version = "0.4.6", default-features = false } [badges] maintenance = { status = "experimental" } diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index f00a878379..4a372a8be5 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -2,15 +2,16 @@ use anyhow::anyhow; use cranelift_codegen::binemit::{ - Addend, CodeOffset, NullStackMapSink, Reloc, RelocSink, TrapSink, + Addend, CodeInfo, CodeOffset, NullStackMapSink, Reloc, RelocSink, TrapSink, }; use cranelift_codegen::entity::SecondaryMap; use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::{self, ir}; use cranelift_module::{ - Backend, DataContext, DataDescription, DataId, FuncId, Init, Linkage, ModuleDeclarations, - ModuleError, ModuleResult, + Backend, DataContext, DataDescription, DataId, FuncId, Init, Linkage, ModuleCompiledFunction, + ModuleDeclarations, ModuleError, ModuleResult, }; +use log::info; use object::write::{ Object, Relocation, SectionId, StandardSection, Symbol, SymbolId, SymbolSection, }; @@ -18,6 +19,7 @@ use object::{ RelocationEncoding, RelocationKind, SectionKind, SymbolFlags, SymbolKind, SymbolScope, }; use std::collections::HashMap; +use std::convert::TryInto; use std::mem; use target_lexicon::PointerWidth; @@ -208,14 +210,23 @@ impl Backend for ObjectBackend { fn define_function( &mut self, func_id: FuncId, - ctx: &cranelift_codegen::Context, + ctx: &mut cranelift_codegen::Context, declarations: &ModuleDeclarations, - code_size: u32, trap_sink: &mut TS, - ) -> ModuleResult<()> + ) -> ModuleResult where TS: TrapSink, { + info!( + "defining function {}: {}", + func_id, + ctx.func.display(self.isa()) + ); + let CodeInfo { + total_size: code_size, + .. + } = ctx.compile(self.isa())?; + let decl = declarations.get_function_decl(func_id); if !decl.linkage.is_definable() { return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); @@ -267,7 +278,8 @@ impl Backend for ObjectBackend { relocs: reloc_sink.relocs, }); } - Ok(()) + + Ok(ModuleCompiledFunction { size: code_size }) } fn define_function_bytes( @@ -275,12 +287,19 @@ impl Backend for ObjectBackend { func_id: FuncId, bytes: &[u8], declarations: &ModuleDeclarations, - ) -> ModuleResult<()> { + ) -> ModuleResult { + info!("defining function {} with bytes", func_id); + let decl = declarations.get_function_decl(func_id); if !decl.linkage.is_definable() { return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); } + let total_size: u32 = match bytes.len().try_into() { + Ok(total_size) => total_size, + _ => Err(ModuleError::FunctionTooLarge(decl.name.clone()))?, + }; + let &mut (symbol, ref mut defined) = self.functions[func_id].as_mut().unwrap(); if *defined { return Err(ModuleError::DuplicateDefinition(decl.name.clone())); @@ -304,7 +323,7 @@ impl Backend for ObjectBackend { .add_symbol_data(symbol, section, bytes, self.function_alignment); } - Ok(()) + Ok(ModuleCompiledFunction { size: total_size }) } fn define_data( diff --git a/cranelift/simplejit/Cargo.toml b/cranelift/simplejit/Cargo.toml index dcbdbd7300..623a56add6 100644 --- a/cranelift/simplejit/Cargo.toml +++ b/cranelift/simplejit/Cargo.toml @@ -19,6 +19,7 @@ libc = { version = "0.2.42" } errno = "0.2.4" target-lexicon = "0.11" memmap = { version = "0.7.0", optional = true } +log = { version = "0.4.6", default-features = false } [target.'cfg(target_os = "windows")'.dependencies] winapi = { version = "0.3", features = ["winbase", "memoryapi"] } diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 391eaa517e..a5429a4297 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -2,7 +2,7 @@ use crate::memory::Memory; use cranelift_codegen::binemit::{ - Addend, CodeOffset, Reloc, RelocSink, StackMap, StackMapSink, TrapSink, + Addend, CodeInfo, CodeOffset, Reloc, RelocSink, StackMap, StackMapSink, TrapSink, }; use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::settings::Configurable; @@ -10,12 +10,14 @@ use cranelift_codegen::{self, ir, settings}; use cranelift_entity::SecondaryMap; use cranelift_module::{ Backend, DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, - ModuleDeclarations, ModuleError, ModuleResult, + ModuleCompiledFunction, ModuleDeclarations, ModuleError, ModuleResult, }; use cranelift_native; #[cfg(not(windows))] use libc; +use log::info; use std::collections::HashMap; +use std::convert::TryInto; use std::ffi::CString; use std::io::Write; use std::ptr; @@ -413,14 +415,19 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { fn define_function( &mut self, id: FuncId, - ctx: &cranelift_codegen::Context, + ctx: &mut cranelift_codegen::Context, declarations: &ModuleDeclarations, - code_size: u32, trap_sink: &mut TS, - ) -> ModuleResult<()> + ) -> ModuleResult where TS: TrapSink, { + info!("defining function {}: {}", id, ctx.func.display(self.isa())); + let CodeInfo { + total_size: code_size, + .. + } = ctx.compile(self.isa())?; + let decl = declarations.get_function_decl(id); if !decl.linkage.is_definable() { return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); @@ -458,7 +465,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { relocs: reloc_sink.relocs, }); - Ok(()) + Ok(ModuleCompiledFunction { size: code_size }) } fn define_function_bytes( @@ -466,12 +473,17 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { id: FuncId, bytes: &[u8], declarations: &ModuleDeclarations, - ) -> ModuleResult<()> { + ) -> ModuleResult { let decl = declarations.get_function_decl(id); if !decl.linkage.is_definable() { return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); } + let total_size: u32 = match bytes.len().try_into() { + Ok(total_size) => total_size, + _ => Err(ModuleError::FunctionTooLarge(decl.name.clone()))?, + }; + if !self.functions[id].is_none() { return Err(ModuleError::DuplicateDefinition(decl.name.to_owned())); } @@ -496,7 +508,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { relocs: vec![], }); - Ok(()) + Ok(ModuleCompiledFunction { size: total_size }) } fn define_data( From b44c5bb2bef5c6567c96aca6acc529ccd9e866ff Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 18:33:29 +0200 Subject: [PATCH 12/17] Move ModuleDeclarations to backends --- cranelift/module/src/backend.rs | 20 ++++--- cranelift/module/src/module.rs | 39 ++++---------- cranelift/object/src/backend.rs | 69 ++++++++++++++---------- cranelift/simplejit/src/backend.rs | 85 ++++++++++++++++-------------- 4 files changed, 106 insertions(+), 107 deletions(-) diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs index a72adf00cd..9d12fa17a9 100644 --- a/cranelift/module/src/backend.rs +++ b/cranelift/module/src/backend.rs @@ -44,28 +44,34 @@ where /// Return the `TargetIsa` to compile for. fn isa(&self) -> &dyn TargetIsa; + /// Get all declarations in this module. + fn declarations(&self) -> &ModuleDeclarations; + /// Declare a function. - fn declare_function(&mut self, id: FuncId, name: &str, linkage: Linkage); + fn declare_function( + &mut self, + name: &str, + linkage: Linkage, + signature: &ir::Signature, + ) -> ModuleResult; /// Declare a data object. fn declare_data( &mut self, - id: DataId, name: &str, linkage: Linkage, writable: bool, tls: bool, align: Option, - ); + ) -> ModuleResult; /// Define a function, producing the function body from the given `Context`. /// /// Functions must be declared before being defined. fn define_function( &mut self, - id: FuncId, + func: FuncId, ctx: &mut Context, - declarations: &ModuleDeclarations, trap_sink: &mut TS, ) -> ModuleResult where @@ -78,7 +84,6 @@ where &mut self, id: FuncId, bytes: &[u8], - declarations: &ModuleDeclarations, ) -> ModuleResult; /// Define a zero-initialized data object of the given size. @@ -88,12 +93,11 @@ where &mut self, id: DataId, data_ctx: &DataContext, - declarations: &ModuleDeclarations, ) -> ModuleResult<()>; /// Consume this `Backend` and return a result. Some implementations may /// provide additional functionality through this result. - fn finish(self, declarations: ModuleDeclarations) -> Self::Product; + fn finish(self) -> Self::Product; } /// Default names for `ir::LibCall`s. A function by this name is imported into the object as diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 62050142bb..239ceb4d1f 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -327,7 +327,6 @@ pub struct Module where B: Backend, { - declarations: ModuleDeclarations, backend: B, } @@ -344,11 +343,6 @@ where /// Create a new `Module`. pub fn new(backend_builder: B::Builder) -> Self { Self { - declarations: ModuleDeclarations { - names: HashMap::new(), - functions: PrimaryMap::new(), - data_objects: PrimaryMap::new(), - }, backend: B::new(backend_builder), } } @@ -356,7 +350,7 @@ where /// Get the module identifier for a given name, if that name /// has been declared. pub fn get_name(&self, name: &str) -> Option { - self.declarations.names.get(name).cloned() + self.backend.declarations().get_name(name) } /// Return the target information needed by frontends to produce Cranelift IR @@ -406,16 +400,7 @@ where linkage: Linkage, signature: &ir::Signature, ) -> ModuleResult { - let (id, decl) = self - .declarations - .declare_function(name, linkage, signature)?; - self.backend.declare_function(id, name, decl.linkage); - Ok(id) - } - - /// An iterator over functions that have been declared in this module. - pub fn declared_functions(&self) -> core::slice::Iter<'_, FunctionDeclaration> { - self.declarations.functions.values() + self.backend.declare_function(name, linkage, signature) } /// Declare a data object in this module. @@ -427,12 +412,8 @@ where tls: bool, align: Option, // An alignment bigger than 128 is unlikely ) -> ModuleResult { - let (id, decl) = self - .declarations - .declare_data(name, linkage, writable, tls, align)?; self.backend - .declare_data(id, name, decl.linkage, decl.writable, decl.tls, decl.align); - Ok(id) + .declare_data(name, linkage, writable, tls, align) } /// Use this when you're building the IR of a function to reference a function. @@ -440,7 +421,7 @@ where /// TODO: Coalesce redundant decls and signatures. /// TODO: Look into ways to reduce the risk of using a FuncRef in the wrong function. pub fn declare_func_in_func(&self, func: FuncId, in_func: &mut ir::Function) -> ir::FuncRef { - let decl = &self.declarations.functions[func]; + let decl = &self.backend.declarations().functions[func]; let signature = in_func.import_signature(decl.signature.clone()); let colocated = decl.linkage.is_final(); in_func.import_function(ir::ExtFuncData { @@ -454,7 +435,7 @@ where /// /// TODO: Same as above. pub fn declare_data_in_func(&self, data: DataId, func: &mut ir::Function) -> ir::GlobalValue { - let decl = &self.declarations.data_objects[data]; + let decl = &self.backend.declarations().data_objects[data]; let colocated = decl.linkage.is_final(); func.create_global_value(ir::GlobalValueData::Symbol { name: ir::ExternalName::user(1, data.as_u32()), @@ -488,8 +469,7 @@ where where TS: binemit::TrapSink, { - self.backend - .define_function(func, ctx, &self.declarations, trap_sink) + self.backend.define_function(func, ctx, trap_sink) } /// Define a function, taking the function body from the given `bytes`. @@ -504,13 +484,12 @@ where func: FuncId, bytes: &[u8], ) -> ModuleResult { - self.backend - .define_function_bytes(func, bytes, &self.declarations) + self.backend.define_function_bytes(func, bytes) } /// Define a data object, producing the data contents from the given `DataContext`. pub fn define_data(&mut self, data: DataId, data_ctx: &DataContext) -> ModuleResult<()> { - self.backend.define_data(data, data_ctx, &self.declarations) + self.backend.define_data(data, data_ctx) } /// Return the target isa @@ -522,6 +501,6 @@ where /// implementations may provide additional functionality available after /// a `Module` is complete. pub fn finish(self) -> B::Product { - self.backend.finish(self.declarations) + self.backend.finish() } } diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index 4a372a8be5..c0bb4c46a5 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -114,6 +114,7 @@ impl ObjectBuilder { pub struct ObjectBackend { isa: Box, object: Object, + declarations: ModuleDeclarations, functions: SecondaryMap>, data_objects: SecondaryMap>, relocs: Vec, @@ -135,6 +136,7 @@ impl Backend for ObjectBackend { Self { isa: builder.isa, object, + declarations: ModuleDeclarations::default(), functions: SecondaryMap::new(), data_objects: SecondaryMap::new(), relocs: Vec::new(), @@ -149,8 +151,21 @@ impl Backend for ObjectBackend { &*self.isa } - fn declare_function(&mut self, id: FuncId, name: &str, linkage: Linkage) { - let (scope, weak) = translate_linkage(linkage); + fn declarations(&self) -> &ModuleDeclarations { + &self.declarations + } + + fn declare_function( + &mut self, + name: &str, + linkage: Linkage, + signature: &ir::Signature, + ) -> ModuleResult { + let (id, decl) = self + .declarations + .declare_function(name, linkage, signature)?; + + let (scope, weak) = translate_linkage(decl.linkage); if let Some((function, _defined)) = self.functions[id] { let symbol = self.object.symbol_mut(function); @@ -169,23 +184,28 @@ impl Backend for ObjectBackend { }); self.functions[id] = Some((symbol_id, false)); } + + Ok(id) } fn declare_data( &mut self, - id: DataId, name: &str, linkage: Linkage, - _writable: bool, + writable: bool, tls: bool, - _align: Option, - ) { - let kind = if tls { + align: Option, + ) -> ModuleResult { + let (id, decl) = self + .declarations + .declare_data(name, linkage, writable, tls, align)?; + + let kind = if decl.tls { SymbolKind::Tls } else { SymbolKind::Data }; - let (scope, weak) = translate_linkage(linkage); + let (scope, weak) = translate_linkage(decl.linkage); if let Some((data, _defined)) = self.data_objects[id] { let symbol = self.object.symbol_mut(data); @@ -205,13 +225,14 @@ impl Backend for ObjectBackend { }); self.data_objects[id] = Some((symbol_id, false)); } + + Ok(id) } fn define_function( &mut self, func_id: FuncId, ctx: &mut cranelift_codegen::Context, - declarations: &ModuleDeclarations, trap_sink: &mut TS, ) -> ModuleResult where @@ -227,7 +248,7 @@ impl Backend for ObjectBackend { .. } = ctx.compile(self.isa())?; - let decl = declarations.get_function_decl(func_id); + let decl = self.declarations.get_function_decl(func_id); if !decl.linkage.is_definable() { return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); } @@ -286,11 +307,10 @@ impl Backend for ObjectBackend { &mut self, func_id: FuncId, bytes: &[u8], - declarations: &ModuleDeclarations, ) -> ModuleResult { info!("defining function {} with bytes", func_id); - let decl = declarations.get_function_decl(func_id); + let decl = self.declarations.get_function_decl(func_id); if !decl.linkage.is_definable() { return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); } @@ -326,13 +346,8 @@ impl Backend for ObjectBackend { Ok(ModuleCompiledFunction { size: total_size }) } - fn define_data( - &mut self, - data_id: DataId, - data_ctx: &DataContext, - declarations: &ModuleDeclarations, - ) -> ModuleResult<()> { - let decl = declarations.get_data_decl(data_id); + fn define_data(&mut self, data_id: DataId, data_ctx: &DataContext) -> ModuleResult<()> { + let decl = self.declarations.get_data_decl(data_id); if !decl.linkage.is_definable() { return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); } @@ -438,7 +453,7 @@ impl Backend for ObjectBackend { Ok(()) } - fn finish(mut self, declarations: ModuleDeclarations) -> ObjectProduct { + fn finish(mut self) -> ObjectProduct { let symbol_relocs = mem::take(&mut self.relocs); for symbol in symbol_relocs { for &RelocRecord { @@ -450,7 +465,7 @@ impl Backend for ObjectBackend { addend, } in &symbol.relocs { - let target_symbol = self.get_symbol(&declarations, name); + let target_symbol = self.get_symbol(name); self.object .add_relocation( symbol.section, @@ -487,18 +502,14 @@ impl Backend for ObjectBackend { impl ObjectBackend { // This should only be called during finish because it creates // symbols for missing libcalls. - fn get_symbol( - &mut self, - declarations: &ModuleDeclarations, - name: &ir::ExternalName, - ) -> SymbolId { + fn get_symbol(&mut self, name: &ir::ExternalName) -> SymbolId { match *name { ir::ExternalName::User { .. } => { - if declarations.is_function(name) { - let id = declarations.get_function_id(name); + if self.declarations.is_function(name) { + let id = self.declarations.get_function_id(name); self.functions[id].unwrap().0 } else { - let id = declarations.get_data_id(name); + let id = self.declarations.get_data_id(name); self.data_objects[id].unwrap().0 } } diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index a5429a4297..fcfd72c3b4 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -127,6 +127,7 @@ pub struct SimpleJITBackend { symbols: HashMap, libcall_names: Box String>, memory: SimpleJITMemoryHandle, + declarations: ModuleDeclarations, functions: SecondaryMap>, data_objects: SecondaryMap>, functions_to_finalize: Vec, @@ -224,24 +225,20 @@ impl SimpleJITBackend { } } - fn get_definition( - &self, - declarations: &ModuleDeclarations, - name: &ir::ExternalName, - ) -> *const u8 { + fn get_definition(&self, name: &ir::ExternalName) -> *const u8 { match *name { ir::ExternalName::User { .. } => { - if declarations.is_function(name) { - let func_id = declarations.get_function_id(name); + if self.declarations.is_function(name) { + let func_id = self.declarations.get_function_id(name); match &self.functions[func_id] { Some(compiled) => compiled.code, - None => self.lookup_symbol(&declarations.get_function_decl(func_id).name), + None => self.lookup_symbol(&self.declarations.get_function_decl(func_id).name), } } else { - let data_id = declarations.get_data_id(name); + let data_id = self.declarations.get_data_id(name); match &self.data_objects[data_id] { Some(compiled) => compiled.storage, - None => self.lookup_symbol(&declarations.get_data_decl(data_id).name), + None => self.lookup_symbol(&self.declarations.get_data_decl(data_id).name), } } } @@ -270,7 +267,7 @@ impl SimpleJITBackend { } } - fn finalize_function(&mut self, id: FuncId, declarations: &ModuleDeclarations) { + fn finalize_function(&mut self, id: FuncId) { use std::ptr::write_unaligned; let func = self.functions[id] @@ -287,7 +284,7 @@ impl SimpleJITBackend { let ptr = func.code; debug_assert!((offset as usize) < func.size); let at = unsafe { ptr.offset(offset as isize) }; - let base = self.get_definition(declarations, name); + let base = self.get_definition(name); // TODO: Handle overflow. let what = unsafe { base.offset(addend as isize) }; match reloc { @@ -318,7 +315,7 @@ impl SimpleJITBackend { } } - fn finalize_data(&mut self, id: DataId, declarations: &ModuleDeclarations) { + fn finalize_data(&mut self, id: DataId) { use std::ptr::write_unaligned; let data = self.data_objects[id] @@ -335,7 +332,7 @@ impl SimpleJITBackend { let ptr = data.storage; debug_assert!((offset as usize) < data.size); let at = unsafe { ptr.offset(offset as isize) }; - let base = self.get_definition(declarations, name); + let base = self.get_definition(name); // TODO: Handle overflow. let what = unsafe { base.offset(addend as isize) }; match reloc { @@ -384,6 +381,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { symbols: builder.symbols, libcall_names: builder.libcall_names, memory, + declarations: ModuleDeclarations::default(), functions: SecondaryMap::new(), data_objects: SecondaryMap::new(), functions_to_finalize: Vec::new(), @@ -395,28 +393,41 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { &*self.isa } - fn declare_function(&mut self, _id: FuncId, _name: &str, _linkage: Linkage) { - // Nothing to do. + fn declarations(&self) -> &ModuleDeclarations { + &self.declarations + } + + fn declare_function( + &mut self, + name: &str, + linkage: Linkage, + signature: &ir::Signature, + ) -> ModuleResult { + let (id, _decl) = self + .declarations + .declare_function(name, linkage, signature)?; + Ok(id) } fn declare_data( &mut self, - _id: DataId, - _name: &str, - _linkage: Linkage, - _writable: bool, + name: &str, + linkage: Linkage, + writable: bool, tls: bool, - _align: Option, - ) { + align: Option, + ) -> ModuleResult { assert!(!tls, "SimpleJIT doesn't yet support TLS"); - // Nothing to do. + let (id, _decl) = self + .declarations + .declare_data(name, linkage, writable, tls, align)?; + Ok(id) } fn define_function( &mut self, id: FuncId, ctx: &mut cranelift_codegen::Context, - declarations: &ModuleDeclarations, trap_sink: &mut TS, ) -> ModuleResult where @@ -428,7 +439,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { .. } = ctx.compile(self.isa())?; - let decl = declarations.get_function_decl(id); + let decl = self.declarations.get_function_decl(id); if !decl.linkage.is_definable() { return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); } @@ -472,9 +483,8 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { &mut self, id: FuncId, bytes: &[u8], - declarations: &ModuleDeclarations, ) -> ModuleResult { - let decl = declarations.get_function_decl(id); + let decl = self.declarations.get_function_decl(id); if !decl.linkage.is_definable() { return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); } @@ -511,13 +521,8 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { Ok(ModuleCompiledFunction { size: total_size }) } - fn define_data( - &mut self, - id: DataId, - data: &DataContext, - declarations: &ModuleDeclarations, - ) -> ModuleResult<()> { - let decl = declarations.get_data_decl(id); + fn define_data(&mut self, id: DataId, data: &DataContext) -> ModuleResult<()> { + let decl = self.declarations.get_data_decl(id); if !decl.linkage.is_definable() { return Err(ModuleError::InvalidImportDefinition(decl.name.clone())); } @@ -604,16 +609,16 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { /// /// This method does not need to be called when access to the memory /// handle is not required. - fn finish(mut self, declarations: ModuleDeclarations) -> Self::Product { + fn finish(mut self) -> Self::Product { for func in std::mem::take(&mut self.functions_to_finalize) { - let decl = declarations.get_function_decl(func); + let decl = self.declarations.get_function_decl(func); debug_assert!(decl.linkage.is_definable()); - self.finalize_function(func, &declarations); + self.finalize_function(func); } for data in std::mem::take(&mut self.data_objects_to_finalize) { - let decl = declarations.get_data_decl(data); + let decl = self.declarations.get_data_decl(data); debug_assert!(decl.linkage.is_definable()); - self.finalize_data(data, &declarations); + self.finalize_data(data); } // Now that we're done patching, prepare the memory for execution! @@ -622,7 +627,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { SimpleJITProduct { memory: self.memory, - declarations, + declarations: self.declarations, functions: self.functions, data_objects: self.data_objects, } From 84c6ec32148307716edecf48e2a32710be7387b7 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 19:23:23 +0200 Subject: [PATCH 13/17] Move alignment config from declare_data to define_data --- cranelift/module/src/backend.rs | 1 - cranelift/module/src/data_context.rs | 11 +++++++++++ cranelift/module/src/module.rs | 11 +++-------- cranelift/object/src/backend.rs | 6 +++--- cranelift/simplejit/src/backend.rs | 14 +++++++------- cranelift/simplejit/src/memory.rs | 10 ++++++---- 6 files changed, 30 insertions(+), 23 deletions(-) diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs index 9d12fa17a9..9b9df893b2 100644 --- a/cranelift/module/src/backend.rs +++ b/cranelift/module/src/backend.rs @@ -62,7 +62,6 @@ where linkage: Linkage, writable: bool, tls: bool, - align: Option, ) -> ModuleResult; /// Define a function, producing the function body from the given `Context`. diff --git a/cranelift/module/src/data_context.rs b/cranelift/module/src/data_context.rs index bca8471a77..b7b3e6c18f 100644 --- a/cranelift/module/src/data_context.rs +++ b/cranelift/module/src/data_context.rs @@ -50,6 +50,8 @@ pub struct DataDescription { pub data_relocs: Vec<(CodeOffset, ir::GlobalValue, Addend)>, /// Object file section pub custom_segment_section: Option<(String, String)>, + /// Alignment + pub align: Option, } /// This is to data objects what cranelift_codegen::Context is to functions. @@ -68,6 +70,7 @@ impl DataContext { function_relocs: vec![], data_relocs: vec![], custom_segment_section: None, + align: None, }, } } @@ -79,6 +82,8 @@ impl DataContext { self.description.data_decls.clear(); self.description.function_relocs.clear(); self.description.data_relocs.clear(); + self.description.custom_segment_section = None; + self.description.align = None; } /// Define a zero-initialized object with the given size. @@ -100,6 +105,12 @@ impl DataContext { self.description.custom_segment_section = Some((seg.to_owned(), sec.to_owned())) } + /// Set the alignment for data. The alignment must be a power of two. + pub fn set_align(&mut self, align: u64) { + assert!(align.is_power_of_two()); + self.description.align = Some(align); + } + /// Declare an external function import. /// /// Users of the `Module` API generally should call diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 239ceb4d1f..bc1d0ac2f8 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -182,14 +182,12 @@ pub struct DataDeclaration { pub linkage: Linkage, pub writable: bool, pub tls: bool, - pub align: Option, } impl DataDeclaration { - fn merge(&mut self, linkage: Linkage, writable: bool, tls: bool, align: Option) { + fn merge(&mut self, linkage: Linkage, writable: bool, tls: bool) { self.linkage = Linkage::merge(self.linkage, linkage); self.writable = self.writable || writable; - self.align = self.align.max(align); assert_eq!( self.tls, tls, "Can't change TLS data object to normal or in the opposite way", @@ -291,7 +289,6 @@ impl ModuleDeclarations { linkage: Linkage, writable: bool, tls: bool, - align: Option, // An alignment bigger than 128 is unlikely ) -> ModuleResult<(DataId, &DataDeclaration)> { // TODO: Can we avoid allocating names so often? use super::hash_map::Entry::*; @@ -299,7 +296,7 @@ impl ModuleDeclarations { Occupied(entry) => match *entry.get() { FuncOrDataId::Data(id) => { let existing = &mut self.data_objects[id]; - existing.merge(linkage, writable, tls, align); + existing.merge(linkage, writable, tls); Ok((id, existing)) } @@ -313,7 +310,6 @@ impl ModuleDeclarations { linkage, writable, tls, - align, }); entry.insert(FuncOrDataId::Data(id)); Ok((id, &self.data_objects[id])) @@ -410,10 +406,9 @@ where linkage: Linkage, writable: bool, tls: bool, - align: Option, // An alignment bigger than 128 is unlikely ) -> ModuleResult { self.backend - .declare_data(name, linkage, writable, tls, align) + .declare_data(name, linkage, writable, tls) } /// Use this when you're building the IR of a function to reference a function. diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index c0bb4c46a5..e4faed3e4a 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -194,11 +194,10 @@ impl Backend for ObjectBackend { linkage: Linkage, writable: bool, tls: bool, - align: Option, ) -> ModuleResult { let (id, decl) = self .declarations - .declare_data(name, linkage, writable, tls, align)?; + .declare_data(name, linkage, writable, tls)?; let kind = if decl.tls { SymbolKind::Tls @@ -365,6 +364,7 @@ impl Backend for ObjectBackend { ref function_relocs, ref data_relocs, ref custom_segment_section, + align, } = data_ctx.description(); let reloc_size = match self.isa.triple().pointer_width().unwrap() { @@ -431,7 +431,7 @@ impl Backend for ObjectBackend { ) }; - let align = u64::from(decl.align.unwrap_or(1)); + let align = align.unwrap_or(1); let offset = match *init { Init::Uninitialized => { panic!("data is not initialized yet"); diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index fcfd72c3b4..9910fdc625 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -25,9 +25,9 @@ use target_lexicon::PointerWidth; #[cfg(windows)] use winapi; -const EXECUTABLE_DATA_ALIGNMENT: u8 = 0x10; -const WRITABLE_DATA_ALIGNMENT: u8 = 0x8; -const READONLY_DATA_ALIGNMENT: u8 = 0x1; +const EXECUTABLE_DATA_ALIGNMENT: u64 = 0x10; +const WRITABLE_DATA_ALIGNMENT: u64 = 0x8; +const READONLY_DATA_ALIGNMENT: u64 = 0x1; /// A builder for `SimpleJITBackend`. pub struct SimpleJITBuilder { @@ -415,12 +415,11 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { linkage: Linkage, writable: bool, tls: bool, - align: Option, ) -> ModuleResult { assert!(!tls, "SimpleJIT doesn't yet support TLS"); let (id, _decl) = self .declarations - .declare_data(name, linkage, writable, tls, align)?; + .declare_data(name, linkage, writable, tls)?; Ok(id) } @@ -542,18 +541,19 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { ref function_relocs, ref data_relocs, custom_segment_section: _, + align, } = data.description(); let size = init.size(); let storage = if decl.writable { self.memory .writable - .allocate(size, decl.align.unwrap_or(WRITABLE_DATA_ALIGNMENT)) + .allocate(size, align.unwrap_or(WRITABLE_DATA_ALIGNMENT)) .expect("TODO: handle OOM etc.") } else { self.memory .readonly - .allocate(size, decl.align.unwrap_or(READONLY_DATA_ALIGNMENT)) + .allocate(size, align.unwrap_or(READONLY_DATA_ALIGNMENT)) .expect("TODO: handle OOM etc.") }; diff --git a/cranelift/simplejit/src/memory.rs b/cranelift/simplejit/src/memory.rs index 2618233c63..7d0d84311b 100644 --- a/cranelift/simplejit/src/memory.rs +++ b/cranelift/simplejit/src/memory.rs @@ -8,6 +8,7 @@ use libc; use memmap::MmapMut; use region; +use std::convert::TryFrom; use std::mem; use std::ptr; @@ -149,10 +150,11 @@ impl Memory { } /// TODO: Use a proper error type. - pub fn allocate(&mut self, size: usize, align: u8) -> Result<*mut u8, String> { - if self.position % align as usize != 0 { - self.position += align as usize - self.position % align as usize; - debug_assert!(self.position % align as usize == 0); + pub fn allocate(&mut self, size: usize, align: u64) -> Result<*mut u8, String> { + let align = usize::try_from(align).expect("alignment too big"); + if self.position % align != 0 { + self.position += align - self.position % align; + debug_assert!(self.position % align == 0); } if size <= self.current.len - self.position { From d84ca235d2a627ce674738a47a10841c59a777e8 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 19:52:57 +0200 Subject: [PATCH 14/17] Remove Backend trait and turn Module into a trait --- cranelift/module/src/backend.rs | 128 ----------------------------- cranelift/module/src/lib.rs | 35 +++++++- cranelift/module/src/module.rs | 94 +++++++-------------- cranelift/object/src/backend.rs | 31 ++++--- cranelift/object/src/lib.rs | 2 +- cranelift/simplejit/src/backend.rs | 36 ++++---- cranelift/simplejit/src/lib.rs | 2 +- 7 files changed, 95 insertions(+), 233 deletions(-) delete mode 100644 cranelift/module/src/backend.rs diff --git a/cranelift/module/src/backend.rs b/cranelift/module/src/backend.rs deleted file mode 100644 index 9b9df893b2..0000000000 --- a/cranelift/module/src/backend.rs +++ /dev/null @@ -1,128 +0,0 @@ -//! Defines the `Backend` trait. - -use crate::module::ModuleCompiledFunction; -use crate::DataContext; -use crate::DataId; -use crate::FuncId; -use crate::Linkage; -use crate::ModuleDeclarations; -use crate::ModuleResult; -use core::marker; -use cranelift_codegen::isa::TargetIsa; -use cranelift_codegen::Context; -use cranelift_codegen::{binemit, ir}; - -use std::borrow::ToOwned; -use std::boxed::Box; -use std::string::String; - -/// A `Backend` implements the functionality needed to support a `Module`. -/// -/// Three notable implementations of this trait are: -/// - `SimpleJITBackend`, defined in [cranelift-simplejit], which JITs -/// the contents of a `Module` to memory which can be directly executed. -/// - `ObjectBackend`, defined in [cranelift-object], which writes the -/// contents of a `Module` out as a native object file. -/// -/// [cranelift-simplejit]: https://docs.rs/cranelift-simplejit/ -/// [cranelift-object]: https://docs.rs/cranelift-object/ -pub trait Backend -where - Self: marker::Sized, -{ - /// A builder for constructing `Backend` instances. - type Builder; - - /// This is an object returned by `Module`'s - /// [`finish`](struct.Module.html#method.finish) function, - /// if the `Backend` has a purpose for this. - type Product; - - /// Create a new `Backend` instance. - fn new(_: Self::Builder) -> Self; - - /// Return the `TargetIsa` to compile for. - fn isa(&self) -> &dyn TargetIsa; - - /// Get all declarations in this module. - fn declarations(&self) -> &ModuleDeclarations; - - /// Declare a function. - fn declare_function( - &mut self, - name: &str, - linkage: Linkage, - signature: &ir::Signature, - ) -> ModuleResult; - - /// Declare a data object. - fn declare_data( - &mut self, - name: &str, - linkage: Linkage, - writable: bool, - tls: bool, - ) -> ModuleResult; - - /// Define a function, producing the function body from the given `Context`. - /// - /// Functions must be declared before being defined. - fn define_function( - &mut self, - func: FuncId, - ctx: &mut Context, - trap_sink: &mut TS, - ) -> ModuleResult - where - TS: binemit::TrapSink; - - /// Define a function, taking the function body from the given `bytes`. - /// - /// Functions must be declared before being defined. - fn define_function_bytes( - &mut self, - id: FuncId, - bytes: &[u8], - ) -> ModuleResult; - - /// Define a zero-initialized data object of the given size. - /// - /// Data objects must be declared before being defined. - fn define_data( - &mut self, - id: DataId, - data_ctx: &DataContext, - ) -> ModuleResult<()>; - - /// Consume this `Backend` and return a result. Some implementations may - /// provide additional functionality through this result. - fn finish(self) -> Self::Product; -} - -/// Default names for `ir::LibCall`s. A function by this name is imported into the object as -/// part of the translation of a `ir::ExternalName::LibCall` variant. -pub fn default_libcall_names() -> Box String> { - Box::new(move |libcall| match libcall { - ir::LibCall::Probestack => "__cranelift_probestack".to_owned(), - ir::LibCall::UdivI64 => "__udivdi3".to_owned(), - ir::LibCall::SdivI64 => "__divdi3".to_owned(), - ir::LibCall::UremI64 => "__umoddi3".to_owned(), - ir::LibCall::SremI64 => "__moddi3".to_owned(), - ir::LibCall::IshlI64 => "__ashldi3".to_owned(), - ir::LibCall::UshrI64 => "__lshrdi3".to_owned(), - ir::LibCall::SshrI64 => "__ashrdi3".to_owned(), - ir::LibCall::CeilF32 => "ceilf".to_owned(), - ir::LibCall::CeilF64 => "ceil".to_owned(), - ir::LibCall::FloorF32 => "floorf".to_owned(), - ir::LibCall::FloorF64 => "floor".to_owned(), - ir::LibCall::TruncF32 => "truncf".to_owned(), - ir::LibCall::TruncF64 => "trunc".to_owned(), - ir::LibCall::NearestF32 => "nearbyintf".to_owned(), - ir::LibCall::NearestF64 => "nearbyint".to_owned(), - ir::LibCall::Memcpy => "memcpy".to_owned(), - ir::LibCall::Memset => "memset".to_owned(), - ir::LibCall::Memmove => "memmove".to_owned(), - - ir::LibCall::ElfTlsGetAddr => "__tls_get_addr".to_owned(), - }) -} diff --git a/cranelift/module/src/lib.rs b/cranelift/module/src/lib.rs index b04d29b87c..9292a9b188 100644 --- a/cranelift/module/src/lib.rs +++ b/cranelift/module/src/lib.rs @@ -29,15 +29,18 @@ extern crate std; #[cfg(not(feature = "std"))] use hashbrown::{hash_map, HashMap}; +use std::borrow::ToOwned; +use std::boxed::Box; #[cfg(feature = "std")] use std::collections::{hash_map, HashMap}; +use std::string::String; + +use cranelift_codegen::ir; -mod backend; mod data_context; mod module; mod traps; -pub use crate::backend::{default_libcall_names, Backend}; pub use crate::data_context::{DataContext, DataDescription, Init}; pub use crate::module::{ DataId, FuncId, FuncOrDataId, Linkage, Module, ModuleCompiledFunction, ModuleDeclarations, @@ -47,3 +50,31 @@ pub use crate::traps::TrapSite; /// Version number of this crate. pub const VERSION: &str = env!("CARGO_PKG_VERSION"); + +/// Default names for `ir::LibCall`s. A function by this name is imported into the object as +/// part of the translation of a `ir::ExternalName::LibCall` variant. +pub fn default_libcall_names() -> Box String> { + Box::new(move |libcall| match libcall { + ir::LibCall::Probestack => "__cranelift_probestack".to_owned(), + ir::LibCall::UdivI64 => "__udivdi3".to_owned(), + ir::LibCall::SdivI64 => "__divdi3".to_owned(), + ir::LibCall::UremI64 => "__umoddi3".to_owned(), + ir::LibCall::SremI64 => "__moddi3".to_owned(), + ir::LibCall::IshlI64 => "__ashldi3".to_owned(), + ir::LibCall::UshrI64 => "__lshrdi3".to_owned(), + ir::LibCall::SshrI64 => "__ashrdi3".to_owned(), + ir::LibCall::CeilF32 => "ceilf".to_owned(), + ir::LibCall::CeilF64 => "ceil".to_owned(), + ir::LibCall::FloorF32 => "floorf".to_owned(), + ir::LibCall::FloorF64 => "floor".to_owned(), + ir::LibCall::TruncF32 => "truncf".to_owned(), + ir::LibCall::TruncF64 => "trunc".to_owned(), + ir::LibCall::NearestF32 => "nearbyintf".to_owned(), + ir::LibCall::NearestF64 => "nearbyint".to_owned(), + ir::LibCall::Memcpy => "memcpy".to_owned(), + ir::LibCall::Memset => "memset".to_owned(), + ir::LibCall::Memmove => "memmove".to_owned(), + + ir::LibCall::ElfTlsGetAddr => "__tls_get_addr".to_owned(), + }) +} diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index bc1d0ac2f8..84fc19c96f 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -7,7 +7,6 @@ use super::HashMap; use crate::data_context::DataContext; -use crate::Backend; use cranelift_codegen::binemit; use cranelift_codegen::entity::{entity_impl, PrimaryMap}; use cranelift_codegen::{ir, isa, CodegenError, Context}; @@ -318,40 +317,29 @@ impl ModuleDeclarations { } } -/// A `Module` is a utility for collecting functions and data objects, and linking them together. -pub struct Module -where - B: Backend, -{ - backend: B, -} - /// Information about the compiled function. pub struct ModuleCompiledFunction { /// The size of the compiled function. pub size: binemit::CodeOffset, } -impl Module -where - B: Backend, -{ - /// Create a new `Module`. - pub fn new(backend_builder: B::Builder) -> Self { - Self { - backend: B::new(backend_builder), - } - } +/// A `Module` is a utility for collecting functions and data objects, and linking them together. +pub trait Module { + /// Return the `TargetIsa` to compile for. + fn isa(&self) -> &dyn isa::TargetIsa; + + /// Get all declarations in this module. + fn declarations(&self) -> &ModuleDeclarations; /// Get the module identifier for a given name, if that name /// has been declared. - pub fn get_name(&self, name: &str) -> Option { - self.backend.declarations().get_name(name) + fn get_name(&self, name: &str) -> Option { + self.declarations().get_name(name) } /// Return the target information needed by frontends to produce Cranelift IR /// for the current target. - pub fn target_config(&self) -> isa::TargetFrontendConfig { + fn target_config(&self) -> isa::TargetFrontendConfig { self.isa().frontend_config() } @@ -359,7 +347,7 @@ where /// /// This ensures that the `Context` is initialized with the default calling /// convention for the `TargetIsa`. - pub fn make_context(&self) -> Context { + fn make_context(&self) -> Context { let mut ctx = Context::new(); ctx.func.signature.call_conv = self.isa().default_call_conv(); ctx @@ -369,7 +357,7 @@ where /// /// This ensures that the `Context` is initialized with the default calling /// convention for the `TargetIsa`. - pub fn clear_context(&self, ctx: &mut Context) { + fn clear_context(&self, ctx: &mut Context) { ctx.clear(); ctx.func.signature.call_conv = self.isa().default_call_conv(); } @@ -377,7 +365,7 @@ where /// Create a new empty `Signature` with the default calling convention for /// the `TargetIsa`, to which parameter and return types can be added for /// declaring a function to be called by this `Module`. - pub fn make_signature(&self) -> ir::Signature { + fn make_signature(&self) -> ir::Signature { ir::Signature::new(self.isa().default_call_conv()) } @@ -385,38 +373,33 @@ where /// /// This ensures that the `Signature` is initialized with the default /// calling convention for the `TargetIsa`. - pub fn clear_signature(&self, sig: &mut ir::Signature) { + fn clear_signature(&self, sig: &mut ir::Signature) { sig.clear(self.isa().default_call_conv()); } /// Declare a function in this module. - pub fn declare_function( + fn declare_function( &mut self, name: &str, linkage: Linkage, signature: &ir::Signature, - ) -> ModuleResult { - self.backend.declare_function(name, linkage, signature) - } + ) -> ModuleResult; /// Declare a data object in this module. - pub fn declare_data( + fn declare_data( &mut self, name: &str, linkage: Linkage, writable: bool, tls: bool, - ) -> ModuleResult { - self.backend - .declare_data(name, linkage, writable, tls) - } + ) -> ModuleResult; /// Use this when you're building the IR of a function to reference a function. /// /// TODO: Coalesce redundant decls and signatures. /// TODO: Look into ways to reduce the risk of using a FuncRef in the wrong function. - pub fn declare_func_in_func(&self, func: FuncId, in_func: &mut ir::Function) -> ir::FuncRef { - let decl = &self.backend.declarations().functions[func]; + fn declare_func_in_func(&self, func: FuncId, in_func: &mut ir::Function) -> ir::FuncRef { + let decl = &self.declarations().functions[func]; let signature = in_func.import_signature(decl.signature.clone()); let colocated = decl.linkage.is_final(); in_func.import_function(ir::ExtFuncData { @@ -429,8 +412,8 @@ where /// Use this when you're building the IR of a function to reference a data object. /// /// TODO: Same as above. - pub fn declare_data_in_func(&self, data: DataId, func: &mut ir::Function) -> ir::GlobalValue { - let decl = &self.backend.declarations().data_objects[data]; + fn declare_data_in_func(&self, data: DataId, func: &mut ir::Function) -> ir::GlobalValue { + let decl = &self.declarations().data_objects[data]; let colocated = decl.linkage.is_final(); func.create_global_value(ir::GlobalValueData::Symbol { name: ir::ExternalName::user(1, data.as_u32()), @@ -441,12 +424,12 @@ where } /// TODO: Same as above. - pub fn declare_func_in_data(&self, func: FuncId, ctx: &mut DataContext) -> ir::FuncRef { + fn declare_func_in_data(&self, func: FuncId, ctx: &mut DataContext) -> ir::FuncRef { ctx.import_function(ir::ExternalName::user(0, func.as_u32())) } /// TODO: Same as above. - pub fn declare_data_in_data(&self, data: DataId, ctx: &mut DataContext) -> ir::GlobalValue { + fn declare_data_in_data(&self, data: DataId, ctx: &mut DataContext) -> ir::GlobalValue { ctx.import_global_value(ir::ExternalName::user(1, data.as_u32())) } @@ -455,17 +438,14 @@ where /// Returns the size of the function's code and constant data. /// /// Note: After calling this function the given `Context` will contain the compiled function. - pub fn define_function( + fn define_function( &mut self, func: FuncId, ctx: &mut Context, trap_sink: &mut TS, ) -> ModuleResult where - TS: binemit::TrapSink, - { - self.backend.define_function(func, ctx, trap_sink) - } + TS: binemit::TrapSink; /// Define a function, taking the function body from the given `bytes`. /// @@ -474,28 +454,12 @@ where /// `define_function`. /// /// Returns the size of the function's code. - pub fn define_function_bytes( + fn define_function_bytes( &mut self, func: FuncId, bytes: &[u8], - ) -> ModuleResult { - self.backend.define_function_bytes(func, bytes) - } + ) -> ModuleResult; /// Define a data object, producing the data contents from the given `DataContext`. - pub fn define_data(&mut self, data: DataId, data_ctx: &DataContext) -> ModuleResult<()> { - self.backend.define_data(data, data_ctx) - } - - /// Return the target isa - pub fn isa(&self) -> &dyn isa::TargetIsa { - self.backend.isa() - } - - /// Consume the module and return the resulting `Product`. Some `Backend` - /// implementations may provide additional functionality available after - /// a `Module` is complete. - pub fn finish(self) -> B::Product { - self.backend.finish() - } + fn define_data(&mut self, data: DataId, data_ctx: &DataContext) -> ModuleResult<()>; } diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index e4faed3e4a..a0c964cd09 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -8,7 +8,7 @@ use cranelift_codegen::entity::SecondaryMap; use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::{self, ir}; use cranelift_module::{ - Backend, DataContext, DataDescription, DataId, FuncId, Init, Linkage, ModuleCompiledFunction, + DataContext, DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleCompiledFunction, ModuleDeclarations, ModuleError, ModuleResult, }; use log::info; @@ -108,10 +108,10 @@ impl ObjectBuilder { } } -/// A `ObjectBackend` implements `Backend` and emits ".o" files using the `object` library. +/// An `ObjectModule` implements `Module` and emits ".o" files using the `object` library. /// -/// See the `ObjectBuilder` for a convenient way to construct `ObjectBackend` instances. -pub struct ObjectBackend { +/// See the `ObjectBuilder` for a convenient way to construct `ObjectModule` instances. +pub struct ObjectModule { isa: Box, object: Object, declarations: ModuleDeclarations, @@ -124,13 +124,9 @@ pub struct ObjectBackend { per_function_section: bool, } -impl Backend for ObjectBackend { - type Builder = ObjectBuilder; - - type Product = ObjectProduct; - - /// Create a new `ObjectBackend` using the given Cranelift target. - fn new(builder: ObjectBuilder) -> Self { +impl ObjectModule { + /// Create a new `ObjectModule` using the given Cranelift target. + pub fn new(builder: ObjectBuilder) -> Self { let mut object = Object::new(builder.binary_format, builder.architecture, builder.endian); object.add_file_symbol(builder.name); Self { @@ -146,7 +142,9 @@ impl Backend for ObjectBackend { per_function_section: builder.per_function_section, } } +} +impl Module for ObjectModule { fn isa(&self) -> &dyn TargetIsa { &*self.isa } @@ -452,8 +450,11 @@ impl Backend for ObjectBackend { } Ok(()) } +} - fn finish(mut self) -> ObjectProduct { +impl ObjectModule { + /// Finalize all relocations and output an object. + pub fn finish(mut self) -> ObjectProduct { let symbol_relocs = mem::take(&mut self.relocs); for symbol in symbol_relocs { for &RelocRecord { @@ -497,11 +498,9 @@ impl Backend for ObjectBackend { data_objects: self.data_objects, } } -} -impl ObjectBackend { - // This should only be called during finish because it creates - // symbols for missing libcalls. + /// This should only be called during finish because it creates + /// symbols for missing libcalls. fn get_symbol(&mut self, name: &ir::ExternalName) -> SymbolId { match *name { ir::ExternalName::User { .. } => { diff --git a/cranelift/object/src/lib.rs b/cranelift/object/src/lib.rs index 0eeaec1484..12ca1d7aca 100644 --- a/cranelift/object/src/lib.rs +++ b/cranelift/object/src/lib.rs @@ -27,7 +27,7 @@ mod backend; -pub use crate::backend::{ObjectBackend, ObjectBuilder, ObjectProduct}; +pub use crate::backend::{ObjectModule, ObjectBuilder, ObjectProduct}; /// Version number of this crate. pub const VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 9910fdc625..5d42be75fe 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -1,4 +1,4 @@ -//! Defines `SimpleJITBackend`. +//! Defines `SimpleJITModule`. use crate::memory::Memory; use cranelift_codegen::binemit::{ @@ -9,7 +9,7 @@ use cranelift_codegen::settings::Configurable; use cranelift_codegen::{self, ir, settings}; use cranelift_entity::SecondaryMap; use cranelift_module::{ - Backend, DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, + DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, Module, ModuleCompiledFunction, ModuleDeclarations, ModuleError, ModuleResult, }; use cranelift_native; @@ -29,7 +29,7 @@ const EXECUTABLE_DATA_ALIGNMENT: u64 = 0x10; const WRITABLE_DATA_ALIGNMENT: u64 = 0x8; const READONLY_DATA_ALIGNMENT: u64 = 0x1; -/// A builder for `SimpleJITBackend`. +/// A builder for `SimpleJITModule`. pub struct SimpleJITBuilder { isa: Box, symbols: HashMap, @@ -118,11 +118,11 @@ impl SimpleJITBuilder { } } -/// A `SimpleJITBackend` implements `Backend` and emits code and data into memory where it can be +/// A `SimpleJITModule` implements `Module` and emits code and data into memory where it can be /// directly called and accessed. /// -/// See the `SimpleJITBuilder` for a convenient way to construct `SimpleJITBackend` instances. -pub struct SimpleJITBackend { +/// See the `SimpleJITBuilder` for a convenient way to construct `SimpleJITModule` instances. +pub struct SimpleJITModule { isa: Box, symbols: HashMap, libcall_names: Box String>, @@ -217,7 +217,7 @@ impl SimpleJITProduct { } } -impl SimpleJITBackend { +impl SimpleJITModule { fn lookup_symbol(&self, name: &str) -> *const u8 { match self.symbols.get(name) { Some(&ptr) => ptr, @@ -232,7 +232,9 @@ impl SimpleJITBackend { let func_id = self.declarations.get_function_id(name); match &self.functions[func_id] { Some(compiled) => compiled.code, - None => self.lookup_symbol(&self.declarations.get_function_decl(func_id).name), + None => { + self.lookup_symbol(&self.declarations.get_function_decl(func_id).name) + } } } else { let data_id = self.declarations.get_data_id(name); @@ -357,19 +359,9 @@ impl SimpleJITBackend { } } } -} - -impl<'simple_jit_backend> Backend for SimpleJITBackend { - type Builder = SimpleJITBuilder; - - /// SimpleJIT emits code and data into memory as it processes them, so it - /// doesn't need to provide anything after the `Module` is complete. - /// The handle object that is returned can optionally be used to free - /// allocated memory if required. - type Product = SimpleJITProduct; /// Create a new `SimpleJITBackend`. - fn new(builder: SimpleJITBuilder) -> Self { + pub fn new(builder: SimpleJITBuilder) -> Self { let memory = SimpleJITMemoryHandle { code: Memory::new(), readonly: Memory::new(), @@ -388,7 +380,9 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { data_objects_to_finalize: Vec::new(), } } +} +impl<'simple_jit_backend> Module for SimpleJITModule { fn isa(&self) -> &dyn TargetIsa { &*self.isa } @@ -601,7 +595,9 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { Ok(()) } +} +impl SimpleJITModule { /// SimpleJIT emits code and data into memory as it processes them. This /// method performs no additional processing, but returns a handle which /// allows freeing the allocated memory. Otherwise said memory is leaked @@ -609,7 +605,7 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { /// /// This method does not need to be called when access to the memory /// handle is not required. - fn finish(mut self) -> Self::Product { + pub fn finish(mut self) -> SimpleJITProduct { for func in std::mem::take(&mut self.functions_to_finalize) { let decl = self.declarations.get_function_decl(func); debug_assert!(decl.linkage.is_definable()); diff --git a/cranelift/simplejit/src/lib.rs b/cranelift/simplejit/src/lib.rs index 705b8decfc..831dc751b2 100644 --- a/cranelift/simplejit/src/lib.rs +++ b/cranelift/simplejit/src/lib.rs @@ -26,7 +26,7 @@ mod backend; mod memory; -pub use crate::backend::{SimpleJITBackend, SimpleJITBuilder, SimpleJITProduct}; +pub use crate::backend::{SimpleJITModule, SimpleJITBuilder, SimpleJITProduct}; /// Version number of this crate. pub const VERSION: &str = env!("CARGO_PKG_VERSION"); From 9ccf8370926ffd6094076b4b83c437ff41476ea6 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 30 Sep 2020 19:58:12 +0200 Subject: [PATCH 15/17] Fix SimpleJIT tests --- .../simplejit/examples/simplejit-minimal.rs | 10 +++---- cranelift/simplejit/tests/basic.rs | 30 +++++-------------- 2 files changed, 13 insertions(+), 27 deletions(-) diff --git a/cranelift/simplejit/examples/simplejit-minimal.rs b/cranelift/simplejit/examples/simplejit-minimal.rs index 1534e144de..7c5df36eee 100644 --- a/cranelift/simplejit/examples/simplejit-minimal.rs +++ b/cranelift/simplejit/examples/simplejit-minimal.rs @@ -1,12 +1,12 @@ use cranelift::prelude::*; use cranelift_codegen::binemit::NullTrapSink; use cranelift_module::{default_libcall_names, Linkage, Module}; -use cranelift_simplejit::{SimpleJITBackend, SimpleJITBuilder}; +use cranelift_simplejit::{SimpleJITBuilder, SimpleJITModule}; use std::mem; fn main() { - let mut module: Module = - Module::new(SimpleJITBuilder::new(default_libcall_names())); + let mut module: SimpleJITModule = + SimpleJITModule::new(SimpleJITBuilder::new(default_libcall_names())); let mut ctx = module.make_context(); let mut func_ctx = FunctionBuilderContext::new(); @@ -70,10 +70,10 @@ fn main() { module.clear_context(&mut ctx); // Perform linking. - module.finalize_definitions(); + let product = module.finish(); // Get a raw pointer to the generated code. - let code_b = module.get_finalized_function(func_b); + let code_b = product.lookup_func(func_b); // Cast it to a rust function pointer type. let ptr_b = unsafe { mem::transmute::<_, fn() -> u32>(code_b) }; diff --git a/cranelift/simplejit/tests/basic.rs b/cranelift/simplejit/tests/basic.rs index 31d19fa97a..e7b52417c1 100644 --- a/cranelift/simplejit/tests/basic.rs +++ b/cranelift/simplejit/tests/basic.rs @@ -9,8 +9,8 @@ use cranelift_simplejit::*; #[test] fn error_on_incompatible_sig_in_declare_function() { - let mut module: Module = - Module::new(SimpleJITBuilder::new(default_libcall_names())); + let mut module: SimpleJITModule = + SimpleJITModule::new(SimpleJITBuilder::new(default_libcall_names())); let mut sig = Signature { params: vec![AbiParam::new(types::I64)], returns: vec![], @@ -26,7 +26,7 @@ fn error_on_incompatible_sig_in_declare_function() { .unwrap(); // Make sure this is an error } -fn define_simple_function(module: &mut Module) -> FuncId { +fn define_simple_function(module: &mut SimpleJITModule) -> FuncId { let sig = Signature { params: vec![], returns: vec![], @@ -55,27 +55,13 @@ fn define_simple_function(module: &mut Module) -> FuncId { func_id } -#[test] -fn double_finalize() { - let mut module: Module = - Module::new(SimpleJITBuilder::new(default_libcall_names())); - - define_simple_function(&mut module); - module.finalize_definitions(); - - // Calling `finalize_definitions` a second time without any new definitions - // should have no effect. - module.finalize_definitions(); -} - #[test] #[should_panic(expected = "Result::unwrap()` on an `Err` value: DuplicateDefinition(\"abc\")")] fn panic_on_define_after_finalize() { - let mut module: Module = - Module::new(SimpleJITBuilder::new(default_libcall_names())); + let mut module: SimpleJITModule = + SimpleJITModule::new(SimpleJITBuilder::new(default_libcall_names())); define_simple_function(&mut module); - module.finalize_definitions(); define_simple_function(&mut module); } @@ -154,8 +140,8 @@ fn switch_error() { #[test] fn libcall_function() { - let mut module: Module = - Module::new(SimpleJITBuilder::new(default_libcall_names())); + let mut module: SimpleJITModule = + SimpleJITModule::new(SimpleJITBuilder::new(default_libcall_names())); let sig = Signature { params: vec![], @@ -200,5 +186,5 @@ fn libcall_function() { .define_function(func_id, &mut ctx, &mut trap_sink) .unwrap(); - module.finalize_definitions(); + module.finish(); } From b061694491d1d4f39eb25944232dfeeeaa9301f7 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 1 Oct 2020 09:53:23 +0200 Subject: [PATCH 16/17] Rustfmt and update docs --- cranelift/module/README.md | 11 +++++------ cranelift/module/src/data_context.rs | 3 ++- cranelift/module/src/module.rs | 2 +- cranelift/object/src/backend.rs | 6 +++--- cranelift/object/src/lib.rs | 2 +- cranelift/simplejit/src/backend.rs | 4 ++-- cranelift/simplejit/src/lib.rs | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/cranelift/module/README.md b/cranelift/module/README.md index a357aa637d..1727975f96 100644 --- a/cranelift/module/README.md +++ b/cranelift/module/README.md @@ -6,14 +6,13 @@ This crate is structured as an optional layer on top of cranelift-codegen. It provides additional functionality, such as linking, however users that require greater flexibility don't need to use it. -A `Module` is a collection of functions and data objects that are linked -together. `Backend` is a trait that defines an interface for backends -that compile modules into various forms. Most users will use one of the -following `Backend` implementations: +A module is a collection of functions and data objects that are linked +together. The `Module` trait that defines a common interface for various kinds +of modules. Most users will use one of the following `Module` implementations: - - `SimpleJITBackend`, provided by [cranelift-simplejit], which JITs + - `SimpleJITModule`, provided by [cranelift-simplejit], which JITs code to memory for direct execution. - - `ObjectBackend`, provided by [cranelift-object], which emits native + - `ObjectModule`, provided by [cranelift-object], which emits native object files. [cranelift-simplejit]: https://crates.io/crates/cranelift-simplejit diff --git a/cranelift/module/src/data_context.rs b/cranelift/module/src/data_context.rs index b7b3e6c18f..ca490a09e9 100644 --- a/cranelift/module/src/data_context.rs +++ b/cranelift/module/src/data_context.rs @@ -50,7 +50,8 @@ pub struct DataDescription { pub data_relocs: Vec<(CodeOffset, ir::GlobalValue, Addend)>, /// Object file section pub custom_segment_section: Option<(String, String)>, - /// Alignment + /// Alignment in bytes. `None` means that the default alignment of the respective module should + /// be used. pub align: Option, } diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 84fc19c96f..26704ae5e5 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -142,7 +142,7 @@ impl FunctionDeclaration { } } -/// Error messages for all `Module` and `Backend` methods +/// Error messages for all `Module` methods #[derive(Error, Debug)] pub enum ModuleError { /// Indicates an identifier was used before it was declared diff --git a/cranelift/object/src/backend.rs b/cranelift/object/src/backend.rs index a0c964cd09..56008a1cf9 100644 --- a/cranelift/object/src/backend.rs +++ b/cranelift/object/src/backend.rs @@ -1,4 +1,4 @@ -//! Defines `ObjectBackend`. +//! Defines `ObjectModule`. use anyhow::anyhow; use cranelift_codegen::binemit::{ @@ -23,7 +23,7 @@ use std::convert::TryInto; use std::mem; use target_lexicon::PointerWidth; -/// A builder for `ObjectBackend`. +/// A builder for `ObjectModule`. pub struct ObjectBuilder { isa: Box, binary_format: object::BinaryFormat, @@ -37,7 +37,7 @@ pub struct ObjectBuilder { impl ObjectBuilder { /// Create a new `ObjectBuilder` using the given Cranelift target, that - /// can be passed to [`Module::new`](cranelift_module::Module::new). + /// can be passed to [`ObjectModule::new`]. /// /// The `libcall_names` function provides a way to translate `cranelift_codegen`'s `ir::LibCall` /// enum to symbols. LibCalls are inserted in the IR as part of the legalization for certain diff --git a/cranelift/object/src/lib.rs b/cranelift/object/src/lib.rs index 12ca1d7aca..1b9b677e89 100644 --- a/cranelift/object/src/lib.rs +++ b/cranelift/object/src/lib.rs @@ -27,7 +27,7 @@ mod backend; -pub use crate::backend::{ObjectModule, ObjectBuilder, ObjectProduct}; +pub use crate::backend::{ObjectBuilder, ObjectModule, ObjectProduct}; /// Version number of this crate. pub const VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/cranelift/simplejit/src/backend.rs b/cranelift/simplejit/src/backend.rs index 5d42be75fe..369561f06b 100644 --- a/cranelift/simplejit/src/backend.rs +++ b/cranelift/simplejit/src/backend.rs @@ -164,7 +164,7 @@ pub struct SimpleJITCompiledData { relocs: Vec, } -/// A handle to allow freeing memory allocated by the `Backend`. +/// A handle to allow freeing memory allocated by the `Module`. struct SimpleJITMemoryHandle { code: Memory, readonly: Memory, @@ -360,7 +360,7 @@ impl SimpleJITModule { } } - /// Create a new `SimpleJITBackend`. + /// Create a new `SimpleJITModule`. pub fn new(builder: SimpleJITBuilder) -> Self { let memory = SimpleJITMemoryHandle { code: Memory::new(), diff --git a/cranelift/simplejit/src/lib.rs b/cranelift/simplejit/src/lib.rs index 831dc751b2..682dcaecb6 100644 --- a/cranelift/simplejit/src/lib.rs +++ b/cranelift/simplejit/src/lib.rs @@ -26,7 +26,7 @@ mod backend; mod memory; -pub use crate::backend::{SimpleJITModule, SimpleJITBuilder, SimpleJITProduct}; +pub use crate::backend::{SimpleJITBuilder, SimpleJITModule, SimpleJITProduct}; /// Version number of this crate. pub const VERSION: &str = env!("CARGO_PKG_VERSION"); From 13c6bdd9bab2e8aaacad420a2f90b87624b8e6f3 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 7 Oct 2020 20:06:58 -0700 Subject: [PATCH 17/17] cranelift-module: add iterator methods to ModuleDeclarations The predecessor interface made it possible to iterate across all function and data declarations. This is still useful and easy enough to provide. --- cranelift/module/src/module.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cranelift/module/src/module.rs b/cranelift/module/src/module.rs index 26704ae5e5..58ddb2aa76 100644 --- a/cranelift/module/src/module.rs +++ b/cranelift/module/src/module.rs @@ -210,6 +210,11 @@ impl ModuleDeclarations { self.names.get(name).copied() } + /// Get an iterator of all function declarations + pub fn get_functions(&self) -> impl Iterator { + self.functions.iter() + } + /// Get the `FuncId` for the function named by `name`. pub fn get_function_id(&self, name: &ir::ExternalName) -> FuncId { if let ir::ExternalName::User { namespace, index } = *name { @@ -235,6 +240,11 @@ impl ModuleDeclarations { &self.functions[func_id] } + /// Get an iterator of all data declarations + pub fn get_data_objects(&self) -> impl Iterator { + self.data_objects.iter() + } + /// Get the `DataDeclaration` for the data object named by `name`. pub fn get_data_decl(&self, data_id: DataId) -> &DataDeclaration { &self.data_objects[data_id]