Skip to content

Commit 7807d9d

Browse files
mhaselmhasel
authored
refactor: use memcpy for by-val aggregate type input parameters (PLC-lang#1196)
Aggregate VAR_INPUT args to function calls are now generated/passed as pointers and then memcpyd into a local variable instead of passing it by value and using store.
1 parent 73f559d commit 7807d9d

File tree

29 files changed

+955
-519
lines changed

29 files changed

+955
-519
lines changed

‎src/codegen/generators/expression_generator.rs

Lines changed: 7 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -714,13 +714,16 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
714714
})?;
715715

716716
if let Some((declaration_type, type_name)) = parameter_info {
717-
let argument: BasicValueEnum = if declaration_type.is_by_ref() {
717+
let argument: BasicValueEnum = if declaration_type.is_by_ref()
718+
|| (self.index.get_effective_type_or_void_by_name(type_name).is_aggregate_type()
719+
&& declaration_type.is_input())
720+
{
718721
let declared_parameter = declared_parameters.get(i);
719722
self.generate_argument_by_ref(parameter, type_name, declared_parameter.copied())?
720723
} else {
721724
// by val
722725
if !parameter.is_empty_statement() {
723-
self.generate_argument_by_val(type_name, parameter)?
726+
self.generate_expression(parameter)?
724727
} else if let Some(param) = declared_parameters.get(i) {
725728
self.generate_empty_expression(param)?
726729
} else {
@@ -757,83 +760,6 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
757760
Ok(result.into_iter().map(|(_, v)| v.into()).collect::<Vec<BasicMetadataValueEnum>>())
758761
}
759762

760-
fn generate_argument_by_val(
761-
&self,
762-
type_name: &str,
763-
param_statement: &AstNode,
764-
) -> Result<BasicValueEnum<'ink>, Diagnostic> {
765-
let Some(type_info) = self.index.find_effective_type_by_name(type_name) else {
766-
return self.generate_expression(param_statement);
767-
};
768-
769-
if type_info.is_string() {
770-
return self.generate_string_argument(type_info, param_statement);
771-
}
772-
773-
// https://github.com/PLC-lang/rusty/issues/1037:
774-
// This if-statement covers the case where we want to convert a pointer into its actual
775-
// type, e.g. if an argument is passed into a function A by-ref (INOUT) which in turn is
776-
// passed into another function B by-val (INPUT) then the pointer argument in function A has
777-
// to be bit-cast into its actual type before passing it into function B.
778-
if type_info.is_aggregate_type() && !type_info.is_vla() {
779-
let deref = self.generate_expression_value(param_statement)?;
780-
781-
if deref.get_basic_value_enum().is_pointer_value() {
782-
let ty = self.llvm_index.get_associated_type(type_name)?;
783-
let cast = self.llvm.builder.build_bitcast(
784-
deref.get_basic_value_enum(),
785-
ty.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC)),
786-
"",
787-
);
788-
789-
let load = self.llvm.builder.build_load(
790-
cast.into_pointer_value(),
791-
&self.get_load_name(param_statement).unwrap_or_default(),
792-
);
793-
794-
if let Some(target_ty) = self.annotations.get_type_hint(param_statement, self.index) {
795-
let actual_ty = self.annotations.get_type_or_void(param_statement, self.index);
796-
let annotation = self.annotations.get(param_statement);
797-
798-
return Ok(cast_if_needed!(self, target_ty, actual_ty, load, annotation));
799-
}
800-
801-
return Ok(load);
802-
}
803-
}
804-
805-
// Fallback
806-
self.generate_expression(param_statement)
807-
}
808-
809-
/// Before passing a string to a function, it is copied to a new string with the
810-
/// appropriate size for the called function
811-
fn generate_string_argument(
812-
&self,
813-
type_info: &DataType,
814-
argument: &AstNode,
815-
) -> Result<BasicValueEnum<'ink>, Diagnostic> {
816-
// allocate a temporary string of correct size and pass it
817-
let llvm_type = self
818-
.llvm_index
819-
.find_associated_type(type_info.get_name())
820-
.ok_or_else(|| Diagnostic::unknown_type(type_info.get_name(), argument.get_location()))?;
821-
let temp_variable = self.llvm.builder.build_alloca(llvm_type, "");
822-
self.llvm
823-
.builder
824-
.build_memset(
825-
temp_variable,
826-
1,
827-
self.llvm.context.i8_type().const_zero(),
828-
llvm_type
829-
.size_of()
830-
.ok_or_else(|| Diagnostic::unknown_type(type_info.get_name(), argument.get_location()))?,
831-
)
832-
.map_err(|it| Diagnostic::codegen_error(it, argument.get_location()))?;
833-
self.generate_store(temp_variable, type_info.get_type_information(), argument)?;
834-
Ok(self.llvm.builder.build_load(temp_variable, ""))
835-
}
836-
837763
/// generates a value that is passed by reference
838764
/// this generates and returns a PointerValue
839765
/// pointing to the given `argument`
@@ -951,7 +877,7 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
951877
self.index.get_variadic_member(pou.get_name()),
952878
)
953879
} else {
954-
self.generate_argument_by_val(type_name, param_statement)
880+
self.generate_expression(param_statement)
955881
}
956882
})
957883
})
@@ -1048,7 +974,6 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
1048974
vec![class_struct.as_basic_value_enum().into(), parameter_struct.as_basic_value_enum().into()]
1049975
})
1050976
.unwrap_or_else(|| vec![parameter_struct.as_basic_value_enum().into()]);
1051-
1052977
for (i, stmt) in passed_parameters.iter().enumerate() {
1053978
let parameter = self.generate_call_struct_argument_assignment(&CallParameterAssignment {
1054979
assignment_statement: stmt,
@@ -1174,8 +1099,7 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
11741099
.unwrap_or_else(|| self.index.get_void_type().get_type_information());
11751100

11761101
if let DataTypeInformation::Pointer { auto_deref: true, inner_type_name, .. } = parameter {
1177-
//this is VAR_IN_OUT assignemt, so don't load the value, assign the pointer
1178-
1102+
//this is a VAR_IN_OUT assignment, so don't load the value, assign the pointer
11791103
//expression may be empty -> generate a local variable for it
11801104
let generated_exp = if expression.is_empty_statement() {
11811105
let temp_type =
@@ -2379,7 +2303,6 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
23792303
};
23802304

23812305
let struct_ptr = reference.get_basic_value_enum().into_pointer_value();
2382-
23832306
// GEPs into the VLA struct, getting an LValue for the array pointer and the dimension array and
23842307
// dereferences the former
23852308
let arr_ptr_gep = self.llvm.builder.build_struct_gep(struct_ptr, 0, "vla_arr_gep")?;

‎src/codegen/generators/pou_generator.rs

Lines changed: 107 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use crate::{
1515
},
1616
index::{self, ImplementationType},
1717
resolver::{AstAnnotations, Dependency},
18-
typesystem::{self, DataType, VarArgs},
18+
typesystem::{DataType, DataTypeInformation, VarArgs, DINT_TYPE},
1919
};
2020
use std::collections::HashMap;
2121

@@ -24,9 +24,10 @@ use std::collections::HashMap;
2424
/// - generates a struct-datatype for the POU's members
2525
/// - generates a function for the pou
2626
/// - declares a global instance if the POU is a PROGRAM
27-
use crate::index::{ImplementationIndexEntry, VariableIndexEntry};
27+
use crate::index::{ArgumentType, ImplementationIndexEntry, VariableIndexEntry};
2828

2929
use crate::index::Index;
30+
use index::VariableType;
3031
use indexmap::{IndexMap, IndexSet};
3132
use inkwell::{
3233
module::Module,
@@ -190,32 +191,59 @@ impl<'ink, 'cg> PouGenerator<'ink, 'cg> {
190191
new_llvm_index: &mut LlvmTypedIndex<'ink>,
191192
) -> Result<FunctionValue<'ink>, Diagnostic> {
192193
let declared_parameters = self.index.get_declared_parameters(implementation.get_call_name());
193-
194194
let parameters = self
195195
.collect_parameters_for_implementation(implementation)?
196196
.iter()
197197
.enumerate()
198-
.map(|(i, p)| match declared_parameters.get(i) {
199-
Some(v)
200-
if v.is_in_parameter_by_ref() &&
201-
// parameters by ref will always be a pointer
202-
p.into_pointer_type().get_element_type().is_array_type() =>
203-
{
204-
// for array types we will generate a pointer to the arrays element type
205-
// not a pointer to array
206-
let ty = p
207-
.into_pointer_type()
208-
.get_element_type()
209-
.into_array_type()
210-
.get_element_type()
211-
.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC));
212-
213-
// set the new type for further codegen
214-
let _ = new_llvm_index.associate_type(v.get_type_name(), ty.into());
215-
216-
ty.into()
198+
.map(|(i, p)| {
199+
let param = declared_parameters.get(i);
200+
let dti = param.map(|it| self.index.get_type_information_or_void(it.get_type_name()));
201+
match param {
202+
Some(v)
203+
if v.is_in_parameter_by_ref() &&
204+
// parameters by ref will always be a pointer
205+
p.into_pointer_type().get_element_type().is_array_type() =>
206+
{
207+
// for by-ref array types we will generate a pointer to the arrays element type
208+
// not a pointer to array
209+
let ty = p
210+
.into_pointer_type()
211+
.get_element_type()
212+
.into_array_type()
213+
.get_element_type()
214+
.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC));
215+
216+
// set the new type for further codegen
217+
let _ = new_llvm_index.associate_type(v.get_type_name(), ty.into());
218+
219+
ty.into()
220+
}
221+
_ => {
222+
dti.map(|it| {
223+
if !matches!(
224+
implementation.get_implementation_type(),
225+
ImplementationType::Function
226+
) {
227+
return *p;
228+
}
229+
// for aggregate function parameters we will generate a pointer instead of the value type.
230+
// it will then later be memcopied into a locally allocated variable
231+
match it {
232+
DataTypeInformation::Struct { .. } => p
233+
.into_struct_type()
234+
.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC))
235+
.into(),
236+
DataTypeInformation::Array { .. } | DataTypeInformation::String { .. } => p
237+
.into_array_type()
238+
.get_element_type()
239+
.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC))
240+
.into(),
241+
_ => *p,
242+
}
243+
})
244+
.unwrap_or(*p)
245+
}
217246
}
218-
_ => *p,
219247
})
220248
.collect::<Vec<BasicMetadataTypeEnum>>();
221249

@@ -521,11 +549,10 @@ impl<'ink, 'cg> PouGenerator<'ink, 'cg> {
521549
let ptr_value = params_iter
522550
.next()
523551
.ok_or_else(|| Diagnostic::missing_function(m.source_location.clone()))?;
524-
525-
let ptr = self
526-
.llvm
527-
.create_local_variable(m.get_name(), &index.get_associated_type(m.get_type_name())?);
528-
552+
let member_type_name = m.get_type_name();
553+
let type_info = self.index.get_type_information_or_void(member_type_name);
554+
let ty = index.get_associated_type(member_type_name)?;
555+
let ptr = self.llvm.create_local_variable(m.get_name(), &ty);
529556
if let Some(block) = self.llvm.builder.get_insert_block() {
530557
debug.add_variable_declaration(
531558
m.get_qualified_name(),
@@ -536,7 +563,57 @@ impl<'ink, 'cg> PouGenerator<'ink, 'cg> {
536563
m.source_location.get_column(),
537564
);
538565
}
539-
self.llvm.builder.build_store(ptr, ptr_value);
566+
567+
if matches!(m.argument_type, ArgumentType::ByVal(VariableType::Input))
568+
&& type_info.is_aggregate()
569+
{
570+
// a by-value aggregate type => we need to memcpy the data into the local variable
571+
let ty = if ty.is_array_type() {
572+
ty.into_array_type()
573+
.get_element_type()
574+
.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC))
575+
} else {
576+
ty.into_struct_type().ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC))
577+
};
578+
let bitcast = self.llvm.builder.build_bitcast(ptr, ty, "bitcast").into_pointer_value();
579+
let (size, alignment) = if let DataTypeInformation::String { size, encoding } = type_info
580+
{
581+
// since passed string args might be larger than the local acceptor, we need to first memset the local variable to 0
582+
let size = size.as_int_value(self.index).map_err(|err| {
583+
Diagnostic::codegen_error(err.as_str(), m.source_location.clone())
584+
})? as u64;
585+
let char_width = encoding.get_bytes_per_char();
586+
self.llvm
587+
.builder
588+
.build_memset(
589+
bitcast,
590+
char_width,
591+
self.llvm.context.i8_type().const_zero(),
592+
self.llvm.context.i64_type().const_int(size * char_width as u64, true),
593+
)
594+
.map_err(|e| Diagnostic::codegen_error(e, m.source_location.clone()))?;
595+
(
596+
// we then reduce the amount of bytes to be memcopied by the equivalent of one grapheme in bytes to preserve the null-terminator
597+
self.llvm.context.i64_type().const_int((size - 1) * char_width as u64, true),
598+
char_width,
599+
)
600+
} else {
601+
let Some(size) = index.get_associated_type(member_type_name)?.size_of() else {
602+
// XXX: can this still fail at this point? might be `unreachable`
603+
return Err(Diagnostic::codegen_error(
604+
"Unable to determine type size",
605+
m.source_location.clone(),
606+
));
607+
};
608+
(size, 1)
609+
};
610+
self.llvm
611+
.builder
612+
.build_memcpy(bitcast, alignment, ptr_value.into_pointer_value(), alignment, size)
613+
.map_err(|e| Diagnostic::codegen_error(e, m.source_location.clone()))?;
614+
} else {
615+
self.llvm.builder.build_store(ptr, ptr_value);
616+
};
540617

541618
(parameter_name, ptr)
542619
} else {
@@ -753,7 +830,7 @@ impl<'ink, 'cg> PouGenerator<'ink, 'cg> {
753830
(variadic, variadic.and_then(VariableIndexEntry::get_varargs))
754831
{
755832
//Create a size parameter of type i32 (DINT)
756-
let size = self.llvm_index.find_associated_type(typesystem::DINT_TYPE).map(Into::into)?;
833+
let size = self.llvm_index.find_associated_type(DINT_TYPE).map(Into::into)?;
757834

758835
let ty = self
759836
.llvm_index

‎src/codegen/llvm_typesystem.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,7 @@ impl<'ctx, 'cast> Castable<'ctx, 'cast> for ArrayValue<'ctx> {
310310
let dimensions =
311311
dims.iter().map(|it| cast_data.llvm.i32_type().const_int(*it as u64, true)).collect::<Vec<_>>();
312312
let array_value = cast_data.llvm.i32_type().const_array(&dimensions);
313+
// FIXME: should be memcopied, but is an rvalue. can only initialize global variables with value types. any other way for alloca'd variables than using store?
313314
builder.build_store(vla_dimensions_ptr, array_value);
314315

315316
builder.build_store(vla_arr_ptr, arr_gep);

‎src/codegen/tests/initialization_test/snapshots/rusty__codegen__tests__initialization_test__pou_initializers__class_struct_initialized_in_function.snap

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,16 @@ entry:
1717
ret void
1818
}
1919

20-
define i32 @func(%fb %0) section "fn-func:i32[v]" {
20+
define i32 @func(%fb* %0) section "fn-func:i32[v]" {
2121
entry:
2222
%func = alloca i32, align 4
2323
%in = alloca %fb, align 8
24-
store %fb %0, %fb* %in, align 2
24+
%1 = bitcast %fb* %in to i8*
25+
%2 = bitcast %fb* %0 to i8*
26+
call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %1, i8* align 1 %2, i64 ptrtoint (%fb* getelementptr (%fb, %fb* null, i32 1) to i64), i1 false)
2527
%x = alloca %fb, align 8
26-
%1 = bitcast %fb* %x to i8*
27-
call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %1, i8* align 1 bitcast (%fb* @__fb__init to i8*), i64 ptrtoint (%fb* getelementptr (%fb, %fb* null, i32 1) to i64), i1 false)
28+
%3 = bitcast %fb* %x to i8*
29+
call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %3, i8* align 1 bitcast (%fb* @__fb__init to i8*), i64 ptrtoint (%fb* getelementptr (%fb, %fb* null, i32 1) to i64), i1 false)
2830
store i32 0, i32* %func, align 4
2931
%func_ret = load i32, i32* %func, align 4
3032
ret i32 %func_ret
@@ -33,8 +35,7 @@ entry:
3335
define void @main(%main* %0) section "fn-main:v" {
3436
entry:
3537
%fb0 = getelementptr inbounds %main, %main* %0, i32 0, i32 0
36-
%load_fb0 = load %fb, %fb* %fb0, align 2
37-
%call = call i32 @func(%fb %load_fb0)
38+
%call = call i32 @func(%fb* %fb0)
3839
ret void
3940
}
4041

‎src/codegen/tests/initialization_test/snapshots/rusty__codegen__tests__initialization_test__pou_initializers__function_block_struct_initialized_in_function.snap

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,16 @@ entry:
1616
ret void
1717
}
1818

19-
define i32 @func(%fb %0) section "fn-func:i32[v]" {
19+
define i32 @func(%fb* %0) section "fn-func:i32[v]" {
2020
entry:
2121
%func = alloca i32, align 4
2222
%in = alloca %fb, align 8
23-
store %fb %0, %fb* %in, align 1
23+
%1 = bitcast %fb* %in to i8*
24+
%2 = bitcast %fb* %0 to i8*
25+
call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %1, i8* align 1 %2, i64 ptrtoint (%fb* getelementptr (%fb, %fb* null, i32 1) to i64), i1 false)
2426
%x = alloca %fb, align 8
25-
%1 = bitcast %fb* %x to i8*
26-
call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %1, i8* align 1 bitcast (%fb* @__fb__init to i8*), i64 ptrtoint (%fb* getelementptr (%fb, %fb* null, i32 1) to i64), i1 false)
27+
%3 = bitcast %fb* %x to i8*
28+
call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %3, i8* align 1 bitcast (%fb* @__fb__init to i8*), i64 ptrtoint (%fb* getelementptr (%fb, %fb* null, i32 1) to i64), i1 false)
2729
store i32 0, i32* %func, align 4
2830
%func_ret = load i32, i32* %func, align 4
2931
ret i32 %func_ret
@@ -32,8 +34,7 @@ entry:
3234
define void @main(%main* %0) section "fn-main:v" {
3335
entry:
3436
%fb0 = getelementptr inbounds %main, %main* %0, i32 0, i32 0
35-
%load_fb0 = load %fb, %fb* %fb0, align 1
36-
%call = call i32 @func(%fb %load_fb0)
37+
%call = call i32 @func(%fb* %fb0)
3738
ret void
3839
}
3940

0 commit comments

Comments
 (0)