diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b249f5b8..0da26d93 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -76,13 +76,12 @@ jobs: key: amd64-linux-gnu-${{ hashFiles('Cargo.lock', 'rust-toolchain.toml') }} - name: Run compiler tests run: cargo test - # We run tests with and without optimizations, such that we can catch any - # potential miscompilations introduced by optimizations. We only do this - # for this particular target as our optimizations aren't target specific. - - name: Run stdlib tests with optimizations + - name: Run stdlib tests with default optimizations run: 'cd std && cargo run -- test' - name: Run stdlib tests without optimizations run: 'cd std && cargo run -- test --opt=none' + - name: Run stdlib tests with aggressive optimizations + run: 'cd std && cargo run -- test --opt=aggressive' amd64-linux-musl: timeout-minutes: 15 @@ -97,8 +96,12 @@ jobs: key: amd64-linux-musl-${{ hashFiles('Cargo.lock', 'rust-toolchain.toml') }} - name: Run compiler tests run: cargo test - - name: Run stdlib tests + - name: Run stdlib tests with default optimizations run: 'cd std && cargo run -- test' + - name: Run stdlib tests without optimizations + run: 'cd std && cargo run -- test --opt=none' + - name: Run stdlib tests with aggressive optimizations + run: 'cd std && cargo run -- test --opt=aggressive' amd64-mac-native: timeout-minutes: 15 @@ -117,8 +120,12 @@ jobs: run: ./ci/mac.sh - name: Run compiler tests run: cargo test - - name: Run stdlib tests + - name: Run stdlib tests with default optimizations run: 'cd std && cargo run -- test' + - name: Run stdlib tests without optimizations + run: 'cd std && cargo run -- test --opt=none' + - name: Run stdlib tests with aggressive optimizations + run: 'cd std && cargo run -- test --opt=aggressive' arm64-mac-native: timeout-minutes: 15 @@ -137,8 +144,12 @@ jobs: run: ./ci/mac.sh - name: Run compiler tests run: cargo test - - name: Run stdlib tests + - name: Run stdlib tests with default optimizations run: 'cd std && cargo run -- test' + - name: Run stdlib tests without optimizations + run: 'cd std && cargo run -- test --opt=none' + - name: Run stdlib tests with aggressive optimizations + run: 'cd std && cargo run -- test --opt=aggressive' amd64-freebsd-native: timeout-minutes: 15 diff --git a/compiler/src/llvm.rs b/compiler/src/llvm.rs index 0a8f07db..4a005ff0 100644 --- a/compiler/src/llvm.rs +++ b/compiler/src/llvm.rs @@ -1,3 +1,4 @@ +pub(crate) mod abi; pub(crate) mod builder; pub(crate) mod constants; pub(crate) mod context; diff --git a/compiler/src/llvm/abi/amd64.rs b/compiler/src/llvm/abi/amd64.rs new file mode 100644 index 00000000..ffb405a7 --- /dev/null +++ b/compiler/src/llvm/abi/amd64.rs @@ -0,0 +1,42 @@ +use crate::llvm::context::{size_in_bits, Context}; +use crate::llvm::layouts::{ArgumentType, ReturnType}; +use inkwell::targets::TargetData; +use inkwell::types::{BasicType, StructType}; + +pub(crate) fn struct_argument<'ctx>( + ctx: &'ctx Context, + tdata: &TargetData, + typ: StructType<'ctx>, +) -> ArgumentType<'ctx> { + let bytes = tdata.get_abi_size(&typ) as u32; + + if bytes <= 8 { + let bits = ctx.custom_int(size_in_bits(bytes)); + + ArgumentType::Regular(bits.as_basic_type_enum()) + } else if bytes <= 16 { + ArgumentType::Regular( + ctx.binary_struct(tdata, typ).as_basic_type_enum(), + ) + } else { + ArgumentType::StructValue(typ) + } +} + +pub(crate) fn struct_return<'ctx>( + ctx: &'ctx Context, + tdata: &TargetData, + typ: StructType<'ctx>, +) -> ReturnType<'ctx> { + let bytes = tdata.get_abi_size(&typ) as u32; + + if bytes <= 8 { + let bits = ctx.custom_int(size_in_bits(bytes)); + + ReturnType::Regular(bits.as_basic_type_enum()) + } else if bytes <= 16 { + ReturnType::Regular(ctx.binary_struct(tdata, typ).as_basic_type_enum()) + } else { + ReturnType::Struct(typ) + } +} diff --git a/compiler/src/llvm/abi/arm64.rs b/compiler/src/llvm/abi/arm64.rs new file mode 100644 index 00000000..2c077a1d --- /dev/null +++ b/compiler/src/llvm/abi/arm64.rs @@ -0,0 +1,211 @@ +use crate::llvm::abi::generic::classify; +use crate::llvm::context::{size_in_bits, Context}; +use crate::llvm::layouts::{ArgumentType, ReturnType}; +use inkwell::targets::TargetData; +use inkwell::types::{BasicType, StructType}; + +pub(crate) fn struct_argument<'ctx>( + ctx: &'ctx Context, + tdata: &TargetData, + typ: StructType<'ctx>, +) -> ArgumentType<'ctx> { + if let Some(h) = homogeneous_struct(ctx, tdata, typ) { + return ArgumentType::Regular(h.as_basic_type_enum()); + } + + let bytes = tdata.get_abi_size(&typ) as u32; + + if bytes <= 8 { + return ArgumentType::Regular(ctx.i64_type().as_basic_type_enum()); + } + + if bytes <= 16 { + ArgumentType::Regular(ctx.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. + ArgumentType::Pointer + } +} + +pub(crate) fn struct_return<'ctx>( + ctx: &'ctx Context, + tdata: &TargetData, + typ: StructType<'ctx>, +) -> ReturnType<'ctx> { + let bytes = tdata.get_abi_size(&typ) as u32; + + if let Some(h) = homogeneous_struct(ctx, tdata, typ) { + return ReturnType::Regular(h.as_basic_type_enum()); + } + + if bytes <= 8 { + let bits = ctx.custom_int(size_in_bits(bytes)); + + return ReturnType::Regular(bits.as_basic_type_enum()); + } + + if bytes <= 16 { + ReturnType::Regular(ctx.two_words().as_basic_type_enum()) + } else { + ReturnType::Struct(typ) + } +} + +pub(crate) fn homogeneous_struct<'ctx>( + context: &'ctx Context, + tdata: &TargetData, + typ: StructType<'ctx>, +) -> Option> { + let mut classes = Vec::new(); + + classify(tdata, typ.as_basic_type_enum(), &mut classes); + + if classes.is_empty() || classes.len() > 4 { + return None; + } + + let first = classes[0]; + + if classes.iter().all(|&c| c.is_float() && c == first) { + let fields: Vec<_> = + classes.into_iter().map(|c| c.to_llvm_type(context)).collect(); + + Some(context.struct_type(&fields)) + } else { + None + } +} + +#[cfg(test)] +mod tests { + use super::*; + use inkwell::targets::{ + CodeModel, InitializationConfig, RelocMode, Target, TargetMachine, + TargetTriple, + }; + use inkwell::types::BasicTypeEnum; + use inkwell::OptimizationLevel; + + fn setup() -> TargetMachine { + Target::initialize_aarch64(&InitializationConfig::default()); + + let triple = TargetTriple::create("aarch64-unknown-linux-gnu"); + + Target::from_triple(&triple) + .unwrap() + .create_target_machine( + &triple, + "", + "", + OptimizationLevel::None, + RelocMode::PIC, + CodeModel::Default, + ) + .unwrap() + } + + #[test] + fn test_struct_argument_with_homogeneous() { + let machine = setup(); + let tdata = machine.get_target_data(); + let ctx = Context::new(); + let tests = [ + (vec![ctx.f32_type().into()], vec![ctx.f32_type().into()]), + (vec![ctx.f32_type().into(); 2], vec![ctx.f32_type().into(); 2]), + (vec![ctx.f32_type().into(); 3], vec![ctx.f32_type().into(); 3]), + (vec![ctx.f32_type().into(); 4], vec![ctx.f32_type().into(); 4]), + (vec![ctx.f64_type().into()], vec![ctx.f64_type().into()]), + (vec![ctx.f64_type().into(); 2], vec![ctx.f64_type().into(); 2]), + (vec![ctx.f64_type().into(); 3], vec![ctx.f64_type().into(); 3]), + (vec![ctx.f64_type().into(); 4], vec![ctx.f64_type().into(); 4]), + ]; + + for (in_fields, out_fields) in tests { + let inp = ctx.struct_type(in_fields.as_slice()); + let ArgumentType::Regular(BasicTypeEnum::StructType(out)) = + struct_argument(&ctx, &tdata, inp) + else { + panic!("expected a struct") + }; + + assert_eq!(out.get_field_types(), out_fields); + } + } + + #[test] + fn test_struct_argument_with_scalar() { + let machine = setup(); + let tdata = machine.get_target_data(); + let ctx = Context::new(); + let int64 = ctx.i64_type().as_basic_type_enum(); + let tests = [ + (vec![ctx.i8_type().into()], ArgumentType::Regular(int64)), + (vec![ctx.i16_type().into()], ArgumentType::Regular(int64)), + (vec![ctx.i32_type().into()], ArgumentType::Regular(int64)), + (vec![ctx.i64_type().into()], ArgumentType::Regular(int64)), + ( + vec![ctx.i32_type().into(), ctx.i32_type().into()], + ArgumentType::Regular(int64), + ), + ]; + + for (in_fields, exp) in tests { + let inp = ctx.struct_type(in_fields.as_slice()); + + assert_eq!(struct_argument(&ctx, &tdata, inp), exp); + } + } + + #[test] + fn test_struct_argument_sixteen_bytes() { + let machine = setup(); + let tdata = machine.get_target_data(); + let ctx = Context::new(); + let inp = + ctx.struct_type(&[ctx.i64_type().into(), ctx.i32_type().into()]); + let ArgumentType::Regular(BasicTypeEnum::StructType(out)) = + struct_argument(&ctx, &tdata, inp) + else { + panic!("expected a struct") + }; + + assert_eq!( + out.get_field_types(), + vec![ctx.i64_type().into(), ctx.i64_type().into()] + ); + } + + #[test] + fn test_struct_argument_large() { + let machine = setup(); + let tdata = machine.get_target_data(); + let ctx = Context::new(); + let inp = ctx.struct_type(&[ + ctx.i64_type().into(), + ctx.i64_type().into(), + ctx.i64_type().into(), + ]); + + assert_eq!(struct_argument(&ctx, &tdata, inp), ArgumentType::Pointer); + } + + #[test] + fn test_struct_argument_mixed_floats() { + let machine = setup(); + let tdata = machine.get_target_data(); + let ctx = Context::new(); + let inp = + ctx.struct_type(&[ctx.f64_type().into(), ctx.f32_type().into()]); + let ArgumentType::Regular(BasicTypeEnum::StructType(out)) = + struct_argument(&ctx, &tdata, inp) + else { + panic!("expected a struct") + }; + + assert_eq!( + out.get_field_types(), + vec![ctx.i64_type().into(), ctx.i64_type().into()] + ); + } +} diff --git a/compiler/src/llvm/abi/generic.rs b/compiler/src/llvm/abi/generic.rs new file mode 100644 index 00000000..54e5519d --- /dev/null +++ b/compiler/src/llvm/abi/generic.rs @@ -0,0 +1,166 @@ +use crate::llvm::context::Context; +use inkwell::targets::TargetData; +use inkwell::types::{BasicType, BasicTypeEnum}; +use std::cmp::max; + +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +pub(crate) enum Class { + Int(u64), + Float(u64), +} + +impl Class { + pub(crate) fn to_llvm_type(self, context: &Context) -> BasicTypeEnum { + match self { + Class::Int(bytes) => { + context.custom_int(bytes as u32 * 8).as_basic_type_enum() + } + Class::Float(4) => context.f32_type().as_basic_type_enum(), + Class::Float(_) => context.f64_type().as_basic_type_enum(), + } + } + + pub(crate) fn is_float(self) -> bool { + matches!(self, Class::Float(_)) + } +} + +pub(crate) fn classify( + target_data: &TargetData, + typ: BasicTypeEnum, + classes: &mut Vec, +) { + match typ { + BasicTypeEnum::StructType(t) => { + for field in t.get_field_types_iter() { + classify(target_data, field, classes); + } + } + BasicTypeEnum::ArrayType(t) => { + let field = t.get_element_type(); + + for _ in 0..t.len() { + classify(target_data, field, classes); + } + } + BasicTypeEnum::FloatType(t) => { + classes.push(Class::Float(target_data.get_abi_size(&t))) + } + BasicTypeEnum::IntType(t) => { + classes.push(Class::Int(target_data.get_abi_size(&t))) + } + BasicTypeEnum::PointerType(t) => { + classes.push(Class::Int(target_data.get_abi_size(&t))) + } + BasicTypeEnum::VectorType(_) => { + panic!("vector types are not yet supported") + } + } +} + +pub(crate) fn combine_classes( + classes: Vec, + align: u64, +) -> (Class, Class) { + let mut a = 0; + let mut a_float = true; + let mut b = 0; + let mut b_float = true; + + for cls in classes { + match cls { + Class::Int(v) if a + v <= 8 => { + a += v; + a_float = false; + } + Class::Float(v) if a + v <= 8 => a += v, + Class::Int(v) => { + b += v; + b_float = false; + } + Class::Float(v) => b += v, + } + } + + a = max(a, align); + b = max(b, align); + + ( + if a_float { Class::Float(a) } else { Class::Int(a) }, + if b_float { Class::Float(b) } else { Class::Int(b) }, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_context_type_sizes() { + let ctx = Context::new(); + + // These tests exists just to make sure the layouts match that which the + // runtime expects. This would only ever fail if Rust suddenly changes + // the layout of String/Vec. + assert_eq!(ctx.rust_string_type().len(), 24); + assert_eq!(ctx.rust_vec_type().len(), 24); + } + + #[test] + fn test_combine_classes() { + assert_eq!( + combine_classes(vec![Class::Float(4), Class::Float(4)], 4), + (Class::Float(8), Class::Float(4)) + ); + assert_eq!( + combine_classes( + vec![Class::Float(4), Class::Float(4), Class::Int(4)], + 4 + ), + (Class::Float(8), Class::Int(4)) + ); + assert_eq!( + combine_classes( + vec![ + Class::Float(4), + Class::Float(4), + Class::Int(4), + Class::Int(4) + ], + 4 + ), + (Class::Float(8), Class::Int(8)) + ); + assert_eq!( + combine_classes( + vec![ + Class::Float(4), + Class::Int(4), + Class::Int(4), + Class::Int(4) + ], + 4 + ), + (Class::Int(8), Class::Int(8)) + ); + assert_eq!( + combine_classes( + vec![ + Class::Int(4), + Class::Float(4), + Class::Int(4), + Class::Int(4) + ], + 4 + ), + (Class::Int(8), Class::Int(8)) + ); + assert_eq!( + combine_classes( + vec![Class::Int(1), Class::Int(4), Class::Int(8)], + 8 + ), + (Class::Int(8), Class::Int(8)) + ); + } +} diff --git a/compiler/src/llvm/abi/mod.rs b/compiler/src/llvm/abi/mod.rs new file mode 100644 index 00000000..2961dc44 --- /dev/null +++ b/compiler/src/llvm/abi/mod.rs @@ -0,0 +1,3 @@ +pub(crate) mod amd64; +pub(crate) mod arm64; +pub(crate) mod generic; diff --git a/compiler/src/llvm/builder.rs b/compiler/src/llvm/builder.rs index d0bdc619..974aaa2f 100644 --- a/compiler/src/llvm/builder.rs +++ b/compiler/src/llvm/builder.rs @@ -11,7 +11,10 @@ use inkwell::debug_info::{ DWARFSourceLanguage, DebugInfoBuilder, }; use inkwell::module::{FlagBehavior, Module as InkwellModule}; -use inkwell::types::{ArrayType, BasicType, FunctionType, StructType}; +use inkwell::targets::TargetData; +use inkwell::types::{ + ArrayType, BasicType, BasicTypeEnum, FunctionType, StructType, +}; use inkwell::values::{ AggregateValue, ArrayValue, BasicMetadataValueEnum, BasicValue, BasicValueEnum, CallSiteValue, FloatValue, FunctionValue, @@ -152,6 +155,44 @@ impl<'ctx> Builder<'ctx> { self.store(field_ptr, value); } + pub(crate) fn memcpy( + &self, + target_data: &TargetData, + from: PointerValue<'ctx>, + from_type: BasicTypeEnum<'ctx>, + to: PointerValue<'ctx>, + to_type: BasicTypeEnum<'ctx>, + ) { + let len = self.u64_literal(target_data.get_abi_size(&to_type)); + let from_align = target_data.get_abi_alignment(&from_type); + let to_align = target_data.get_abi_alignment(&to_type); + + self.inner.build_memcpy(to, to_align, from, from_align, len).unwrap(); + } + + /// Copies a value to a pointer using memcpy. + /// + /// When passing structs as arguments or returning them, their types may + /// differ from the destination type. In addition, the ABI may require that + /// the data be copied explicitly instead of being passed as-is. + /// + /// Using this method we can easily handle these cases and leave it up to + /// LLVM to optimize and generate the correct code. The approach taken here + /// is similar to what clang and Rust do. + pub(crate) fn copy_value( + &self, + target_data: &TargetData, + from: BasicValueEnum<'ctx>, + to: PointerValue<'ctx>, + to_type: BasicTypeEnum<'ctx>, + ) { + let from_type = from.get_type(); + let tmp = self.new_stack_slot(from_type); + + self.store(tmp, from); + self.memcpy(target_data, tmp, from_type, to, to_type); + } + pub(crate) fn store>( &self, variable: PointerValue<'ctx>, diff --git a/compiler/src/llvm/context.rs b/compiler/src/llvm/context.rs index e3c78d2e..9f195601 100644 --- a/compiler/src/llvm/context.rs +++ b/compiler/src/llvm/context.rs @@ -1,3 +1,6 @@ +use crate::llvm::abi::amd64; +use crate::llvm::abi::arm64; +use crate::llvm::abi::generic::{classify, combine_classes}; use crate::llvm::layouts::{ArgumentType, Layouts, ReturnType}; use crate::state::State; use crate::target::Architecture; @@ -5,6 +8,7 @@ use inkwell::attributes::Attribute; use inkwell::basic_block::BasicBlock; use inkwell::builder::Builder; use inkwell::module::Module; +use inkwell::targets::TargetData; use inkwell::types::{ AnyTypeEnum, ArrayType, BasicType, BasicTypeEnum, FloatType, IntType, PointerType, StructType, VoidType, @@ -18,7 +22,7 @@ use types::{ FLOAT_ID, INT_ID, NIL_ID, }; -fn size_in_bits(bytes: u32) -> u32 { +pub(crate) fn size_in_bits(bytes: u32) -> u32 { // LLVM crashes when passing/returning zero sized types (e.g. structs). In // addition, such types aren't useful in Inko, so we enforce a minimum size // of 1 bit. @@ -213,45 +217,16 @@ impl Context { pub(crate) fn argument_type<'ctx>( &'ctx self, state: &State, - layouts: &Layouts<'ctx>, + tdata: &TargetData, typ: BasicTypeEnum<'ctx>, ) -> ArgumentType<'ctx> { let BasicTypeEnum::StructType(typ) = typ else { return ArgumentType::Regular(typ); }; - let bytes = layouts.target_data.get_abi_size(&typ) as u32; match state.config.target.arch { - Architecture::Amd64 => { - if bytes <= 8 { - let bits = self.custom_int(size_in_bits(bytes)); - - ArgumentType::Regular(bits.as_basic_type_enum()) - } else if bytes <= 16 { - // 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) - } - } - Architecture::Arm64 => { - if bytes <= 8 { - ArgumentType::Regular(self.i64_type().as_basic_type_enum()) - } else if bytes <= 16 { - 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. - ArgumentType::Pointer - } - } + Architecture::Amd64 => amd64::struct_argument(self, tdata, typ), + Architecture::Arm64 => arm64::struct_argument(self, tdata, typ), } } @@ -268,7 +243,7 @@ impl Context { method.return_type(&state.db), ); - self.return_type(state, layouts, typ) + self.return_type(state, layouts.target_data, typ) } else { ReturnType::None } @@ -277,45 +252,33 @@ impl Context { pub(crate) fn return_type<'ctx>( &'ctx self, state: &State, - layouts: &Layouts<'ctx>, + tdata: &TargetData, typ: BasicTypeEnum<'ctx>, ) -> ReturnType<'ctx> { let BasicTypeEnum::StructType(typ) = typ else { return ReturnType::Regular(typ); }; - let bytes = layouts.target_data.get_abi_size(&typ) as u32; - match state.config.target.arch { - // 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.two_words().as_basic_type_enum()) - } else { - ReturnType::Struct(typ) - } - } + Architecture::Amd64 => amd64::struct_return(self, tdata, typ), + Architecture::Arm64 => arm64::struct_return(self, tdata, typ), } } -} -#[cfg(test)] -mod tests { - use super::*; + pub(crate) fn binary_struct<'ctx>( + &'ctx self, + target_data: &TargetData, + typ: StructType<'ctx>, + ) -> StructType<'ctx> { + let mut classes = Vec::new(); + + classify(target_data, typ.as_basic_type_enum(), &mut classes); - #[test] - fn test_context_type_sizes() { - let ctx = Context::new(); + let (a, b) = combine_classes( + classes, + target_data.get_abi_alignment(&typ) as u64, + ); - // These tests exists just to make sure the layouts match that which the - // runtime expects. This would only ever fail if Rust suddenly changes - // the layout of String/Vec. - assert_eq!(ctx.rust_string_type().len(), 24); - assert_eq!(ctx.rust_vec_type().len(), 24); + self.struct_type(&[a.to_llvm_type(self), b.to_llvm_type(self)]) } } diff --git a/compiler/src/llvm/layouts.rs b/compiler/src/llvm/layouts.rs index b1654bb9..26ba339c 100644 --- a/compiler/src/llvm/layouts.rs +++ b/compiler/src/llvm/layouts.rs @@ -47,7 +47,7 @@ impl Sized { } } -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Eq, PartialEq, Debug)] pub(crate) enum ArgumentType<'ctx> { /// The argument should be passed as a normal value. Regular(BasicTypeEnum<'ctx>), @@ -132,7 +132,7 @@ impl<'ctx> Method<'ctx> { .chain(method.argument_types(db)) { let raw = context.llvm_type(db, layouts, typ); - let typ = context.argument_type(state, layouts, raw); + let typ = context.argument_type(state, layouts.target_data, raw); args.push(typ); } diff --git a/compiler/src/llvm/passes.rs b/compiler/src/llvm/passes.rs index 042bb59d..2e5ef92e 100644 --- a/compiler/src/llvm/passes.rs +++ b/compiler/src/llvm/passes.rs @@ -32,7 +32,7 @@ use inkwell::targets::{ CodeModel, FileType, InitializationConfig, RelocMode, Target, TargetMachine, TargetTriple, }; -use inkwell::types::{BasicType, BasicTypeEnum, FunctionType}; +use inkwell::types::{BasicType, BasicTypeEnum, FunctionType, StructType}; use inkwell::values::{ BasicMetadataValueEnum, BasicValue, BasicValueEnum, FloatValue, FunctionValue, GlobalValue, IntValue, PointerValue, @@ -950,7 +950,7 @@ pub struct LowerMethod<'shared, 'module, 'ctx> { /// The pointer to write structs to when performing an ABI compliant /// structure return. - struct_return_value: Option>, + struct_return_value: Option<(PointerValue<'ctx>, StructType<'ctx>)>, } impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> { @@ -967,10 +967,13 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> { builder.switch_to_block(entry_block); - let sret = layouts.methods[method.id.0 as usize] - .returns - .is_struct() - .then(|| builder.argument(0).into_pointer_value()); + let sret = if let ReturnType::Struct(t) = + layouts.methods[method.id.0 as usize].returns + { + Some((builder.argument(0).into_pointer_value(), t)) + } else { + None + }; LowerMethod { shared, @@ -1013,6 +1016,17 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> { let val = self.builder.load(typ, arg.into_pointer_value()); self.builder.store(var, val); + } else if typ.is_struct_type() { + // When passing structs the argument type might be an i64 in + // case the struct fits in a single i64. We need to use a memcpy + // here to ensure the generated code is correct (i.e. just a + // load with the target struct type isn't sufficient). + self.builder.copy_value( + self.layouts.target_data, + arg, + var, + typ, + ); } else { self.builder.store(var, arg); } @@ -1791,11 +1805,18 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> { Instruction::Return(ins) => { let var = self.variables[&ins.register]; - if let Some(ptr) = self.struct_return_value { + if let Some((ptr, to_typ)) = self.struct_return_value { let typ = self.variable_types[&ins.register]; let val = self.builder.load(typ, var); - self.builder.store(ptr, val); + self.builder.copy_value( + self.layouts.target_data, + val, + ptr, + to_typ.as_basic_type_enum(), + ); + + //self.builder.store(ptr, val); self.builder.return_value(None); } else { // When returning a struct on the stack, the return type @@ -1805,6 +1826,8 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> { // For example, if the struct is `{ i64 }` we may // in fact return a value of type `i64`. While both have the // same layout, they're not compatible at the LLVM level. + // + // TODO: use memcpy when returning structs let typ = self .builder .function @@ -2063,7 +2086,7 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> { layout.returns = self.builder.context.return_type( self.shared.state, - self.layouts, + self.layouts.target_data, reg_typ, ); @@ -2075,7 +2098,7 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> { let raw = self.variable_types[reg]; let typ = self.builder.context.argument_type( self.shared.state, - self.layouts, + self.layouts.target_data, raw, ); @@ -2131,7 +2154,7 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> { layout.returns = ReturnType::Regular(reg_typ); layout.arguments.push(self.builder.context.argument_type( self.shared.state, - self.layouts, + self.layouts.target_data, rec_typ, )); @@ -2720,10 +2743,26 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> { for (idx, reg) in receiver.iter().chain(arguments.iter()).enumerate() { let idx = if sret.is_some() { idx + 1 } else { idx }; let var = self.variables[reg]; + let typ = self.variable_types[reg]; match layout.arguments.get(idx).cloned() { Some(ArgumentType::Regular(t)) => { - args.push(self.builder.load(t, var).into()); + let raw_val = self.builder.load(typ, var); + let val = if t != typ { + let tmp = self.builder.new_stack_slot(t); + + self.builder.copy_value( + self.layouts.target_data, + raw_val, + tmp, + t, + ); + self.builder.load(t, tmp) + } else { + raw_val + }; + + args.push(val.into()); } Some(ArgumentType::StructValue(t)) => { attrs.push(( @@ -2745,14 +2784,13 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> { None => { // We may run into this case when calling a variadic // function and passing more arguments than are defined. - let typ = self.variable_types[reg]; - args.push(self.builder.load(typ, var).into()); } } } let reg_var = self.variables[®ister]; + let reg_typ = self.variable_types[®ister]; let call_site = match kind { CallKind::Direct(f) => self.builder.direct_call(f, &args), CallKind::Indirect(t, f) => self.builder.indirect_call(t, f, &args), @@ -2765,11 +2803,21 @@ impl<'shared, 'module, 'ctx> LowerMethod<'shared, 'module, 'ctx> { if layout.returns.is_regular() { let val = call_site.try_as_basic_value().left().unwrap(); - self.builder.store(reg_var, val); + self.builder.copy_value( + self.layouts.target_data, + val, + reg_var, + reg_typ, + ); } else if let Some((typ, tmp)) = sret { let val = self.builder.load(typ, tmp); - self.builder.store(reg_var, val); + self.builder.copy_value( + self.layouts.target_data, + val, + reg_var, + reg_typ, + ); } if self.register_type(register).is_never(&self.shared.state.db) { diff --git a/inko/src/command/test.rs b/inko/src/command/test.rs index 5c6ee423..73c69973 100644 --- a/inko/src/command/test.rs +++ b/inko/src/command/test.rs @@ -91,14 +91,30 @@ pub(crate) fn run(arguments: &[String]) -> Result { compiler.print_diagnostics(); match result { - Ok(exe) => Command::new(exe) - .args(matches.free) - .spawn() - .and_then(|mut child| child.wait()) - .map_err(|err| { - Error::from(format!("Failed to run the tests: {}", err)) - }) - .map(|status| status.code().unwrap_or(0)), + Ok(exe) => { + let status = Command::new(exe) + .args(matches.free) + .spawn() + .and_then(|mut child| child.wait()) + .map_err(|err| { + Error::from(format!("failed to run the tests: {}", err)) + })?; + + // If the program is terminated due to e.g. a SIGSEGV, the status + // code is missing and we default to 1. If we don't explicitly + // handle this, the program might terminate silently in the event of + // such a signal. + let code = status.code().unwrap_or(1); + + if code == 0 { + Ok(0) + } else { + Err(Error::from(format!( + "the tests runner exited with status code {}", + code + ))) + } + } Err(CompileError::Invalid) => Ok(1), Err(CompileError::Internal(msg)) => Err(Error::from(msg)), }