Merge pull request #718 from FractalFir/needless_align

Skip needless calls to get_align in some cases.

Author: antoyo

src/builder.rs

@@ -938,22 +938,36 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
     fn load(&mut self, pointee_ty: Type<'gcc>, ptr: RValue<'gcc>, align: Align) -> RValue<'gcc> {
         let block = self.llbb();
         let function = block.get_function();
+        // NOTE(FractalFir): In some cases, we *should* skip the call to get_aligned.
+        // For example, calling `get_aligned` on a i8 is pointless(since it can only be 1 aligned)
+        // Calling get_aligned on a `u128`/`i128` causes the attribute to become "stacked"
+        //
+        // From GCCs perspective:
+        // __int128_t  __attribute__((aligned(16)))  __attribute__((aligned(16)))
+        // and:
+        // __int128_t  __attribute__((aligned(16)))
+        // are 2 distinct, incompatible types.
+        //
+        // So, we skip the call to `get_aligned` in such a case. *Ideally*, we could do this for all the types,
+        // but the GCC APIs to facilitate this just aren't quite there yet.
+
+        // This checks that we only skip `get_aligned` on 128 bit ints if they have the correct alignment.
+        // Otherwise, this may be an under-aligned load, so we will still call get_aligned.
+        let mut can_skip_align = (pointee_ty == self.cx.u128_type
+            || pointee_ty == self.cx.i128_type)
+            && align == self.int128_align;
+        // We can skip the call to `get_aligned` for byte-sized types with alignment of 1.
+        can_skip_align = can_skip_align
+            || (pointee_ty == self.cx.u8_type || pointee_ty == self.cx.i8_type)
+                && align.bytes() == 1;
+        // Skip the call to `get_aligned` when possible.
+        let aligned_type =
+            if can_skip_align { pointee_ty } else { pointee_ty.get_aligned(align.bytes()) };
+
+        let ptr = self.context.new_cast(self.location, ptr, aligned_type.make_pointer());
         // NOTE: instead of returning the dereference here, we have to assign it to a variable in
         // the current basic block. Otherwise, it could be used in another basic block, causing a
         // dereference after a drop, for instance.
-        // FIXME(antoyo): this check that we don't call get_aligned() a second time on a type.
-        // Ideally, we shouldn't need to do this check.
-        // FractalFir: the `align == self.int128_align` check ensures we *do* call `get_aligned` if
-        // the alignment of a `u128`/`i128` is not the one mandated by the ABI. This ensures we handle
-        // under-aligned loads correctly.
-        let aligned_type = if (pointee_ty == self.cx.u128_type || pointee_ty == self.cx.i128_type)
-            && align == self.int128_align
-        {
-            pointee_ty
-        } else {
-            pointee_ty.get_aligned(align.bytes())
-        };
-        let ptr = self.context.new_cast(self.location, ptr, aligned_type.make_pointer());
         let deref = ptr.dereference(self.location).to_rvalue();
         let loaded_value = function.new_local(
             self.location,

src/context.rs

@@ -157,6 +157,13 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
                 .layout_of(ty::TypingEnv::fully_monomorphized().as_query_input(rust_type))
                 .unwrap();
             let align = layout.align.abi.bytes();
+            // For types with size 1, the alignment can be 1 and only 1
+            // So, we can skip the call to ``get_aligned`.
+            // In the future, we can add a GCC API to query the type align,
+            // and call `get_aligned` if and only if that differs from Rust's expectations.
+            if layout.size.bytes() == 1 {
+                return context.new_c_type(ctype);
+            }
             #[cfg(feature = "master")]
             {
                 context.new_c_type(ctype).get_aligned(align)