Move alignment config from declare_data to define_data

cranelift/module/src/backend.rs

@@ -62,7 +62,6 @@ where
         linkage: Linkage,
         writable: bool,
         tls: bool,
-        align: Option<u8>,
     ) -> ModuleResult<DataId>;
 
     /// Define a function, producing the function body from the given `Context`.

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<u64>,
 }
 
 /// 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

cranelift/module/src/module.rs

@@ -182,14 +182,12 @@ pub struct DataDeclaration {
     pub linkage: Linkage,
     pub writable: bool,
     pub tls: bool,
-    pub align: Option<u8>,
 }
 
 impl DataDeclaration {
-    fn merge(&mut self, linkage: Linkage, writable: bool, tls: bool, align: Option<u8>) {
+    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<u8>, // 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<u8>, // An alignment bigger than 128 is unlikely
     ) -> ModuleResult<DataId> {
         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.

cranelift/object/src/backend.rs

@@ -194,11 +194,10 @@ impl Backend for ObjectBackend {
         linkage: Linkage,
         writable: bool,
         tls: bool,
-        align: Option<u8>,
     ) -> ModuleResult<DataId> {
         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");

cranelift/simplejit/src/backend.rs

@@ -25,9 +25,9 @@ use target_lexicon::PointerWidth;
 #[cfg(windows)]
 use winapi;
 
-const EXECUTABLE_DATA_ALIGNMENT: u8 = 0x10;
+const EXECUTABLE_DATA_ALIGNMENT: u64 = 0x10;
-const WRITABLE_DATA_ALIGNMENT: u8 = 0x8;
+const WRITABLE_DATA_ALIGNMENT: u64 = 0x8;
-const READONLY_DATA_ALIGNMENT: u8 = 0x1;
+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<u8>,
     ) -> ModuleResult<DataId> {
         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.")
         };
 

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> {
+    pub fn allocate(&mut self, size: usize, align: u64) -> Result<*mut u8, String> {
-        if self.position % align as usize != 0 {
+        let align = usize::try_from(align).expect("alignment too big");
-            self.position += align as usize - self.position % align as usize;
+        if self.position % align != 0 {
-            debug_assert!(self.position % align as usize == 0);
+            self.position += align - self.position % align;
+            debug_assert!(self.position % align == 0);
         }
 
         if size <= self.current.len - self.position {