diff --git a/src/codegen/generators/llvm.rs b/src/codegen/generators/llvm.rs index ea4ea8800d7..a31539dba3e 100644 --- a/src/codegen/generators/llvm.rs +++ b/src/codegen/generators/llvm.rs @@ -10,7 +10,9 @@ use inkwell::{ context::Context, module::{Linkage, Module}, types::{BasicTypeEnum, StringRadix}, - values::{AnyValue, AsValueRef, BasicValue, BasicValueEnum, GlobalValue, IntValue, PointerValue}, + values::{ + AnyValue, AsValueRef, BasicValue, BasicValueEnum, FunctionValue, GlobalValue, IntValue, PointerValue, + }, AddressSpace, }; use plc_ast::ast::{AstNode, AstStatement}; @@ -149,6 +151,47 @@ impl<'a> Llvm<'a> { self.builder.build_alloca(*data_type, name).map_err(Into::into) } + /// creates a local variable in the function's entry block + /// + /// This is used for lowered local declarations that may syntactically appear inside loops. + /// The storage must still live for the whole function invocation, while any initialization + /// remains at the original insertion point. + pub fn create_entry_local_variable( + &self, + function: FunctionValue<'a>, + name: &str, + data_type: &BasicTypeEnum<'a>, + ) -> Result, CodegenError> { + // Hoist storage into the function entry block so lowered loop temporaries + // are allocated once per function invocation instead of once per iteration. + let entry = function.get_first_basic_block().ok_or_else(|| { + CodegenError::new( + "Cannot create entry local variable without an entry block", + SourceLocation::internal(), + ) + })?; + + // Use a dedicated builder so we can place the `alloca` in `entry` without + // disturbing the caller's current insertion point. + let builder = self.context.create_builder(); + if let Some(last_instruction) = entry.get_last_instruction() { + if last_instruction.is_terminator() { + // Entry already ends in a terminator, so insert the `alloca` right before it. + builder.position_before(&last_instruction); + } else { + // Otherwise append at the end of the entry block. + builder.position_at_end(entry); + } + } else { + // Empty entry block: place the builder at its end and emit the `alloca` there. + builder.position_at_end(entry); + } + + // Only the stack slot is hoisted; any initialization still happens at the + // original statement location chosen by the caller. + builder.build_alloca(*data_type, name).map_err(Into::into) + } + /// sets a const-zero initializer for the given global_value according to the given type /// sets a const_zero initializer if the given variable_type is either an int_type or a struct_type /// diff --git a/src/codegen/generators/statement_generator.rs b/src/codegen/generators/statement_generator.rs index 40b5c59442a..bda95783af3 100644 --- a/src/codegen/generators/statement_generator.rs +++ b/src/codegen/generators/statement_generator.rs @@ -225,7 +225,8 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> { AstStatement::AllocationStatement(Allocation { name, reference_type }) => { let ty = llvm_index.find_associated_type(reference_type).expect("Type must exist at this point"); - let value = self.llvm.builder.build_alloca(ty, name)?; + let value = + self.llvm.create_entry_local_variable(self.function_context.function, name, &ty)?; self.llvm.generate_variable_initializer( &llvm_index, self.index, diff --git a/src/codegen/tests/code_gen_tests.rs b/src/codegen/tests/code_gen_tests.rs index 5d750d48be4..3ad6ca2cdb4 100644 --- a/src/codegen/tests/code_gen_tests.rs +++ b/src/codegen/tests/code_gen_tests.rs @@ -884,6 +884,174 @@ fn for_statement_with_exit() { filtered_assert_snapshot!(result); } +#[test] +fn nested_loop_temporaries_are_allocated_in_entry_block() { + let result = codegen( + r#" + FUNCTION main : DINT + VAR + i, j : DINT; + END_VAR + + FOR i := 1 TO 10 DO + FOR j := 1 TO 1 DO + END_FOR + END_FOR + END_FUNCTION + "#, + ); + + let result = result + .lines() + .take_while(|line| !line.trim_start().starts_with("condition_check:")) + .collect::>() + .join("\n"); + + // Nested FOR loops become nested WHILE TRUE loops once desugared, with temporary alloca locals like + // `ran_once_*` and `is_incrementing_*`. If their `alloca`s stay in the loop body, the inner temporaries + // are reallocated on each iteration. Codegen must hoist those stack allocations to the entry block. + filtered_assert_snapshot!(result, @r#" + ; ModuleID = '' + source_filename = "" + target datalayout = "[filtered]" + target triple = "[filtered]" + + define i32 @main() { + entry: + %main = alloca i32, align [filtered] + %i = alloca i32, align [filtered] + %j = alloca i32, align [filtered] + store i32 0, ptr %i, align [filtered] + store i32 0, ptr %j, align [filtered] + store i32 0, ptr %main, align [filtered] + %ran_once_1 = alloca i8, align [filtered] + store i8 0, ptr %ran_once_1, align [filtered] + %is_incrementing_1 = alloca i8, align [filtered] + store i8 0, ptr %is_incrementing_1, align [filtered] + store i32 1, ptr %i, align [filtered] + store i8 1, ptr %is_incrementing_1, align [filtered] + %ran_once_0 = alloca i8, align [filtered] + %is_incrementing_0 = alloca i8, align [filtered] + br label %while_body + + while_body: ; preds = %continue14, %entry + %load_ran_once_1 = load i8, ptr %ran_once_1, align [filtered] + %0 = icmp ne i8 %load_ran_once_1, 0 + br i1 %0, label %condition_body, label %continue1 + + continue: ; preds = %condition_body11, %condition_body7 + %main_ret = load i32, ptr %main, align [filtered] + ret i32 %main_ret + + condition_body: ; preds = %while_body + %load_i = load i32, ptr %i, align [filtered] + %tmpVar = add i32 %load_i, 1 + store i32 %tmpVar, ptr %i, align [filtered] + br label %continue1 + + continue1: ; preds = %condition_body, %while_body + store i8 1, ptr %ran_once_1, align [filtered] + %load_is_incrementing_1 = load i8, ptr %is_incrementing_1, align [filtered] + %1 = icmp ne i8 %load_is_incrementing_1, 0 + br i1 %1, label %condition_body3, label %else + + condition_body3: ; preds = %continue1 + %load_i5 = load i32, ptr %i, align [filtered] + %tmpVar6 = icmp sgt i32 %load_i5, 10 + %2 = zext i1 %tmpVar6 to i8 + %3 = icmp ne i8 %2, 0 + br i1 %3, label %condition_body7, label %continue4 + + else: ; preds = %continue1 + %load_i9 = load i32, ptr %i, align [filtered] + %tmpVar10 = icmp slt i32 %load_i9, 10 + %4 = zext i1 %tmpVar10 to i8 + %5 = icmp ne i8 %4, 0 + br i1 %5, label %condition_body11, label %continue8 + + continue2: ; preds = %continue8, %continue4 + store i8 0, ptr %ran_once_0, align [filtered] + store i8 0, ptr %is_incrementing_0, align [filtered] + store i32 1, ptr %j, align [filtered] + store i8 1, ptr %is_incrementing_0, align [filtered] + br label %while_body13 + + condition_body7: ; preds = %condition_body3 + br label %continue + + buffer_block: ; No predecessors! + br label %continue4 + + continue4: ; preds = %buffer_block, %condition_body3 + br label %continue2 + + condition_body11: ; preds = %else + br label %continue + + buffer_block12: ; No predecessors! + br label %continue8 + + continue8: ; preds = %buffer_block12, %else + br label %continue2 + + while_body13: ; preds = %continue19, %continue2 + %load_ran_once_0 = load i8, ptr %ran_once_0, align [filtered] + %6 = icmp ne i8 %load_ran_once_0, 0 + br i1 %6, label %condition_body16, label %continue15 + + continue14: ; preds = %condition_body29, %condition_body24 + br label %while_body + + condition_body16: ; preds = %while_body13 + %load_j = load i32, ptr %j, align [filtered] + %tmpVar17 = add i32 %load_j, 1 + store i32 %tmpVar17, ptr %j, align [filtered] + br label %continue15 + + continue15: ; preds = %condition_body16, %while_body13 + store i8 1, ptr %ran_once_0, align [filtered] + %load_is_incrementing_0 = load i8, ptr %is_incrementing_0, align [filtered] + %7 = icmp ne i8 %load_is_incrementing_0, 0 + br i1 %7, label %condition_body20, label %else18 + + condition_body20: ; preds = %continue15 + %load_j22 = load i32, ptr %j, align [filtered] + %tmpVar23 = icmp sgt i32 %load_j22, 1 + %8 = zext i1 %tmpVar23 to i8 + %9 = icmp ne i8 %8, 0 + br i1 %9, label %condition_body24, label %continue21 + + else18: ; preds = %continue15 + %load_j27 = load i32, ptr %j, align [filtered] + %tmpVar28 = icmp slt i32 %load_j27, 1 + %10 = zext i1 %tmpVar28 to i8 + %11 = icmp ne i8 %10, 0 + br i1 %11, label %condition_body29, label %continue26 + + continue19: ; preds = %continue26, %continue21 + br label %while_body13 + + condition_body24: ; preds = %condition_body20 + br label %continue14 + + buffer_block25: ; No predecessors! + br label %continue21 + + continue21: ; preds = %buffer_block25, %condition_body20 + br label %continue19 + + condition_body29: ; preds = %else18 + br label %continue14 + + buffer_block30: ; No predecessors! + br label %continue26 + + continue26: ; preds = %buffer_block30, %else18 + br label %continue19 + } + "#); +} + #[test] fn class_method_in_pou() { let result = codegen( diff --git a/src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__for_conditions_location_marked.snap b/src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__for_conditions_location_marked.snap index b1e4806471b..8370554065a 100644 --- a/src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__for_conditions_location_marked.snap +++ b/src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__for_conditions_location_marked.snap @@ -12,9 +12,9 @@ entry: %myFunc = alloca i32, align [filtered] #dbg_declare(ptr %myFunc, !9, !DIExpression(), !11) store i32 0, ptr %myFunc, align [filtered] - %ran_once_0 = alloca i8, align [filtered], !dbg !12 + %ran_once_0 = alloca i8, align [filtered] store i8 0, ptr %ran_once_0, align [filtered], !dbg !12 - %is_incrementing_0 = alloca i8, align [filtered], !dbg !12 + %is_incrementing_0 = alloca i8, align [filtered] store i8 0, ptr %is_incrementing_0, align [filtered], !dbg !12 store i32 1, ptr %myFunc, align [filtered], !dbg !13 store i8 1, ptr %is_incrementing_0, align [filtered], !dbg !12 diff --git a/src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__repeat_conditions_location_marked.snap b/src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__repeat_conditions_location_marked.snap index c8e5678f40b..d5c5b8b9166 100644 --- a/src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__repeat_conditions_location_marked.snap +++ b/src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__repeat_conditions_location_marked.snap @@ -12,7 +12,7 @@ entry: %myFunc = alloca i32, align [filtered] #dbg_declare(ptr %myFunc, !9, !DIExpression(), !11) store i32 0, ptr %myFunc, align [filtered] - %ran_once_0 = alloca i8, align [filtered], !dbg !12 + %ran_once_0 = alloca i8, align [filtered] store i8 0, ptr %ran_once_0, align [filtered], !dbg !12 br label %while_body, !dbg !12 diff --git a/src/codegen/tests/oop_tests/super_tests.rs b/src/codegen/tests/oop_tests/super_tests.rs index 5f17843c0df..ff4e995b8c0 100644 --- a/src/codegen/tests/oop_tests/super_tests.rs +++ b/src/codegen/tests/oop_tests/super_tests.rs @@ -300,7 +300,7 @@ fn super_in_method_calls() { VAR value : INT := 10; END_VAR - + METHOD process : INT process := value * 2; END_METHOD @@ -310,7 +310,7 @@ fn super_in_method_calls() { METHOD process : INT // Override parent's method process := value + 5; END_METHOD - + METHOD test : INT // Call parent's implementation test := SUPER^.process(); @@ -860,7 +860,7 @@ fn super_in_multi_level_inheritance() { VAR g_val : INT := 10; END_VAR - + METHOD gp_method : INT gp_method := g_val; END_METHOD @@ -870,7 +870,7 @@ fn super_in_multi_level_inheritance() { VAR p_val : INT := 20; END_VAR - + METHOD p_method : INT p_method := p_val + SUPER^.gp_method(); END_METHOD @@ -880,7 +880,7 @@ fn super_in_multi_level_inheritance() { VAR c_val : INT := 30; END_VAR - + METHOD test : INT // Access parent's method which itself uses SUPER^ test := SUPER^.p_method(); @@ -1373,7 +1373,7 @@ fn super_in_conditionals() { ELSE SUPER^.value := 100; END_IF; - + // In CASE statement CASE SUPER^.value OF 10: SUPER^.threshold := 40; @@ -1732,14 +1732,14 @@ fn super_as_function_parameter() { process_val(SUPER^); END_METHOD END_FUNCTION_BLOCK - + FUNCTION process_ref : INT VAR_INPUT ref : REF_TO parent; END_VAR ref^.val := 20; END_FUNCTION - + FUNCTION process_val : INT VAR_INPUT val : parent; @@ -1939,7 +1939,7 @@ fn super_with_deeply_nested_expressions() { b : INT := 2; c : INT := 3; END_VAR - + METHOD calc : INT calc := a + b * c; END_METHOD @@ -2186,7 +2186,7 @@ fn super_in_loop_constructs() { counter : INT := 0; arr : ARRAY[0..5] OF INT := [1,2,3,4,5,6]; END_VAR - + METHOD increment counter := counter + 1; END_METHOD @@ -2198,18 +2198,18 @@ fn super_in_loop_constructs() { i : INT; sum : INT := 0; END_VAR - + // FOR loop with SUPER^ FOR i := 0 TO 5 BY 1 DO sum := sum + SUPER^.arr[i]; SUPER^.increment(); END_FOR; - + // WHILE loop with SUPER^ WHILE SUPER^.counter < 10 DO SUPER^.increment(); END_WHILE; - + // REPEAT loop with SUPER^ REPEAT SUPER^.counter := SUPER^.counter - 1; @@ -2285,6 +2285,7 @@ fn super_in_loop_constructs() { store i8 0, ptr %is_incrementing_1, align [filtered] store i16 0, ptr %i, align [filtered] store i8 1, ptr %is_incrementing_1, align [filtered] + %ran_once_0 = alloca i8, align [filtered] br label %while_body while_body: ; preds = %continue2, %entry @@ -2371,7 +2372,6 @@ fn super_in_loop_constructs() { br i1 %tmpVar22, label %condition_body23, label %continue20 continue19: ; preds = %condition_body23 - %ran_once_0 = alloca i8, align [filtered] store i8 0, ptr %ran_once_0, align [filtered] br label %while_body25 @@ -2853,11 +2853,11 @@ fn super_with_return_value_in_multiple_contexts() { VAR value : INT := 10; END_VAR - + METHOD get_value : INT get_value := value; END_METHOD - + METHOD get_ref : REF_TO parent get_ref := THIS; END_METHOD @@ -2868,12 +2868,12 @@ fn super_with_return_value_in_multiple_contexts() { // Return value directly from SUPER^ call test_value := SUPER^.get_value(); END_METHOD - + METHOD test_ref : REF_TO parent // Return REF_TO parent from SUPER test_ref := SUPER; END_METHOD - + METHOD test_mixed : INT // Use SUPER in complex return expression test_mixed := SUPER^.get_value() + SUPER^.get_ref()^.value; @@ -2908,15 +2908,15 @@ fn super_with_structured_types() { VAR local_data : Complex_Type; END_VAR - + // Access structured type through SUPER^ local_data.x := SUPER^.data.x; local_data.y := SUPER^.data.y; local_data.z := SUPER^.data.z; - + // Access structured array through SUPER^ SUPER^.arr_data[0].x := SUPER^.arr_data[1].x; - + // Nested access SUPER^.arr_data[0].z := SUPER^.data.z; END_METHOD @@ -3157,7 +3157,7 @@ fn super_in_action_blocks() { VAR value : INT := 10; END_VAR - + METHOD increment value := value + 1; END_METHOD @@ -3165,7 +3165,7 @@ fn super_in_action_blocks() { FUNCTION_BLOCK child EXTENDS parent END_FUNCTION_BLOCK - + ACTION child.increase // Using SUPER^ inside an ACTION block SUPER^.value := SUPER^.value + 5; diff --git a/tests/lit/correctness/control/for/nested_for_loop_allocas_are_hoisted.st b/tests/lit/correctness/control/for/nested_for_loop_allocas_are_hoisted.st new file mode 100644 index 00000000000..1156fe9f666 --- /dev/null +++ b/tests/lit/correctness/control/for/nested_for_loop_allocas_are_hoisted.st @@ -0,0 +1,43 @@ +// RUN: (%COMPILE -Onone %s && %RUN) | %CHECK %s +// +// Regression test for stack growth caused by lowered loop temporaries. +// Nested FOR loops are desugared into nested WHILE TRUE loops with synthetic +// locals, e.g. roughly: +// +// alloca ran_once_1: BOOL +// alloca is_incrementing_1: BOOL +// i := 1 +// is_incrementing_1 := TRUE +// WHILE TRUE DO +// ... +// alloca ran_once_0: BOOL +// alloca is_incrementing_0: BOOL +// j := 1 +// is_incrementing_0 := TRUE +// WHILE TRUE DO +// ... +// END_WHILE +// END_WHILE +// +// If those `alloca` statements are emitted in-place, the inner loop temps are +// allocated once per outer-loop iteration and the stack eventually overflows at +// `-Onone`. Codegen must hoist the actual stack slots to the function entry +// block and only keep the initialization stores in place. + +FUNCTION main : DINT + VAR + i, j : DINT; + counter : DINT; + END_VAR + + counter := 0; + + FOR i := 1 TO 1000000 DO + FOR j := 1 TO 1 DO + counter := counter + 1; + END_FOR + END_FOR + + printf('counter=%d$N', counter); // CHECK: counter=1000000 + main := 0; +END_FUNCTION