Move logic out of more Module methods

cranelift/module/src/backend.rs

@@ -63,7 +63,6 @@ where
     fn define_function<TS>(
         &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<u8>,
         data_ctx: &DataContext,
         declarations: &ModuleDeclarations,
     ) -> ModuleResult<()>;

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<String, FuncOrDataId>,
     functions: PrimaryMap<FuncId, FunctionDeclaration>,
@@ -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(
+        self.backend
-            id,
+            .declare_data(id, name, decl.linkage, decl.writable, decl.tls, decl.align);
-            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(
+        self.backend
-            func,
+            .define_function(func, ctx, &self.declarations, total_size, trap_sink)?;
-            &decl.name,
-            ctx,
-            &self.declarations,
-            total_size,
-            trap_sink,
-        )?;
 
         Ok(ModuleCompiledFunction { size: total_size })
     }
@@ -530,9 +515,6 @@ where
     ) -> ModuleResult<ModuleCompiledFunction> {
         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

cranelift/object/src/backend.rs

@@ -208,18 +208,22 @@ impl Backend for ObjectBackend {
     fn define_function<TS>(
         &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<u8>,
         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");

cranelift/simplejit/src/backend.rs

@@ -413,17 +413,21 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend {
     fn define_function<TS>(
         &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<u8>,
         data: &DataContext,
-        _declarations: &ModuleDeclarations,
+        declarations: &ModuleDeclarations,
     ) -> ModuleResult<()> {
-        if !self.data_objects[id].is_none() {
+        let decl = declarations.get_data_decl(id);
-            return Err(ModuleError::DuplicateDefinition(name.to_owned()));
+        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.")
         };