fix: aggregate output assignments (PLC-lang#1234)

mhasel · web-flow · commit 96d04923a5bd · 2024-06-04T17:26:55.000+02:00
* fix: aggregate output assignments

Renames `generate_string_store` to `build_memcpy` and adapts the logic to work for all aggregate types, not just strings.
diff --git a/src/codegen/generators/expression_generator.rs b/src/codegen/generators/expression_generator.rs
@@ -572,12 +572,8 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
                     let output_value_type =
                         self.index.get_type_information_or_void(parameter.get_type_name());
 
-                    //Special string handling
-                    if (assigned_output_type.is_string() && output_value_type.is_string())
-                        || (assigned_output_type.is_struct() && output_value_type.is_struct())
-                        || (assigned_output_type.is_array() && output_value_type.is_array())
-                    {
-                        self.generate_string_store(
+                    if assigned_output_type.is_aggregate() && output_value_type.is_aggregate() {
+                        self.build_memcpy(
                             assigned_output,
                             assigned_output_type,
                             expression.get_location(),
@@ -2185,55 +2181,26 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
         let right_type =
             self.annotations.get_type_or_void(right_statement, self.index).get_type_information();
 
-        //Special string handling
-        if left_type.is_string() && right_type.is_string()
-        //string-literals are also generated as global constant variables so we can always assume that
-        //we have a pointer-value
-        {
+        // redirect aggregate types
+        if left_type.is_aggregate() && right_type.is_aggregate() {
             let right =
-                self.generate_expression_value(right_statement).map(|expr_value| match expr_value {
-                    ExpressionValue::LValue(ptr) => ptr,
-                    ExpressionValue::RValue(_) => unreachable!(
-                        "strings should be lvalues: {:?}, {:?}",
-                        right_statement.get_location(),
-                        right_statement
-                    ),
-                })?;
-            self.generate_string_store(
+                self.generate_expression_value(right_statement)?.get_basic_value_enum().into_pointer_value();
+            self.build_memcpy(
                 left,
                 left_type,
                 right_statement.get_location(),
                 right,
                 right_type,
                 right_statement.get_location(),
             )?;
-        } else if (left_type.is_struct() && right_type.is_struct())
-            || (left_type.is_array() && right_type.is_array())
-        {
-            //memcopy right_statement into left
-            let expression =
-                self.generate_expression_value(right_statement)?.get_basic_value_enum().into_pointer_value();
-
-            let size =
-                self.llvm_index.get_associated_type(right_type.get_name())?.size_of().ok_or_else(|| {
-                    Diagnostic::codegen_error(
-                        format!("Unknown size of type {}.", right_type.get_name()).as_str(),
-                        right_statement.get_location(),
-                    )
-                })?;
-
-            self.llvm
-                .builder
-                .build_memcpy(left, 1, expression, 1, size)
-                .map_err(|err| Diagnostic::codegen_error(err, right_statement.get_location()))?;
         } else {
             let expression = self.generate_expression(right_statement)?;
             self.llvm.builder.build_store(left, expression);
         }
         Ok(())
     }
 
-    pub fn generate_string_store(
+    fn build_memcpy(
         &self,
         left: inkwell::values::PointerValue<'ink>,
         left_type: &DataTypeInformation,
@@ -2242,35 +2209,45 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
         right_type: &DataTypeInformation,
         right_location: SourceLocation,
     ) -> Result<PointerValue<'ink>, Diagnostic> {
-        let target_size = self.get_string_size(left_type, left_location.clone())?;
-        let value_size = self.get_string_size(right_type, right_location)?;
-        let size = std::cmp::min(target_size - 1, value_size);
-        let align_left = left_type.get_string_character_width(self.index).value();
-        let align_right = right_type.get_string_character_width(self.index).value();
-        //Multiply by the string alignment to copy enough for widestrings
-        //This is done at compile time to avoid generating an extra mul
-        let size = self.llvm.context.i32_type().const_int((size * align_left as i64) as u64, true);
+        let (size, alignment) = match (left_type, right_type) {
+            (
+                DataTypeInformation::String { size: lsize, .. },
+                DataTypeInformation::String { size: rsize, .. },
+            ) => {
+                let target_size = lsize
+                    .as_int_value(self.index)
+                    .map_err(|err| Diagnostic::codegen_error(err.as_str(), left_location.clone()))?;
+                let value_size = rsize
+                    .as_int_value(self.index)
+                    .map_err(|err| Diagnostic::codegen_error(err.as_str(), right_location))?;
+                let size = std::cmp::min(target_size - 1, value_size);
+                let alignment = left_type.get_string_character_width(self.index).value();
+                //Multiply by the string alignment to copy enough for widestrings
+                //This is done at compile time to avoid generating an extra mul
+                let size = self.llvm.context.i32_type().const_int((size * alignment as i64) as u64, true);
+                (size, alignment)
+            }
+            (DataTypeInformation::Array { .. }, DataTypeInformation::Array { .. })
+            | (DataTypeInformation::Struct { .. }, DataTypeInformation::Struct { .. }) => {
+                let size = self.llvm_index.get_associated_type(right_type.get_name())?.size_of().ok_or_else(
+                    || {
+                        Diagnostic::codegen_error(
+                            format!("Unknown size of type {}.", right_type.get_name()).as_str(),
+                            right_location,
+                        )
+                    },
+                )?;
+                (size, 1)
+            }
+            _ => unreachable!("memcpy is not used for non-aggregate types"),
+        };
+
         self.llvm
             .builder
-            .build_memcpy(left, align_left, right, align_right, size)
+            .build_memcpy(left, alignment, right, alignment, size)
             .map_err(|err| Diagnostic::codegen_error(err, left_location))
     }
 
-    fn get_string_size(
-        &self,
-        datatype: &DataTypeInformation,
-        location: SourceLocation,
-    ) -> Result<i64, Diagnostic> {
-        if let DataTypeInformation::String { size, .. } = datatype {
-            size.as_int_value(self.index).map_err(|err| Diagnostic::codegen_error(err.as_str(), location))
-        } else {
-            Err(Diagnostic::codegen_error(
-                format!("{} is not a String", datatype.get_name()).as_str(),
-                location,
-            ))
-        }
-    }
-
     /// returns an optional name used for a temporary variable when loading a pointer represented by `expression`
     fn get_load_name(&self, expression: &AstNode) -> Option<String> {
         match expression.get_stmt() {
diff --git a/src/codegen/tests/parameters_tests.rs b/src/codegen/tests/parameters_tests.rs
@@ -1079,3 +1079,100 @@ fn by_value_fb_arg_aggregates_are_memcopied() {
     attributes #1 = { argmemonly nofree nounwind willreturn }
     "###);
 }
+
+#[test]
+fn var_output_aggregate_types_are_memcopied() {
+    let result = codegen(
+        r#"
+        TYPE OUT_TYPE : STRUCT 
+            a : BYTE;
+        END_STRUCT;
+        END_TYPE
+
+        FUNCTION_BLOCK FB
+        VAR_OUTPUT 
+            output : OUT_TYPE;
+            output2 : ARRAY[0..10] OF DINT;
+            output3 : ARRAY[0..10] OF OUT_TYPE;
+            output4 : STRING;
+            output5 : WSTRING;
+        END_VAR    
+        END_FUNCTION_BLOCK
+
+        PROGRAM PRG
+        VAR
+            out: OUT_TYPE;
+            out2 : ARRAY[0..10] OF DINT;
+            out3 : ARRAY[0..10] OF OUT_TYPE;
+            out4 : STRING;
+            out5 : WSTRING;
+            station: FB;
+        END_VAR
+            station(output => out, output2 => out2, output3 => out3, output4 => out4, output5 => out5);
+        END_PROGRAM
+        "#,
+    );
+
+    assert_snapshot!(result, @r###"
+    ; ModuleID = 'main'
+    source_filename = "main"
+
+    %FB = type { %OUT_TYPE, [11 x i32], [11 x %OUT_TYPE], [81 x i8], [81 x i16] }
+    %OUT_TYPE = type { i8 }
+    %PRG = type { %OUT_TYPE, [11 x i32], [11 x %OUT_TYPE], [81 x i8], [81 x i16], %FB }
+
+    @__FB__init = unnamed_addr constant %FB zeroinitializer
+    @__OUT_TYPE__init = unnamed_addr constant %OUT_TYPE zeroinitializer
+    @PRG_instance = global %PRG zeroinitializer
+
+    define void @FB(%FB* %0) section "fn-FB:v[v][v][v][s8u81][s16u81]" {
+    entry:
+      %output = getelementptr inbounds %FB, %FB* %0, i32 0, i32 0
+      %output2 = getelementptr inbounds %FB, %FB* %0, i32 0, i32 1
+      %output3 = getelementptr inbounds %FB, %FB* %0, i32 0, i32 2
+      %output4 = getelementptr inbounds %FB, %FB* %0, i32 0, i32 3
+      %output5 = getelementptr inbounds %FB, %FB* %0, i32 0, i32 4
+      ret void
+    }
+
+    define void @PRG(%PRG* %0) section "fn-PRG:v" {
+    entry:
+      %out = getelementptr inbounds %PRG, %PRG* %0, i32 0, i32 0
+      %out2 = getelementptr inbounds %PRG, %PRG* %0, i32 0, i32 1
+      %out3 = getelementptr inbounds %PRG, %PRG* %0, i32 0, i32 2
+      %out4 = getelementptr inbounds %PRG, %PRG* %0, i32 0, i32 3
+      %out5 = getelementptr inbounds %PRG, %PRG* %0, i32 0, i32 4
+      %station = getelementptr inbounds %PRG, %PRG* %0, i32 0, i32 5
+      call void @FB(%FB* %station)
+      %1 = getelementptr inbounds %FB, %FB* %station, i32 0, i32 0
+      %2 = bitcast %OUT_TYPE* %out to i8*
+      %3 = bitcast %OUT_TYPE* %1 to i8*
+      call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %2, i8* align 1 %3, i64 ptrtoint (%OUT_TYPE* getelementptr (%OUT_TYPE, %OUT_TYPE* null, i32 1) to i64), i1 false)
+      %4 = getelementptr inbounds %FB, %FB* %station, i32 0, i32 1
+      %5 = bitcast [11 x i32]* %out2 to i8*
+      %6 = bitcast [11 x i32]* %4 to i8*
+      call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %5, i8* align 1 %6, i64 ptrtoint ([11 x i32]* getelementptr ([11 x i32], [11 x i32]* null, i32 1) to i64), i1 false)
+      %7 = getelementptr inbounds %FB, %FB* %station, i32 0, i32 2
+      %8 = bitcast [11 x %OUT_TYPE]* %out3 to i8*
+      %9 = bitcast [11 x %OUT_TYPE]* %7 to i8*
+      call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %8, i8* align 1 %9, i64 ptrtoint ([11 x %OUT_TYPE]* getelementptr ([11 x %OUT_TYPE], [11 x %OUT_TYPE]* null, i32 1) to i64), i1 false)
+      %10 = getelementptr inbounds %FB, %FB* %station, i32 0, i32 3
+      %11 = bitcast [81 x i8]* %out4 to i8*
+      %12 = bitcast [81 x i8]* %10 to i8*
+      call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 1 %11, i8* align 1 %12, i32 80, i1 false)
+      %13 = getelementptr inbounds %FB, %FB* %station, i32 0, i32 4
+      %14 = bitcast [81 x i16]* %out5 to i8*
+      %15 = bitcast [81 x i16]* %13 to i8*
+      call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 2 %14, i8* align 2 %15, i32 160, i1 false)
+      ret void
+    }
+
+    ; Function Attrs: argmemonly nofree nounwind willreturn
+    declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg) #0
+
+    ; Function Attrs: argmemonly nofree nounwind willreturn
+    declare void @llvm.memcpy.p0i8.p0i8.i32(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i32, i1 immarg) #0
+
+    attributes #0 = { argmemonly nofree nounwind willreturn }
+    "###);
+}
diff --git a/tests/correctness/functions.rs b/tests/correctness/functions.rs
@@ -1,3 +1,5 @@
+use std::ffi::CStr;
+
 use rusty::codegen::CodegenContext;
 
 // Copyright (c) 2020 Ghaith Hachem and Mathias Rieder
@@ -642,6 +644,41 @@ fn optional_output_assignment_in_functions() {
     assert_eq!(2, interface.var2);
 }
 
+#[test]
+fn aggregate_var_output_assignment() {
+    #[repr(C)]
+    struct MainType {
+        var1: [u8; 81],
+        var2: [i32; 4],
+    }
+
+    let main = r#"
+        PROGRAM foo
+        VAR_OUTPUT
+            output1 : STRING;
+            output2 : ARRAY[0..3] OF DINT;
+        END_VAR
+            output1 := 'Hello, world!';
+            output2 := [5, 7, 11, 13];
+        END_PROGRAM
+
+        PROGRAM main
+        VAR
+            var1 : STRING;
+            var2 : ARRAY[0..3] OF DINT;
+        END_VAR
+            foo(output1 => var1, output2 => var2);
+        END_PROGRAM
+    "#;
+
+    let mut interface = MainType { var1: [0; 81], var2: [0; 4] };
+    let _: i32 = compile_and_run(main.to_string(), &mut interface);
+
+    let str = CStr::from_bytes_until_nul(&interface.var1).unwrap().to_string_lossy();
+    assert_eq!("Hello, world!", str);
+    assert_eq!([5, 7, 11, 13], interface.var2);
+}
+
 #[test]
 fn direct_call_on_function_block_array_access() {
     #[allow(dead_code)]
@@ -872,12 +909,12 @@ fn mux_struct_ref() {
     #[repr(C)]
     #[derive(Default)]
     struct MainType {
-        res: myStruct,
+        res: MyStruct,
     }
 
     #[repr(C)]
     #[derive(Default)]
-    struct myStruct {
+    struct MyStruct {
         a: bool,
         b: bool,
     }
@@ -1026,12 +1063,12 @@ fn sel_struct_ref() {
     #[repr(C)]
     #[derive(Default)]
     struct MainType {
-        res: myStruct,
+        res: MyStruct,
     }
 
     #[repr(C)]
     #[derive(Default)]
-    struct myStruct {
+    struct MyStruct {
         a: bool,
         b: bool,
     }
