Skip to content

Commit

Permalink
Auto merge of #46623 - eddyb:issue-46449, r=nagisa
Browse files Browse the repository at this point in the history
rustc_trans: approximate ABI alignment for padding/union fillers.

Before #45225 and after this PR, unions and enums are filled with integers of size and alignment matching their alignment (e.g. `Option<u32>` becomes `[u32; 2]`) instead of mere bytes.
Also, the alignment padding between struct fields gets this treatment after this PR.

Partially helps with some reduced testcases in #46449, although it doesn't solve the bug itself.
  • Loading branch information
bors committed Dec 15, 2017
2 parents 50f6c3e + 8a26e04 commit 77efd68
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 13 deletions.
17 changes: 15 additions & 2 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,14 +489,27 @@ impl<'a, 'tcx> Integer {

let wanted = align.abi();
for &candidate in &[I8, I16, I32, I64, I128] {
let ty = Int(candidate, false);
if wanted == ty.align(dl).abi() && wanted == ty.size(dl).bytes() {
if wanted == candidate.align(dl).abi() && wanted == candidate.size().bytes() {
return Some(candidate);
}
}
None
}

/// Find the largest integer with the given alignment or less.
pub fn approximate_abi_align<C: HasDataLayout>(cx: C, align: Align) -> Integer {
let dl = cx.data_layout();

let wanted = align.abi();
// FIXME(eddyb) maybe include I128 in the future, when it works everywhere.
for &candidate in &[I64, I32, I16] {
if wanted >= candidate.align(dl).abi() && wanted >= candidate.size().bytes() {
return candidate;
}
}
I8
}

/// Get the Integer type from an attr::IntType.
pub fn from_attr<C: HasDataLayout>(cx: C, ity: attr::IntType) -> Integer {
let dl = cx.data_layout();
Expand Down
21 changes: 14 additions & 7 deletions src/librustc_trans/type_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use llvm::{Float, Double, X86_FP80, PPC_FP128, FP128};
use context::CrateContext;

use syntax::ast;
use rustc::ty::layout::{self, Align};
use rustc::ty::layout::{self, Align, Size};

use std::ffi::CString;
use std::fmt;
Expand Down Expand Up @@ -279,12 +279,19 @@ impl Type {
/// Return a LLVM type that has at most the required alignment,
/// as a conservative approximation for unknown pointee types.
pub fn pointee_for_abi_align(ccx: &CrateContext, align: Align) -> Type {
if let Some(ity) = layout::Integer::for_abi_align(ccx, align) {
Type::from_integer(ccx, ity)
} else {
// FIXME(eddyb) We could find a better approximation here.
Type::i8(ccx)
}
// FIXME(eddyb) We could find a better approximation if ity.align < align.
let ity = layout::Integer::approximate_abi_align(ccx, align);
Type::from_integer(ccx, ity)
}

/// Return a LLVM type that has at most the required alignment,
/// and exactly the required size, as a best-effort padding array.
pub fn padding_filler(ccx: &CrateContext, size: Size, align: Align) -> Type {
let unit = layout::Integer::approximate_abi_align(ccx, align);
let size = size.bytes();
let unit_size = unit.size().bytes();
assert_eq!(size % unit_size, 0);
Type::array(&Type::from_integer(ccx, unit), size / unit_size)
}

pub fn x86_mmx(ccx: &CrateContext) -> Type {
Expand Down
13 changes: 9 additions & 4 deletions src/librustc_trans/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ fn uncached_llvm_type<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,

match layout.fields {
layout::FieldPlacement::Union(_) => {
let size = layout.size.bytes();
let fill = Type::array(&Type::i8(ccx), size);
let fill = Type::padding_filler(ccx, layout.size, layout.align);
match name {
None => {
Type::struct_(ccx, &[fill], layout.is_packed())
Expand Down Expand Up @@ -115,6 +114,7 @@ fn struct_llfields<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
let field_count = layout.fields.count();

let mut offset = Size::from_bytes(0);
let mut prev_align = layout.align;
let mut result: Vec<Type> = Vec::with_capacity(1 + field_count * 2);
for i in layout.fields.index_by_increasing_offset() {
let field = layout.field(ccx, i);
Expand All @@ -123,7 +123,9 @@ fn struct_llfields<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
i, field, offset, target_offset);
assert!(target_offset >= offset);
let padding = target_offset - offset;
result.push(Type::array(&Type::i8(ccx), padding.bytes()));
let padding_align = layout.align.min(prev_align).min(field.align);
assert_eq!(offset.abi_align(padding_align) + padding, target_offset);
result.push(Type::padding_filler(ccx, padding, padding_align));
debug!(" padding before: {:?}", padding);

result.push(field.llvm_type(ccx));
Expand All @@ -137,16 +139,19 @@ fn struct_llfields<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
}

offset = target_offset + field.size;
prev_align = field.align;
}
if !layout.is_unsized() && field_count > 0 {
if offset > layout.size {
bug!("layout: {:#?} stride: {:?} offset: {:?}",
layout, layout.size, offset);
}
let padding = layout.size - offset;
let padding_align = layout.align.min(prev_align);
assert_eq!(offset.abi_align(padding_align) + padding, layout.size);
debug!("struct_llfields: pad_bytes: {:?} offset: {:?} stride: {:?}",
padding, offset, layout.size);
result.push(Type::array(&Type::i8(ccx), padding.bytes()));
result.push(Type::padding_filler(ccx, padding, padding_align));
assert!(result.len() == 1 + field_count * 2);
} else {
debug!("struct_llfields: offset: {:?} stride: {:?}",
Expand Down
20 changes: 20 additions & 0 deletions src/test/codegen/align-struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,37 @@
// except according to those terms.

// compile-flags: -C no-prepopulate-passes
// ignore-tidy-linelength

#![crate_type = "lib"]

#![feature(attr_literals)]
#![feature(repr_align)]

#[repr(align(64))]
pub struct Align64(i32);
// CHECK: %Align64 = type { [0 x i32], i32, [15 x i32] }

pub struct Nested64 {
a: Align64,
b: i32,
c: i32,
d: i8,
}
// CHECK: %Nested64 = type { [0 x i64], %Align64, [0 x i32], i32, [0 x i32], i32, [0 x i8], i8, [55 x i8] }

pub enum Enum4 {
A(i32),
B(i32),
}
// CHECK: %Enum4 = type { [2 x i32] }

pub enum Enum64 {
A(Align64),
B(i32),
}
// CHECK: %Enum64 = type { [16 x i64] }
// CHECK: %"Enum64::A" = type { [8 x i64], %Align64, [0 x i64] }

// CHECK-LABEL: @align64
#[no_mangle]
Expand All @@ -46,6 +58,14 @@ pub fn nested64(a: Align64, b: i32, c: i32, d: i8) -> Nested64 {
n64
}

// CHECK-LABEL: @enum4
#[no_mangle]
pub fn enum4(a: i32) -> Enum4 {
// CHECK: %e4 = alloca %Enum4, align 4
let e4 = Enum4::A(a);
e4
}

// CHECK-LABEL: @enum64
#[no_mangle]
pub fn enum64(a: Align64) -> Enum64 {
Expand Down

0 comments on commit 77efd68

Please sign in to comment.