Skip to content

Commit

Permalink
Use a u16 instead of an i64 for enum tags
Browse files Browse the repository at this point in the history
Now that we support stack allocated types, the fields of enum types
aren't necessarily all one word in size any more. This in turn means
that using an i64 might be wasteful due to the excess padding. To reduce
the size of enums somewhat, we now use a 16 bits integer for the tag
instead of a 64 bits integer.

This change also revealed an issue with struct returns/arguments on the
AMD64 ABI, as for types such as `{ i16, i64 }` the ABI actually seems to
demand `{ i64, i16 }`. Since this requires re-ordering the struct fields
and remapping all access sites accordingly, which is a pain, we take the
same approach as Rust which in turn matches what the ARM ABI wants: for
structs between 8 and 16 bytes, we just treat them as `{ i64, i64 }`
types. This is valid size/alignment wise because for a `{ i16, i64 }`
the alignment of both members is 8 bytes.

This fixes #785.

Changelog: performance
  • Loading branch information
yorickpeterse committed Dec 23, 2024
1 parent 40b3293 commit 87e6689
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 106 deletions.
12 changes: 8 additions & 4 deletions compiler/src/llvm/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,19 +238,23 @@ impl<'ctx> Builder<'ctx> {
}

pub(crate) fn i64_literal(&self, value: i64) -> IntValue<'ctx> {
self.u64_literal(value as u64)
self.int_literal(64, value as u64)
}

pub(crate) fn u16_literal(&self, value: u16) -> IntValue<'ctx> {
self.context.i16_type().const_int(value as u64, false)
self.int_literal(16, value as u64)
}

pub(crate) fn u32_literal(&self, value: u32) -> IntValue<'ctx> {
self.context.i32_type().const_int(value as u64, false)
self.int_literal(32, value as u64)
}

pub(crate) fn u64_literal(&self, value: u64) -> IntValue<'ctx> {
self.context.i64_type().const_int(value, false)
self.int_literal(64, value)
}

pub(crate) fn int_literal(&self, bits: u32, value: u64) -> IntValue<'ctx> {
self.context.custom_int(bits).const_int(value, false)
}

pub(crate) fn f64_literal(&self, value: f64) -> FloatValue<'ctx> {
Expand Down
79 changes: 22 additions & 57 deletions compiler/src/llvm/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ impl Context {
self.inner.struct_type(fields, false)
}

pub(crate) fn two_words(&self) -> StructType {
let word = self.i64_type().into();

self.inner.struct_type(&[word, word], false)
}

pub(crate) fn class_type<'a>(
&'a self,
method_type: StructType<'a>,
Expand Down Expand Up @@ -178,17 +184,8 @@ impl Context {
};

match id {
TypeId::Foreign(ForeignType::Int(8, _)) => {
self.i8_type().as_basic_type_enum()
}
TypeId::Foreign(ForeignType::Int(16, _)) => {
self.i16_type().as_basic_type_enum()
}
TypeId::Foreign(ForeignType::Int(32, _)) => {
self.i32_type().as_basic_type_enum()
}
TypeId::Foreign(ForeignType::Int(_, _)) => {
self.i64_type().as_basic_type_enum()
TypeId::Foreign(ForeignType::Int(size, _)) => {
self.custom_int(size).as_basic_type_enum()
}
TypeId::Foreign(ForeignType::Float(32)) => {
self.f32_type().as_basic_type_enum()
Expand Down Expand Up @@ -231,9 +228,15 @@ impl Context {

ArgumentType::Regular(bits.as_basic_type_enum())
} else if bytes <= 16 {
ArgumentType::Regular(
self.abi_struct_type(layouts, typ).as_basic_type_enum(),
)
// The AMD64 ABI doesn't handle types such as
// `{ i16, i64 }`. While it does handle `{ i64, i16 }`, this
// requires re-ordering the fields and their corresponding
// access sites.
//
// To avoid the complexity of that we take the same approach
// as Rust: if the struct is larger than 8 bytes, we turn it
// into two 64 bits values.
ArgumentType::Regular(self.two_words().as_basic_type_enum())
} else {
ArgumentType::StructValue(typ)
}
Expand All @@ -242,10 +245,7 @@ impl Context {
if bytes <= 8 {
ArgumentType::Regular(self.i64_type().as_basic_type_enum())
} else if bytes <= 16 {
let word = self.i64_type().into();
let sret = self.struct_type(&[word, word]);

ArgumentType::Regular(sret.as_basic_type_enum())
ArgumentType::Regular(self.two_words().as_basic_type_enum())
} else {
// clang and Rust don't use "byval" for ARM64 when the
// struct is too large, so neither do we.
Expand Down Expand Up @@ -287,56 +287,21 @@ impl Context {
let bytes = layouts.target_data.get_abi_size(&typ) as u32;

match state.config.target.arch {
Architecture::Amd64 => {
// For both AMD64 and ARM64 the way structs are returned is the
// same. For more details, refer to argument_type().
Architecture::Amd64 | Architecture::Arm64 => {
if bytes <= 8 {
let bits = self.custom_int(size_in_bits(bytes));

ReturnType::Regular(bits.as_basic_type_enum())
} else if bytes <= 16 {
ReturnType::Regular(
self.abi_struct_type(layouts, typ).as_basic_type_enum(),
)
} else {
ReturnType::Struct(typ)
}
}
Architecture::Arm64 => {
if bytes <= 8 {
let bits = self.custom_int(size_in_bits(bytes));

ReturnType::Regular(bits.as_basic_type_enum())
} else if bytes <= 16 {
let word = self.i64_type().into();
let sret = self.struct_type(&[word, word]);

ReturnType::Regular(sret.as_basic_type_enum())
ReturnType::Regular(self.two_words().as_basic_type_enum())
} else {
ReturnType::Struct(typ)
}
}
}
}

fn abi_struct_type<'ctx>(
&'ctx self,
layouts: &Layouts<'ctx>,
typ: StructType<'ctx>,
) -> StructType<'ctx> {
// We need to ensure each field is treated as a single/whole value,
// otherwise (e.g. if a field is an array of bytes) we may end up
// generating code that's not ABI compliant.
let fields: Vec<_> = typ
.get_field_types_iter()
.map(|t| {
let bits =
size_in_bits(layouts.target_data.get_abi_size(&t) as u32);

self.custom_int(bits).as_basic_type_enum()
})
.collect();

self.struct_type(&fields)
}
}

#[cfg(test)]
Expand Down
14 changes: 9 additions & 5 deletions compiler/src/llvm/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1827,18 +1827,21 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> {
);
}
Instruction::Switch(ins) => {
let var = self.variables[&ins.register];
let val = self.builder.load_int(var);
let reg_var = self.variables[&ins.register];
let reg_typ = self.variable_types[&ins.register];
let bits = reg_typ.into_int_type().get_bit_width();
let reg_val =
self.builder.load(reg_typ, reg_var).into_int_value();
let mut cases = Vec::with_capacity(ins.blocks.len());

for (index, block) in ins.blocks.iter().enumerate() {
cases.push((
self.builder.u64_literal(index as u64),
self.builder.int_literal(bits, index as u64),
all_blocks[block.0],
));
}

self.builder.exhaustive_switch(val, &cases);
self.builder.exhaustive_switch(reg_val, &cases);
}
Instruction::Nil(ins) => {
let var = self.variables[&ins.register];
Expand All @@ -1854,7 +1857,8 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> {
}
Instruction::Int(ins) => {
let var = self.variables[&ins.register];
let val = self.builder.i64_literal(ins.value);
let val =
self.builder.int_literal(ins.bits as _, ins.value as u64);

self.builder.store(var, val);
}
Expand Down
23 changes: 22 additions & 1 deletion compiler/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,16 +319,36 @@ impl Block {
pub(crate) fn int_literal(
&mut self,
register: RegisterId,
bits: u8,
value: i64,
location: InstructionLocation,
) {
self.instructions.push(Instruction::Int(Box::new(IntLiteral {
register,
bits,
value,
location,
})));
}

pub(crate) fn u16_literal(
&mut self,
register: RegisterId,
value: u16,
location: InstructionLocation,
) {
self.int_literal(register, 16, value as i64, location)
}

pub(crate) fn i64_literal(
&mut self,
register: RegisterId,
value: i64,
location: InstructionLocation,
) {
self.int_literal(register, 64, value, location)
}

pub(crate) fn float_literal(
&mut self,
register: RegisterId,
Expand Down Expand Up @@ -1035,6 +1055,7 @@ pub(crate) struct Return {
#[derive(Clone)]
pub(crate) struct IntLiteral {
pub(crate) register: RegisterId,
pub(crate) bits: u8,
pub(crate) value: i64,
pub(crate) location: InstructionLocation,
}
Expand Down Expand Up @@ -1383,7 +1404,7 @@ impl Instruction {
format!("r{} = nil", v.register.0)
}
Instruction::Int(ref v) => {
format!("r{} = int {:?}", v.register.0, v.value)
format!("r{} = i{} {:?}", v.register.0, v.bits, v.value)
}
Instruction::Float(ref v) => {
format!("r{} = float {:?}", v.register.0, v.value)
Expand Down
Loading

0 comments on commit 87e6689

Please sign in to comment.