Allow offloading compilation in cranelift-object (#2371)

This commit is a slight refactoring of the `Module` trait and backend in
`cranelift-object`. The goal is to enable parallelization of compilation
when using `cranelift-object`. Currently this is difficult because
`ObjectModule::define_function` requires `&mut self`. This instead
soups up the `define_function_bytes` interface to handle relocations so
compilation can happen externally before defining it in a `Module`. This
also means that `define_function` is now a convenience wrapper around
`define_function_bytes`.

cranelift/module/src/lib.rs

@@ -44,7 +44,7 @@ mod traps;
 pub use crate::data_context::{DataContext, DataDescription, Init};
 pub use crate::module::{
     DataId, FuncId, FuncOrDataId, Linkage, Module, ModuleCompiledFunction, ModuleDeclarations,
-    ModuleError, ModuleResult,
+    ModuleError, ModuleResult, RelocRecord,
 };
 pub use crate::traps::TrapSite;
 

cranelift/module/src/module.rs

@@ -335,6 +335,19 @@ pub struct ModuleCompiledFunction {
     pub size: binemit::CodeOffset,
 }
 
+/// A record of a relocation to perform.
+#[derive(Clone)]
+pub struct RelocRecord {
+    /// Where in the generated code this relocation is to be applied.
+    pub offset: binemit::CodeOffset,
+    /// The kind of relocation this represents.
+    pub reloc: binemit::Reloc,
+    /// What symbol we're relocating against.
+    pub name: ir::ExternalName,
+    /// The offset to add to the relocation.
+    pub addend: binemit::Addend,
+}
+
 /// A `Module` is a utility for collecting functions and data objects, and linking them together.
 pub trait Module {
     /// Return the `TargetIsa` to compile for.
@@ -470,6 +483,7 @@ pub trait Module {
         &mut self,
         func: FuncId,
         bytes: &[u8],
+        relocs: &[RelocRecord],
     ) -> ModuleResult<ModuleCompiledFunction>;
 
     /// Define a data object, producing the data contents from the given `DataContext`.

cranelift/object/src/backend.rs

@@ -9,7 +9,7 @@ use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::{self, ir};
 use cranelift_module::{
     DataContext, DataDescription, DataId, FuncId, Init, Linkage, Module, ModuleCompiledFunction,
-    ModuleDeclarations, ModuleError, ModuleResult,
+    ModuleDeclarations, ModuleError, ModuleResult, RelocRecord,
 };
 use log::info;
 use object::write::{
@@ -30,7 +30,7 @@ pub struct ObjectBuilder {
     architecture: object::Architecture,
     endian: object::Endianness,
     name: Vec<u8>,
-    libcall_names: Box<dyn Fn(ir::LibCall) -> String>,
+    libcall_names: Box<dyn Fn(ir::LibCall) -> String + Send + Sync>,
     function_alignment: u64,
     per_function_section: bool,
 }
@@ -46,7 +46,7 @@ impl ObjectBuilder {
     pub fn new<V: Into<Vec<u8>>>(
         isa: Box<dyn TargetIsa>,
         name: V,
-        libcall_names: Box<dyn Fn(ir::LibCall) -> String>,
+        libcall_names: Box<dyn Fn(ir::LibCall) -> String + Send + Sync>,
     ) -> ModuleResult<Self> {
         let binary_format = match isa.triple().binary_format {
             target_lexicon::BinaryFormat::Elf => object::BinaryFormat::Elf,
@@ -119,7 +119,7 @@ pub struct ObjectModule {
     data_objects: SecondaryMap<DataId, Option<(SymbolId, bool)>>,
     relocs: Vec<SymbolRelocs>,
     libcalls: HashMap<ir::LibCall, SymbolId>,
-    libcall_names: Box<dyn Fn(ir::LibCall) -> String>,
+    libcall_names: Box<dyn Fn(ir::LibCall) -> String + Send + Sync>,
     function_alignment: u64,
     per_function_section: bool,
 }
@@ -244,20 +244,8 @@ impl Module for ObjectModule {
             total_size: code_size,
             ..
         } = ctx.compile(self.isa())?;
-
-        let decl = self.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(decl.name.clone()));
-        }
-        *defined = true;
-
         let mut code: Vec<u8> = vec![0; code_size as usize];
-        let mut reloc_sink = ObjectRelocSink::new(self.object.format());
+        let mut reloc_sink = ObjectRelocSink::default();
         let mut stack_map_sink = NullStackMapSink {};
 
         unsafe {
@@ -270,40 +258,14 @@ impl Module for ObjectModule {
             )
         };
 
-        let (section, offset) = if self.per_function_section {
+        self.define_function_bytes(func_id, &code, &reloc_sink.relocs)
-            let symbol_name = self.object.symbol(symbol).name.clone();
-            let (section, offset) = self.object.add_subsection(
-                StandardSection::Text,
-                &symbol_name,
-                &code,
-                self.function_alignment,
-            );
-            self.object.symbol_mut(symbol).section = SymbolSection::Section(section);
-            self.object.symbol_mut(symbol).value = offset;
-            (section, offset)
-        } else {
-            let section = self.object.section_id(StandardSection::Text);
-            let offset =
-                self.object
-                    .add_symbol_data(symbol, section, &code, self.function_alignment);
-            (section, offset)
-        };
-
-        if !reloc_sink.relocs.is_empty() {
-            self.relocs.push(SymbolRelocs {
-                section,
-                offset,
-                relocs: reloc_sink.relocs,
-            });
-        }
-
-        Ok(ModuleCompiledFunction { size: code_size })
     }
 
     fn define_function_bytes(
         &mut self,
         func_id: FuncId,
         bytes: &[u8],
+        relocs: &[RelocRecord],
     ) -> ModuleResult<ModuleCompiledFunction> {
         info!("defining function {} with bytes", func_id);
 
@@ -323,7 +285,7 @@ impl Module for ObjectModule {
         }
         *defined = true;
 
-        if self.per_function_section {
+        let (section, offset) = if self.per_function_section {
             let symbol_name = self.object.symbol(symbol).name.clone();
             let (section, offset) = self.object.add_subsection(
                 StandardSection::Text,
@@ -333,11 +295,22 @@ impl Module for ObjectModule {
             );
             self.object.symbol_mut(symbol).section = SymbolSection::Section(section);
             self.object.symbol_mut(symbol).value = offset;
+            (section, offset)
         } else {
             let section = self.object.section_id(StandardSection::Text);
-            let _offset =
+            let offset =
                 self.object
                     .add_symbol_data(symbol, section, bytes, self.function_alignment);
+            (section, offset)
+        };
+
+        if !relocs.is_empty() {
+            let relocs = self.process_relocs(relocs);
+            self.relocs.push(SymbolRelocs {
+                section,
+                offset,
+                relocs,
+            });
         }
 
         Ok(ModuleCompiledFunction { size: total_size })
@@ -372,7 +345,7 @@ impl Module for ObjectModule {
         };
         let mut relocs = Vec::new();
         for &(offset, id) in function_relocs {
-            relocs.push(RelocRecord {
+            relocs.push(ObjectRelocRecord {
                 offset,
                 name: function_decls[id].clone(),
                 kind: RelocationKind::Absolute,
@@ -382,7 +355,7 @@ impl Module for ObjectModule {
             });
         }
         for &(offset, id, addend) in data_relocs {
-            relocs.push(RelocRecord {
+            relocs.push(ObjectRelocRecord {
                 offset,
                 name: data_decls[id].clone(),
                 kind: RelocationKind::Absolute,
@@ -457,7 +430,7 @@ impl ObjectModule {
     pub fn finish(mut self) -> ObjectProduct {
         let symbol_relocs = mem::take(&mut self.relocs);
         for symbol in symbol_relocs {
-            for &RelocRecord {
+            for &ObjectRelocRecord {
                 offset,
                 ref name,
                 kind,
@@ -536,6 +509,71 @@ impl ObjectModule {
             _ => panic!("invalid ExternalName {}", name),
         }
     }
+
+    fn process_relocs(&self, relocs: &[RelocRecord]) -> Vec<ObjectRelocRecord> {
+        relocs
+            .iter()
+            .map(|record| {
+                let mut addend = record.addend;
+                let (kind, encoding, size) = match record.reloc {
+                    Reloc::Abs4 => (RelocationKind::Absolute, RelocationEncoding::Generic, 32),
+                    Reloc::Abs8 => (RelocationKind::Absolute, RelocationEncoding::Generic, 64),
+                    Reloc::X86PCRel4 => (RelocationKind::Relative, RelocationEncoding::Generic, 32),
+                    Reloc::X86CallPCRel4 => {
+                        (RelocationKind::Relative, RelocationEncoding::X86Branch, 32)
+                    }
+                    // TODO: Get Cranelift to tell us when we can use
+                    // R_X86_64_GOTPCRELX/R_X86_64_REX_GOTPCRELX.
+                    Reloc::X86CallPLTRel4 => (
+                        RelocationKind::PltRelative,
+                        RelocationEncoding::X86Branch,
+                        32,
+                    ),
+                    Reloc::X86GOTPCRel4 => {
+                        (RelocationKind::GotRelative, RelocationEncoding::Generic, 32)
+                    }
+                    Reloc::ElfX86_64TlsGd => {
+                        assert_eq!(
+                            self.object.format(),
+                            object::BinaryFormat::Elf,
+                            "ElfX86_64TlsGd is not supported for this file format"
+                        );
+                        (
+                            RelocationKind::Elf(object::elf::R_X86_64_TLSGD),
+                            RelocationEncoding::Generic,
+                            32,
+                        )
+                    }
+                    Reloc::MachOX86_64Tlv => {
+                        assert_eq!(
+                            self.object.format(),
+                            object::BinaryFormat::MachO,
+                            "MachOX86_64Tlv is not supported for this file format"
+                        );
+                        addend += 4; // X86_64_RELOC_TLV has an implicit addend of -4
+                        (
+                            RelocationKind::MachO {
+                                value: object::macho::X86_64_RELOC_TLV,
+                                relative: true,
+                            },
+                            RelocationEncoding::Generic,
+                            32,
+                        )
+                    }
+                    // FIXME
+                    _ => unimplemented!(),
+                };
+                ObjectRelocRecord {
+                    offset: record.offset,
+                    name: record.name.clone(),
+                    kind,
+                    encoding,
+                    size,
+                    addend,
+                }
+            })
+            .collect()
+    }
 }
 
 fn translate_linkage(linkage: Linkage) -> (SymbolScope, bool) {
@@ -587,11 +625,11 @@ impl ObjectProduct {
 struct SymbolRelocs {
     section: SectionId,
     offset: u64,
-    relocs: Vec<RelocRecord>,
+    relocs: Vec<ObjectRelocRecord>,
 }
 
 #[derive(Clone)]
-struct RelocRecord {
+struct ObjectRelocRecord {
     offset: CodeOffset,
     name: ir::ExternalName,
     kind: RelocationKind,
@@ -600,20 +638,11 @@ struct RelocRecord {
     addend: Addend,
 }
 
+#[derive(Default)]
 struct ObjectRelocSink {
-    format: object::BinaryFormat,
     relocs: Vec<RelocRecord>,
 }
 
-impl ObjectRelocSink {
-    fn new(format: object::BinaryFormat) -> Self {
-        Self {
-            format,
-            relocs: vec![],
-        }
-    }
-}
-
 impl RelocSink for ObjectRelocSink {
     fn reloc_block(&mut self, _offset: CodeOffset, _reloc: Reloc, _block_offset: CodeOffset) {
         unimplemented!();
@@ -625,61 +654,14 @@ impl RelocSink for ObjectRelocSink {
         _srcloc: ir::SourceLoc,
         reloc: Reloc,
         name: &ir::ExternalName,
-        mut addend: Addend,
+        addend: Addend,
     ) {
-        let (kind, encoding, size) = match reloc {
-            Reloc::Abs4 => (RelocationKind::Absolute, RelocationEncoding::Generic, 32),
-            Reloc::Abs8 => (RelocationKind::Absolute, RelocationEncoding::Generic, 64),
-            Reloc::X86PCRel4 => (RelocationKind::Relative, RelocationEncoding::Generic, 32),
-            Reloc::X86CallPCRel4 => (RelocationKind::Relative, RelocationEncoding::X86Branch, 32),
-            // TODO: Get Cranelift to tell us when we can use
-            // R_X86_64_GOTPCRELX/R_X86_64_REX_GOTPCRELX.
-            Reloc::X86CallPLTRel4 => (
-                RelocationKind::PltRelative,
-                RelocationEncoding::X86Branch,
-                32,
-            ),
-            Reloc::X86GOTPCRel4 => (RelocationKind::GotRelative, RelocationEncoding::Generic, 32),
-
-            Reloc::ElfX86_64TlsGd => {
-                assert_eq!(
-                    self.format,
-                    object::BinaryFormat::Elf,
-                    "ElfX86_64TlsGd is not supported for this file format"
-                );
-                (
-                    RelocationKind::Elf(object::elf::R_X86_64_TLSGD),
-                    RelocationEncoding::Generic,
-                    32,
-                )
-            }
-            Reloc::MachOX86_64Tlv => {
-                assert_eq!(
-                    self.format,
-                    object::BinaryFormat::MachO,
-                    "MachOX86_64Tlv is not supported for this file format"
-                );
-                addend += 4; // X86_64_RELOC_TLV has an implicit addend of -4
-                (
-                    RelocationKind::MachO {
-                        value: object::macho::X86_64_RELOC_TLV,
-                        relative: true,
-                    },
-                    RelocationEncoding::Generic,
-                    32,
-                )
-            }
-            // FIXME
-            _ => unimplemented!(),
-        };
         self.relocs.push(RelocRecord {
             offset,
-            name: name.clone(),
+            reloc,
-            kind,
-            encoding,
-            size,
             addend,
-        });
+            name: name.clone(),
+        })
     }
 
     fn reloc_jt(&mut self, _offset: CodeOffset, reloc: Reloc, _jt: ir::JumpTable) {

cranelift/simplejit/src/backend.rs

@@ -10,7 +10,7 @@ use cranelift_codegen::{self, ir, settings};
 use cranelift_entity::SecondaryMap;
 use cranelift_module::{
     DataContext, DataDescription, DataId, FuncId, FuncOrDataId, Init, Linkage, Module,
-    ModuleCompiledFunction, ModuleDeclarations, ModuleError, ModuleResult,
+    ModuleCompiledFunction, ModuleDeclarations, ModuleError, ModuleResult, RelocRecord,
 };
 use cranelift_native;
 #[cfg(not(windows))]
@@ -134,15 +134,6 @@ pub struct SimpleJITModule {
     data_objects_to_finalize: Vec<DataId>,
 }
 
-/// A record of a relocation to perform.
-#[derive(Clone)]
-struct RelocRecord {
-    offset: CodeOffset,
-    reloc: Reloc,
-    name: ir::ExternalName,
-    addend: Addend,
-}
-
 struct StackMapRecord {
     #[allow(dead_code)]
     offset: CodeOffset,
@@ -528,6 +519,7 @@ impl<'simple_jit_backend> Module for SimpleJITModule {
         &mut self,
         id: FuncId,
         bytes: &[u8],
+        relocs: &[RelocRecord],
     ) -> ModuleResult<ModuleCompiledFunction> {
         let decl = self.declarations.get_function_decl(id);
         if !decl.linkage.is_definable() {
@@ -560,7 +552,7 @@ impl<'simple_jit_backend> Module for SimpleJITModule {
         self.functions[id] = Some(CompiledBlob {
             ptr,
             size,
-            relocs: vec![],
+            relocs: relocs.to_vec(),
         });
 
         Ok(ModuleCompiledFunction { size: total_size })