fix: for loop predicate

This PR fixes for loops executing once when the predicate already should not be met for decrementing loops.
I have also re-implemented the codegen logic for for-loops, resulting in fewer predecessors and hopefully more
readable IR.

Resolves #1207

Author: mhasel

src/codegen/generators/statement_generator.rs

@@ -14,7 +14,7 @@ use inkwell::{
     basic_block::BasicBlock,
     builder::Builder,
     context::Context,
-    values::{BasicValueEnum, FunctionValue, PointerValue},
+    values::{FunctionValue, PointerValue},
 };
 use plc_ast::{
     ast::{
@@ -389,115 +389,67 @@ impl<'a, 'b> StatementCodeGenerator<'a, 'b> {
         body: &[AstNode],
     ) -> Result<(), Diagnostic> {
         let (builder, current_function, context) = self.get_llvm_deps();
-        self.generate_assignment_statement(counter, start)?;
-        let condition_check = context.append_basic_block(current_function, "condition_check");
-        let for_body = context.append_basic_block(current_function, "for_body");
-        let increment_block = context.append_basic_block(current_function, "increment");
-        let continue_block = context.append_basic_block(current_function, "continue");
-
-        //Generate an initial jump to the for condition
-        builder.build_unconditional_branch(condition_check);
-
-        //Check loop condition
-        builder.position_at_end(condition_check);
         let exp_gen = self.create_expr_generator();
-        let counter_statement = exp_gen.generate_expression(counter)?;
-
-        //.                                                           /            and_2                \
-        //.                  /             and 1               \
-        //.                   (counter_end_le && counter_start_ge) || (counter_end_ge && counter_start_le)
-        let or_eval = self.generate_compare_expression(counter, end, start, &exp_gen)?;
-
-        builder.build_conditional_branch(to_i1(or_eval.into_int_value(), builder), for_body, continue_block);
+        self.generate_assignment_statement(counter, start)?;
+        let predicate_incrementing = context.append_basic_block(current_function, "predicate_inc");
+        let predicate_decrementing = context.append_basic_block(current_function, "predicate_dec");
+        let loop_body = context.append_basic_block(current_function, "loop");
+        let afterloop = context.append_basic_block(current_function, "continue");
 
-        //Enter the for loop
-        builder.position_at_end(for_body);
-        let body_generator = StatementCodeGenerator {
-            current_loop_exit: Some(continue_block),
-            current_loop_continue: Some(increment_block),
-            load_prefix: self.load_prefix.clone(),
-            load_suffix: self.load_suffix.clone(),
-            ..*self
-        };
-        body_generator.generate_body(body)?;
-        builder.build_unconditional_branch(increment_block);
+        let counter = exp_gen.generate_lvalue(counter)?;
+        let end = exp_gen.generate_expression(end)?;
+        let counter_value = builder.build_load(counter, "");
 
-        //Increment
-        builder.position_at_end(increment_block);
-        let expression_generator = self.create_expr_generator();
-        let step_by_value = by_step.as_ref().map_or_else(
-            || {
-                self.llvm.create_const_numeric(
-                    &counter_statement.get_type(),
-                    "1",
-                    SourceLocation::undefined(),
-                )
-            },
+        let by_step = by_step.as_ref().map_or_else(
+            || self.llvm.create_const_numeric(&counter_value.get_type(), "1", SourceLocation::undefined()),
             |step| {
                 self.register_debug_location(step);
-                expression_generator.generate_expression(step)
+                exp_gen.generate_expression(step)
             },
         )?;
-
-        let next = builder.build_int_add(
-            counter_statement.into_int_value(),
-            step_by_value.into_int_value(),
-            "tmpVar",
+        let is_incrementing = builder.build_int_compare(
+            inkwell::IntPredicate::SGT,
+            counter_value.into_int_value(),
+            self.llvm.i32_type().const_zero(),
+            "is_incrementing",
         );
 
-        let ptr = expression_generator.generate_lvalue(counter)?;
-        builder.build_store(ptr, next);
-
-        //Loop back
-        builder.build_unconditional_branch(condition_check);
-
-        //Continue
-        builder.position_at_end(continue_block);
-
-        Ok(())
-    }
-
-    fn generate_compare_expression(
-        &'a self,
-        counter: &AstNode,
-        end: &AstNode,
-        start: &AstNode,
-        exp_gen: &'a ExpressionCodeGenerator,
-    ) -> Result<BasicValueEnum<'a>, Diagnostic> {
-        let bool_id = self.annotations.get_bool_id();
-        let counter_end_ge = AstFactory::create_binary_expression(
-            counter.clone(),
-            Operator::GreaterOrEqual,
-            end.clone(),
-            bool_id,
-        );
-        let counter_start_ge = AstFactory::create_binary_expression(
-            counter.clone(),
-            Operator::GreaterOrEqual,
-            start.clone(),
-            bool_id,
+        // --check loop predicate--
+        builder.build_conditional_branch(is_incrementing, predicate_incrementing, predicate_decrementing);
+        // --incrementing loops--
+        builder.position_at_end(predicate_incrementing);
+        let value = builder.build_load(counter, "");
+        let inc_cmp = builder.build_int_compare(
+            inkwell::IntPredicate::SLE,
+            value.into_int_value(),
+            end.into_int_value(),
+            "condition",
         );
-        let counter_end_le = AstFactory::create_binary_expression(
-            counter.clone(),
-            Operator::LessOrEqual,
-            end.clone(),
-            bool_id,
+        builder.build_conditional_branch(inc_cmp, loop_body, afterloop);
+        // --decrementing loops--
+        builder.position_at_end(predicate_decrementing);
+        let value = builder.build_load(counter, "");
+        let dec_cmp = builder.build_int_compare(
+            inkwell::IntPredicate::SGE,
+            value.into_int_value(),
+            end.into_int_value(),
+            "condition",
         );
-        let counter_start_le = AstFactory::create_binary_expression(
-            counter.clone(),
-            Operator::LessOrEqual,
-            start.clone(),
-            bool_id,
-        );
-        let and_1 =
-            AstFactory::create_binary_expression(counter_end_le, Operator::And, counter_start_ge, bool_id);
-        let and_2 =
-            AstFactory::create_binary_expression(counter_end_ge, Operator::And, counter_start_le, bool_id);
-        let or = AstFactory::create_binary_expression(and_1, Operator::Or, and_2, bool_id);
-
-        self.register_debug_location(&or);
-        let or_eval = exp_gen.generate_expression(&or)?;
-        Ok(or_eval)
+        builder.build_conditional_branch(dec_cmp, loop_body, afterloop);
+
+        // --body--
+        builder.position_at_end(loop_body);
+        self.generate_body(body)?;
+        // --increment--
+        let value = builder.build_load(counter, "");
+        let inc = builder.build_int_add(value.into_int_value(), by_step.into_int_value(), "increment");
+        builder.build_store(counter, inc);
+        //--check condition again--
+        // builder.build_phi(self.llvm.i32_type(), "phi");
+        builder.build_conditional_branch(is_incrementing, predicate_incrementing, predicate_decrementing);
+        // --continue--
+        builder.position_at_end(afterloop);
+        Ok(())
     }
 
     /// genertes a case statement

src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__for_conditions_location_marked.snap

@@ -11,67 +11,30 @@ entry:
   call void @llvm.dbg.declare(metadata i32* %myFunc, metadata !9, metadata !DIExpression()), !dbg !11
   store i32 0, i32* %myFunc, align 4, !dbg !8
   store i32 1, i32* %myFunc, align 4, !dbg !12
-  br label %condition_check, !dbg !12
-
-condition_check:                                  ; preds = %increment, %entry
-  %load_myFunc = load i32, i32* %myFunc, align 4, !dbg !12
-  %load_myFunc1 = load i32, i32* %myFunc, align 4, !dbg !12
-  %tmpVar = icmp sle i32 %load_myFunc1, 20, !dbg !12
-  %0 = zext i1 %tmpVar to i8, !dbg !12
-  %1 = icmp ne i8 %0, 0, !dbg !12
-  br i1 %1, label %2, label %5, !dbg !12
-
-for_body:                                         ; preds = %12
-  store i32 1, i32* %myFunc, align 4, !dbg !13
-  br label %increment, !dbg !13
-
-increment:                                        ; preds = %for_body
-  %tmpVar8 = add i32 %load_myFunc, 2, !dbg !14
-  store i32 %tmpVar8, i32* %myFunc, align 4, !dbg !14
-  br label %condition_check, !dbg !14
-
-continue:                                         ; preds = %12
+  %0 = load i32, i32* %myFunc, align 4, !dbg !12
+  %is_incrementing = icmp sgt i32 %0, 0, !dbg !13
+  br i1 %is_incrementing, label %predicate_inc, label %predicate_dec, !dbg !13
+
+predicate_inc:                                    ; preds = %loop, %entry
+  %1 = load i32, i32* %myFunc, align 4, !dbg !13
+  %condition = icmp sle i32 %1, 20, !dbg !13
+  br i1 %condition, label %loop, label %continue, !dbg !13
+
+predicate_dec:                                    ; preds = %loop, %entry
+  %2 = load i32, i32* %myFunc, align 4, !dbg !13
+  %condition1 = icmp sge i32 %2, 20, !dbg !13
+  br i1 %condition1, label %loop, label %continue, !dbg !13
+
+loop:                                             ; preds = %predicate_dec, %predicate_inc
+  store i32 1, i32* %myFunc, align 4, !dbg !14
+  %3 = load i32, i32* %myFunc, align 4, !dbg !14
+  %increment = add i32 %3, 2, !dbg !14
+  store i32 %increment, i32* %myFunc, align 4, !dbg !14
+  br i1 %is_incrementing, label %predicate_inc, label %predicate_dec, !dbg !14
+
+continue:                                         ; preds = %predicate_dec, %predicate_inc
   %myFunc_ret = load i32, i32* %myFunc, align 4, !dbg !14
   ret i32 %myFunc_ret, !dbg !14
-
-2:                                                ; preds = %condition_check
-  %load_myFunc2 = load i32, i32* %myFunc, align 4, !dbg !12
-  %tmpVar3 = icmp sge i32 %load_myFunc2, 1, !dbg !12
-  %3 = zext i1 %tmpVar3 to i8, !dbg !12
-  %4 = icmp ne i8 %3, 0, !dbg !12
-  br label %5, !dbg !12
-
-5:                                                ; preds = %2, %condition_check
-  %6 = phi i1 [ %1, %condition_check ], [ %4, %2 ], !dbg !12
-  %7 = zext i1 %6 to i8, !dbg !12
-  %8 = icmp ne i8 %7, 0, !dbg !12
-  br i1 %8, label %12, label %9, !dbg !12
-
-9:                                                ; preds = %5
-  %load_myFunc4 = load i32, i32* %myFunc, align 4, !dbg !12
-  %tmpVar5 = icmp sge i32 %load_myFunc4, 20, !dbg !12
-  %10 = zext i1 %tmpVar5 to i8, !dbg !12
-  %11 = icmp ne i8 %10, 0, !dbg !12
-  br i1 %11, label %16, label %19, !dbg !12
-
-12:                                               ; preds = %19, %5
-  %13 = phi i1 [ %8, %5 ], [ %22, %19 ], !dbg !12
-  %14 = zext i1 %13 to i8, !dbg !12
-  %15 = icmp ne i8 %14, 0, !dbg !12
-  br i1 %15, label %for_body, label %continue, !dbg !12
-
-16:                                               ; preds = %9
-  %load_myFunc6 = load i32, i32* %myFunc, align 4, !dbg !12
-  %tmpVar7 = icmp sle i32 %load_myFunc6, 1, !dbg !12
-  %17 = zext i1 %tmpVar7 to i8, !dbg !12
-  %18 = icmp ne i8 %17, 0, !dbg !12
-  br label %19, !dbg !12
-
-19:                                               ; preds = %16, %9
-  %20 = phi i1 [ %11, %9 ], [ %18, %16 ], !dbg !12
-  %21 = zext i1 %20 to i8, !dbg !12
-  %22 = icmp ne i8 %21, 0, !dbg !12
-  br label %12, !dbg !12
 }
 
 ; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
@@ -95,5 +58,5 @@ attributes #0 = { nofree nosync nounwind readnone speculatable willreturn }
 !10 = !DIBasicType(name: "DINT", size: 32, encoding: DW_ATE_signed, flags: DIFlagPublic)
 !11 = !DILocation(line: 2, column: 17, scope: !4)
 !12 = !DILocation(line: 3, column: 16, scope: !4)
-!13 = !DILocation(line: 4, column: 16, scope: !4)
-!14 = !DILocation(line: 3, column: 37, scope: !4)
+!13 = !DILocation(line: 3, column: 37, scope: !4)
+!14 = !DILocation(line: 4, column: 16, scope: !4)

src/codegen/tests/snapshots/rusty__codegen__tests__code_gen_tests__for_statement_continue.snap

@@ -13,64 +13,27 @@ define void @prg(%prg* %0) section "fn-$RUSTY$prg:v" {
 entry:
   %x = getelementptr inbounds %prg, %prg* %0, i32 0, i32 0
   store i32 3, i32* %x, align 4
-  br label %condition_check
-
-condition_check:                                  ; preds = %increment, %entry
+  %1 = load i32, i32* %x, align 4
+  %is_incrementing = icmp sgt i32 %1, 0
+  br i1 %is_incrementing, label %predicate_inc, label %predicate_dec
+
+predicate_inc:                                    ; preds = %loop, %entry
+  %2 = load i32, i32* %x, align 4
+  %condition = icmp sle i32 %2, 10
+  br i1 %condition, label %loop, label %continue
+
+predicate_dec:                                    ; preds = %loop, %entry
+  %3 = load i32, i32* %x, align 4
+  %condition1 = icmp sge i32 %3, 10
+  br i1 %condition1, label %loop, label %continue
+
+loop:                                             ; preds = %predicate_dec, %predicate_inc
+  %4 = load i32, i32* %x, align 4
+  %increment = add i32 %4, 1
+  store i32 %increment, i32* %x, align 4
+  br i1 %is_incrementing, label %predicate_inc, label %predicate_dec
+
+continue:                                         ; preds = %predicate_dec, %predicate_inc
   %load_x = load i32, i32* %x, align 4
-  %load_x1 = load i32, i32* %x, align 4
-  %tmpVar = icmp sle i32 %load_x1, 10
-  %1 = zext i1 %tmpVar to i8
-  %2 = icmp ne i8 %1, 0
-  br i1 %2, label %3, label %6
-
-for_body:                                         ; preds = %13
-  br label %increment
-
-increment:                                        ; preds = %for_body
-  %tmpVar8 = add i32 %load_x, 1
-  store i32 %tmpVar8, i32* %x, align 4
-  br label %condition_check
-
-continue:                                         ; preds = %13
-  %load_x9 = load i32, i32* %x, align 4
   ret void
-
-3:                                                ; preds = %condition_check
-  %load_x2 = load i32, i32* %x, align 4
-  %tmpVar3 = icmp sge i32 %load_x2, 3
-  %4 = zext i1 %tmpVar3 to i8
-  %5 = icmp ne i8 %4, 0
-  br label %6
-
-6:                                                ; preds = %3, %condition_check
-  %7 = phi i1 [ %2, %condition_check ], [ %5, %3 ]
-  %8 = zext i1 %7 to i8
-  %9 = icmp ne i8 %8, 0
-  br i1 %9, label %13, label %10
-
-10:                                               ; preds = %6
-  %load_x4 = load i32, i32* %x, align 4
-  %tmpVar5 = icmp sge i32 %load_x4, 10
-  %11 = zext i1 %tmpVar5 to i8
-  %12 = icmp ne i8 %11, 0
-  br i1 %12, label %17, label %20
-
-13:                                               ; preds = %20, %6
-  %14 = phi i1 [ %9, %6 ], [ %23, %20 ]
-  %15 = zext i1 %14 to i8
-  %16 = icmp ne i8 %15, 0
-  br i1 %16, label %for_body, label %continue
-
-17:                                               ; preds = %10
-  %load_x6 = load i32, i32* %x, align 4
-  %tmpVar7 = icmp sle i32 %load_x6, 3
-  %18 = zext i1 %tmpVar7 to i8
-  %19 = icmp ne i8 %18, 0
-  br label %20
-
-20:                                               ; preds = %17, %10
-  %21 = phi i1 [ %12, %10 ], [ %19, %17 ]
-  %22 = zext i1 %21 to i8
-  %23 = icmp ne i8 %22, 0
-  br label %13
 }