From 8738c95d6f80115f17cd9551a4c6534141c19039 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 26 Jun 2023 23:51:26 +0200 Subject: [PATCH 1/4] give `0` as a value to our enum attributes --- crates/compiler/build/src/program.rs | 2 +- crates/compiler/gen_llvm/src/llvm/bitcode.rs | 8 ++++---- crates/compiler/gen_llvm/src/llvm/build.rs | 4 ++-- crates/compiler/gen_llvm/src/llvm/lowlevel.rs | 6 +++--- crates/compiler/test_gen/src/helpers/llvm.rs | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/compiler/build/src/program.rs b/crates/compiler/build/src/program.rs index 09f8123f261..6b183d093f6 100644 --- a/crates/compiler/build/src/program.rs +++ b/crates/compiler/build/src/program.rs @@ -166,7 +166,7 @@ fn gen_from_mono_module_llvm<'a>( let kind_id = Attribute::get_named_enum_kind_id("alwaysinline"); debug_assert!(kind_id > 0); - let enum_attr = context.create_enum_attribute(kind_id, 1); + let enum_attr = context.create_enum_attribute(kind_id, 0); for function in module.get_functions() { let name = function.get_name().to_str().unwrap(); diff --git a/crates/compiler/gen_llvm/src/llvm/bitcode.rs b/crates/compiler/gen_llvm/src/llvm/bitcode.rs index 4762076da89..64dac3814e0 100644 --- a/crates/compiler/gen_llvm/src/llvm/bitcode.rs +++ b/crates/compiler/gen_llvm/src/llvm/bitcode.rs @@ -225,7 +225,7 @@ fn build_transform_caller_help<'a, 'ctx>( let kind_id = Attribute::get_named_enum_kind_id("alwaysinline"); debug_assert!(kind_id > 0); - let attr = env.context.create_enum_attribute(kind_id, 1); + let attr = env.context.create_enum_attribute(kind_id, 0); function_value.add_attribute(AttributeLoc::Function, attr); let entry = env.context.append_basic_block(function_value, "entry"); @@ -408,7 +408,7 @@ fn build_rc_wrapper<'a, 'ctx>( let kind_id = Attribute::get_named_enum_kind_id("alwaysinline"); debug_assert!(kind_id > 0); - let attr = env.context.create_enum_attribute(kind_id, 1); + let attr = env.context.create_enum_attribute(kind_id, 0); function_value.add_attribute(AttributeLoc::Function, attr); let entry = env.context.append_basic_block(function_value, "entry"); @@ -497,7 +497,7 @@ pub fn build_eq_wrapper<'a, 'ctx>( let kind_id = Attribute::get_named_enum_kind_id("alwaysinline"); debug_assert!(kind_id > 0); - let attr = env.context.create_enum_attribute(kind_id, 1); + let attr = env.context.create_enum_attribute(kind_id, 0); function_value.add_attribute(AttributeLoc::Function, attr); let entry = env.context.append_basic_block(function_value, "entry"); @@ -598,7 +598,7 @@ pub fn build_compare_wrapper<'a, 'ctx>( let kind_id = Attribute::get_named_enum_kind_id("alwaysinline"); debug_assert!(kind_id > 0); - let attr = env.context.create_enum_attribute(kind_id, 1); + let attr = env.context.create_enum_attribute(kind_id, 0); function_value.add_attribute(AttributeLoc::Function, attr); let entry = env.context.append_basic_block(function_value, "entry"); diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 38a06b4dac7..4d9cf61b795 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -5207,14 +5207,14 @@ fn build_proc_header<'a, 'ctx>( if false { let kind_id = Attribute::get_named_enum_kind_id("alwaysinline"); debug_assert!(kind_id > 0); - let enum_attr = env.context.create_enum_attribute(kind_id, 1); + let enum_attr = env.context.create_enum_attribute(kind_id, 0); fn_val.add_attribute(AttributeLoc::Function, enum_attr); } if false { let kind_id = Attribute::get_named_enum_kind_id("noinline"); debug_assert!(kind_id > 0); - let enum_attr = env.context.create_enum_attribute(kind_id, 1); + let enum_attr = env.context.create_enum_attribute(kind_id, 0); fn_val.add_attribute(AttributeLoc::Function, enum_attr); } diff --git a/crates/compiler/gen_llvm/src/llvm/lowlevel.rs b/crates/compiler/gen_llvm/src/llvm/lowlevel.rs index 63a9a15e740..285f4b929a8 100644 --- a/crates/compiler/gen_llvm/src/llvm/lowlevel.rs +++ b/crates/compiler/gen_llvm/src/llvm/lowlevel.rs @@ -1844,19 +1844,19 @@ fn throw_because_overflow<'ctx>(env: &Env<'_, 'ctx, '_>, message: &str) { // prevent inlining of this function let kind_id = Attribute::get_named_enum_kind_id("noinline"); debug_assert!(kind_id > 0); - let enum_attr = env.context.create_enum_attribute(kind_id, 1); + let enum_attr = env.context.create_enum_attribute(kind_id, 0); function_value.add_attribute(AttributeLoc::Function, enum_attr); // calling this function is unlikely let kind_id = Attribute::get_named_enum_kind_id("cold"); debug_assert!(kind_id > 0); - let enum_attr = env.context.create_enum_attribute(kind_id, 1); + let enum_attr = env.context.create_enum_attribute(kind_id, 0); function_value.add_attribute(AttributeLoc::Function, enum_attr); // this function never returns let kind_id = Attribute::get_named_enum_kind_id("noreturn"); debug_assert!(kind_id > 0); - let enum_attr = env.context.create_enum_attribute(kind_id, 1); + let enum_attr = env.context.create_enum_attribute(kind_id, 0); function_value.add_attribute(AttributeLoc::Function, enum_attr); // Add a basic block for the entry point diff --git a/crates/compiler/test_gen/src/helpers/llvm.rs b/crates/compiler/test_gen/src/helpers/llvm.rs index 2ec40d96245..6ad7cc29a37 100644 --- a/crates/compiler/test_gen/src/helpers/llvm.rs +++ b/crates/compiler/test_gen/src/helpers/llvm.rs @@ -192,7 +192,7 @@ fn create_llvm_module<'a>( let kind_id = Attribute::get_named_enum_kind_id("alwaysinline"); debug_assert!(kind_id > 0); - let attr = context.create_enum_attribute(kind_id, 1); + let attr = context.create_enum_attribute(kind_id, 0); for function in module.get_functions() { let name = function.get_name().to_str().unwrap(); From 7311c565f17c98c92cd03b4a4fa40ac17391e7cb Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 26 Jun 2023 23:54:03 +0200 Subject: [PATCH 2/4] use updated llvm type signatures --- crates/compiler/gen_llvm/src/llvm/build.rs | 25 ++++++++----- crates/compiler/gen_llvm/src/llvm/struct_.rs | 37 ++++++++++---------- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 4d9cf61b795..3870d5ba388 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -1941,8 +1941,12 @@ fn tag_pointer_set_tag_id<'ctx>( // NOTE: assumes the lower bits of `cast_pointer` are all 0 let indexed_pointer = unsafe { - env.builder - .build_in_bounds_gep(cast_pointer, &[tag_id_intval], "indexed_pointer") + env.builder.new_build_in_bounds_gep( + env.context.i8_type(), + cast_pointer, + &[tag_id_intval], + "indexed_pointer", + ) }; env.builder @@ -1994,7 +1998,14 @@ pub fn tag_pointer_clear_tag_id<'ctx>( "cast_to_i8_ptr", ); - let indexed_pointer = unsafe { env.builder.build_gep(cast_pointer, &[index], "new_ptr") }; + let indexed_pointer = unsafe { + env.builder.new_build_in_bounds_gep( + env.context.i8_type(), + cast_pointer, + &[index], + "new_ptr", + ) + }; env.builder .build_pointer_cast(indexed_pointer, pointer.get_type(), "cast_from_i8_ptr") @@ -3954,11 +3965,9 @@ fn expose_function_to_host_help_c_abi_gen_test<'a, 'ctx>( arguments_for_call.push(*arg); } else { match layout_interner.get_repr(*layout) { - LayoutRepr::Builtin(Builtin::List(_)) => { - let list_type = arg_type - .into_pointer_type() - .get_element_type() - .into_struct_type(); + repr @ LayoutRepr::Builtin(Builtin::List(_)) => { + let list_type = basic_type_from_layout(env, layout_interner, repr); + let loaded = env.builder.new_build_load( list_type, arg.into_pointer_value(), diff --git a/crates/compiler/gen_llvm/src/llvm/struct_.rs b/crates/compiler/gen_llvm/src/llvm/struct_.rs index 30da7eb55e0..64226445868 100644 --- a/crates/compiler/gen_llvm/src/llvm/struct_.rs +++ b/crates/compiler/gen_llvm/src/llvm/struct_.rs @@ -92,7 +92,15 @@ impl<'ctx> RocStruct<'ctx> { index_struct_value(env, layout_interner, field_layouts, *argument, index) } (Self::ByReference(ptr), LayoutRepr::Struct(field_layouts)) => { - index_struct_ptr(env, layout_interner, field_layouts, *ptr, index) + let struct_type = basic_type_from_layout(env, layout_interner, struct_layout); + index_struct_ptr( + env, + layout_interner, + struct_type.into_struct_type(), + field_layouts, + *ptr, + index, + ) } (other, layout) => { unreachable!( @@ -135,26 +143,26 @@ fn index_struct_value<'a, 'ctx>( fn index_struct_ptr<'a, 'ctx>( env: &Env<'a, 'ctx, '_>, layout_interner: &STLayoutInterner<'a>, + struct_type: StructType<'ctx>, field_layouts: &[InLayout<'a>], ptr: PointerValue<'ctx>, index: u64, ) -> BasicValueEnum<'ctx> { debug_assert!(!field_layouts.is_empty()); - let field_value = get_field_from_ptr( - env, - ptr, - index as _, - env.arena - .alloc(format!("struct_field_access_record_{}", index)), - ); - let field_layout = field_layouts[index as usize]; + let field_repr = layout_interner.get_repr(field_layout); + + let name = format!("struct_field_access_record_{}", index); + let field_value = env + .builder + .new_build_struct_gep(struct_type, ptr, index as u32, &name) + .unwrap(); load_roc_value( env, layout_interner, - layout_interner.get_repr(field_layout), + field_repr, field_value, "struct_field", ) @@ -171,15 +179,6 @@ fn get_field_from_value<'ctx>( .unwrap() } -fn get_field_from_ptr<'ctx>( - env: &Env<'_, 'ctx, '_>, - ptr: PointerValue<'ctx>, - index: u32, - name: &str, -) -> PointerValue<'ctx> { - env.builder.build_struct_gep(ptr, index, name).unwrap() -} - struct BuildStruct<'ctx> { struct_type: StructType<'ctx>, struct_val: StructValue<'ctx>, From 7c9c3d829cf10ad02ee90527809f7bb435dfc330 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 27 Jun 2023 00:37:09 +0200 Subject: [PATCH 3/4] use 32-bit GEP indices where easily possible --- crates/compiler/gen_llvm/src/llvm/build.rs | 17 +++++++++++------ .../compiler/gen_llvm/src/llvm/refcounting.rs | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 3870d5ba388..6bcae600c44 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -1931,7 +1931,7 @@ fn tag_pointer_set_tag_id<'ctx>( // we only have 3 bits, so can encode only 0..7 (or on 32-bit targets, 2 bits to encode 0..3) debug_assert!((tag_id as u32) < env.target_info.ptr_width() as u32); - let tag_id_intval = env.ptr_int().const_int(tag_id as u64, false); + let tag_id_intval = env.context.i32_type().const_int(tag_id as u64, false); let cast_pointer = env.builder.build_pointer_cast( pointer, @@ -1991,6 +1991,9 @@ pub fn tag_pointer_clear_tag_id<'ctx>( let current_tag_id = env.builder.build_and(as_int, mask, "masked"); let index = env.builder.build_int_neg(current_tag_id, "index"); + let index = env + .builder + .build_int_cast(index, env.context.i32_type(), "to_i32"); let cast_pointer = env.builder.build_pointer_cast( pointer, @@ -2444,8 +2447,9 @@ fn list_literal<'a, 'ctx>( // all elements are constants, so we can use the memory in the constants section directly // here we make a pointer to the first actual element (skipping the 0 bytes that // represent the refcount) - let zero = env.ptr_int().const_zero(); - let offset = env.ptr_int().const_int(zero_elements as _, false); + let i32_type = env.context.i32_type(); + let zero = i32_type.const_zero(); + let offset = i32_type.const_int(zero_elements as _, false); let ptr = unsafe { env.builder.new_build_in_bounds_gep( @@ -2474,7 +2478,7 @@ fn list_literal<'a, 'ctx>( // then replace the `undef`s with the values that we evaluate at runtime for (index, val) in runtime_evaluated_elements { - let index_val = ctx.i64_type().const_int(index as u64, false); + let index_val = ctx.i32_type().const_int(index as u64, false); let elem_ptr = unsafe { builder.new_build_in_bounds_gep(element_type, ptr, &[index_val], "index") }; @@ -2495,7 +2499,7 @@ fn list_literal<'a, 'ctx>( } ListLiteralElement::Symbol(symbol) => scope.load_symbol(symbol), }; - let index_val = ctx.i64_type().const_int(index as u64, false); + let index_val = ctx.i32_type().const_int(index as u64, false); let elem_ptr = unsafe { builder.new_build_in_bounds_gep(element_type, ptr, &[index_val], "index") }; @@ -6259,7 +6263,8 @@ fn define_global_str_literal_ptr<'ctx>( env.context.i8_type(), ptr, &[env - .ptr_int() + .context + .i32_type() .const_int(env.target_info.ptr_width() as u64, false)], "get_rc_ptr", ) diff --git a/crates/compiler/gen_llvm/src/llvm/refcounting.rs b/crates/compiler/gen_llvm/src/llvm/refcounting.rs index 06f9e73bf1e..6d03fa54c29 100644 --- a/crates/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/crates/compiler/gen_llvm/src/llvm/refcounting.rs @@ -61,7 +61,7 @@ impl<'ctx> PointerToRefcount<'ctx> { builder.build_pointer_cast(data_ptr, refcount_ptr_type, "as_usize_ptr"); // get a pointer to index -1 - let index_intvalue = refcount_type.const_int(-1_i64 as u64, false); + let index_intvalue = env.context.i32_type().const_int(-1_i64 as u64, false); let refcount_ptr = unsafe { builder.new_build_in_bounds_gep( env.ptr_int(), From 1c52c23c5faae9582d2cede9c0e6bf57b180283b Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 27 Jun 2023 11:02:24 +0200 Subject: [PATCH 4/4] Revert "use 32-bit GEP indices where easily possible" This reverts commit 7c9c3d829cf10ad02ee90527809f7bb435dfc330. --- crates/compiler/gen_llvm/src/llvm/build.rs | 17 ++++++----------- .../compiler/gen_llvm/src/llvm/refcounting.rs | 2 +- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 6bcae600c44..3870d5ba388 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -1931,7 +1931,7 @@ fn tag_pointer_set_tag_id<'ctx>( // we only have 3 bits, so can encode only 0..7 (or on 32-bit targets, 2 bits to encode 0..3) debug_assert!((tag_id as u32) < env.target_info.ptr_width() as u32); - let tag_id_intval = env.context.i32_type().const_int(tag_id as u64, false); + let tag_id_intval = env.ptr_int().const_int(tag_id as u64, false); let cast_pointer = env.builder.build_pointer_cast( pointer, @@ -1991,9 +1991,6 @@ pub fn tag_pointer_clear_tag_id<'ctx>( let current_tag_id = env.builder.build_and(as_int, mask, "masked"); let index = env.builder.build_int_neg(current_tag_id, "index"); - let index = env - .builder - .build_int_cast(index, env.context.i32_type(), "to_i32"); let cast_pointer = env.builder.build_pointer_cast( pointer, @@ -2447,9 +2444,8 @@ fn list_literal<'a, 'ctx>( // all elements are constants, so we can use the memory in the constants section directly // here we make a pointer to the first actual element (skipping the 0 bytes that // represent the refcount) - let i32_type = env.context.i32_type(); - let zero = i32_type.const_zero(); - let offset = i32_type.const_int(zero_elements as _, false); + let zero = env.ptr_int().const_zero(); + let offset = env.ptr_int().const_int(zero_elements as _, false); let ptr = unsafe { env.builder.new_build_in_bounds_gep( @@ -2478,7 +2474,7 @@ fn list_literal<'a, 'ctx>( // then replace the `undef`s with the values that we evaluate at runtime for (index, val) in runtime_evaluated_elements { - let index_val = ctx.i32_type().const_int(index as u64, false); + let index_val = ctx.i64_type().const_int(index as u64, false); let elem_ptr = unsafe { builder.new_build_in_bounds_gep(element_type, ptr, &[index_val], "index") }; @@ -2499,7 +2495,7 @@ fn list_literal<'a, 'ctx>( } ListLiteralElement::Symbol(symbol) => scope.load_symbol(symbol), }; - let index_val = ctx.i32_type().const_int(index as u64, false); + let index_val = ctx.i64_type().const_int(index as u64, false); let elem_ptr = unsafe { builder.new_build_in_bounds_gep(element_type, ptr, &[index_val], "index") }; @@ -6263,8 +6259,7 @@ fn define_global_str_literal_ptr<'ctx>( env.context.i8_type(), ptr, &[env - .context - .i32_type() + .ptr_int() .const_int(env.target_info.ptr_width() as u64, false)], "get_rc_ptr", ) diff --git a/crates/compiler/gen_llvm/src/llvm/refcounting.rs b/crates/compiler/gen_llvm/src/llvm/refcounting.rs index 6d03fa54c29..06f9e73bf1e 100644 --- a/crates/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/crates/compiler/gen_llvm/src/llvm/refcounting.rs @@ -61,7 +61,7 @@ impl<'ctx> PointerToRefcount<'ctx> { builder.build_pointer_cast(data_ptr, refcount_ptr_type, "as_usize_ptr"); // get a pointer to index -1 - let index_intvalue = env.context.i32_type().const_int(-1_i64 as u64, false); + let index_intvalue = refcount_type.const_int(-1_i64 as u64, false); let refcount_ptr = unsafe { builder.new_build_in_bounds_gep( env.ptr_int(),