[llvm] [Kaleidoscope] Fix ForExprAST::codegen (PR #88207)

Vlad Mishel via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 9 16:13:01 PDT 2024


https://github.com/vmishelcs created https://github.com/llvm/llvm-project/pull/88207

This PR is to address issue #13638. 

## Background

The Kaleidoscope tutorial has a bug in its approach to generate LLVM IR for for-loops introduced in [Chapter 5](https://llvm.org/docs/tutorial/MyFirstLanguageFrontend/LangImpl05.html).

Given input

```
extern putchard(ch);
def printstar(n) for i = 1, i < n, 1.0 in putchard(42);
```

the tutorial explains that we should be generated the following IR.

```
declare double @putchard(double)

define double @printstar(double %n) {
entry:
  ; initial value = 1.0 (inlined into phi)
  br label %loop

loop:       ; preds = %loop, %entry
  %i = phi double [ 1.000000e+00, %entry ], [ %nextvar, %loop ]
  ; body
  %calltmp = call double @putchard(double 4.200000e+01)
  ; increment
  %nextvar = fadd double %i, 1.000000e+00

  ; termination test
  %cmptmp = fcmp ult double %i, %n
  %booltmp = uitofp i1 %cmptmp to double
  %loopcond = fcmp one double %booltmp, 0.000000e+00
  br i1 %loopcond, label %loop, label %afterloop

afterloop:      ; preds = %loop
  ; loop always returns 0.0
  ret double 0.000000e+00
}
```

However, the IR above has a logic error. In particular, we enter the loop even if we are not supposed to. For instance, if we call `printstar(0);`, we still print a star because the loop condition is not checked until the end of the loop body. This PR proposes a fix for this error, and the following IR dump demonstrates the changes.

```
declare double @putchard(double)

define double @printstar(double %n) {
entry:
  ; initial value = 1.0 (inlined into phi)
  br label %loopcond

loopcond:   ; preds = %loop, %entry
  %i = phi double [ 1.000000e+00, %entry ], [ %nextvar, %loop ]
  ; termination test
  %cmptmp = fcmp ult double %i, %n
  %booltmp = uitofp i1 %cmptmp to double
  %endcond = fcmp one double %booltmp, 0.000000e+00
  br i1 %endcond, label %loop, label %endloop

loop:       ; preds = %loopcond
  ; body
  %calltmp = call double @putchard(double 4.200000e+01)
  ; increment
  %nextvar = fadd double %i, 1.000000e+00
  br label %loopcond

endloop:    ; preds = %loopcond
  ; loop always returns 0.0
  ret double 0.000000e+00
}
```

>From 3f21a33bec221cda6dab8164eaf3055cc6bd931b Mon Sep 17 00:00:00 2001
From: Vladimir Mishel <vmishel at outlook.com>
Date: Mon, 8 Apr 2024 01:11:29 -0700
Subject: [PATCH 1/7] Implemented a fix for ForExprAST::codegen in Ch-5 code

---
 llvm/examples/Kaleidoscope/Chapter5/toy.cpp | 95 ++++++++++++---------
 1 file changed, 56 insertions(+), 39 deletions(-)

diff --git a/llvm/examples/Kaleidoscope/Chapter5/toy.cpp b/llvm/examples/Kaleidoscope/Chapter5/toy.cpp
index 48936bddb1d4f2..d76834d1830cf3 100644
--- a/llvm/examples/Kaleidoscope/Chapter5/toy.cpp
+++ b/llvm/examples/Kaleidoscope/Chapter5/toy.cpp
@@ -682,53 +682,78 @@ Value *IfExprAST::codegen() {
 }
 
 // Output for-loop as:
+// preloop:
 //   ...
 //   start = startexpr
-//   goto loop
+//   goto loopcondition
+// loopcondition:
+//   variable = phi [start, loopheader], [nextvariable, loop]
+//   endcond = endexpr
+//   br endcond, loop, endloop
 // loop:
-//   variable = phi [start, loopheader], [nextvariable, loopend]
 //   ...
 //   bodyexpr
 //   ...
-// loopend:
 //   step = stepexpr
 //   nextvariable = variable + step
-//   endcond = endexpr
-//   br endcond, loop, endloop
-// outloop:
+//   goto loopcondition
+// endloop:
+//   ...
 Value *ForExprAST::codegen() {
   // Emit the start code first, without 'variable' in scope.
   Value *StartVal = Start->codegen();
   if (!StartVal)
     return nullptr;
 
-  // Make the new basic block for the loop header, inserting after current
-  // block.
+  // Make new basic blocks for pre-loop, loop condition, loop body and end-loop
+  // code. 
   Function *TheFunction = Builder->GetInsertBlock()->getParent();
-  BasicBlock *PreheaderBB = Builder->GetInsertBlock();
-  BasicBlock *LoopBB = BasicBlock::Create(*TheContext, "loop", TheFunction);
+  BasicBlock *PreLoopBB = Builder->GetInsertBlock();
+  BasicBlock *LoopConditionBB = BasicBlock::Create(*TheContext, "loopcond",
+      TheFunction);
+  BasicBlock *LoopBB = BasicBlock::Create(*TheContext, "loop");
+  BasicBlock *EndLoopBB = BasicBlock::Create(*TheContext, "endloop");
 
-  // Insert an explicit fall through from the current block to the LoopBB.
-  Builder->CreateBr(LoopBB);
+  // Insert an explicit fall through from current block to LoopConditionBB.
+  Builder->CreateBr(LoopConditionBB);
 
-  // Start insertion in LoopBB.
-  Builder->SetInsertPoint(LoopBB);
+  // Start insertion in LoopConditionBB.
+  Builder->SetInsertPoint(LoopConditionBB);
 
   // Start the PHI node with an entry for Start.
   PHINode *Variable =
       Builder->CreatePHI(Type::getDoubleTy(*TheContext), 2, VarName);
-  Variable->addIncoming(StartVal, PreheaderBB);
+  Variable->addIncoming(StartVal, PreLoopBB);
 
-  // Within the loop, the variable is defined equal to the PHI node.  If it
+  // Within the loop, the variable is defined equal to the PHI node. If it
   // shadows an existing variable, we have to restore it, so save it now.
   Value *OldVal = NamedValues[VarName];
   NamedValues[VarName] = Variable;
 
-  // Emit the body of the loop.  This, like any other expr, can change the
-  // current BB.  Note that we ignore the value computed by the body, but don't
-  // allow an error.
-  if (!Body->codegen())
+  // Compute the end condition.
+  Value *EndCond = End->codegen();
+  if (!EndCond)
+    return nullptr;
+
+  // Convert condition to a bool by comparing non-equal to 0.0.
+  EndCond = Builder->CreateFCmpONE(
+      EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "endcond");
+
+  // Insert the conditional branch that either continues the loop, or exits the
+  // loop.
+  Builder->CreateCondBr(EndCond, LoopBB, EndLoopBB);
+
+  // Attach the basic block that will soon hold the loop body to the end of the
+  // parent function.
+  TheFunction->insert(TheFunction->end(), LoopBB);
+
+  // Emit the loop body within the LoopBB. This, like any other expr, can change
+  // the current BB. Note that we ignore the value computed by the body, but
+  // don't allow an error.
+  Builder->SetInsertPoint(LoopBB);
+  if (!Body->codegen()) {
     return nullptr;
+  }
 
   // Emit the step value.
   Value *StepVal = nullptr;
@@ -743,28 +768,20 @@ Value *ForExprAST::codegen() {
 
   Value *NextVar = Builder->CreateFAdd(Variable, StepVal, "nextvar");
 
-  // Compute the end condition.
-  Value *EndCond = End->codegen();
-  if (!EndCond)
-    return nullptr;
-
-  // Convert condition to a bool by comparing non-equal to 0.0.
-  EndCond = Builder->CreateFCmpONE(
-      EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "loopcond");
-
-  // Create the "after loop" block and insert it.
-  BasicBlock *LoopEndBB = Builder->GetInsertBlock();
-  BasicBlock *AfterBB =
-      BasicBlock::Create(*TheContext, "afterloop", TheFunction);
+  // Add a new entry to the PHI node for the backedge.
+  LoopBB = Builder->GetInsertBlock();
+  Variable->addIncoming(NextVar, LoopBB);
 
-  // Insert the conditional branch into the end of LoopEndBB.
-  Builder->CreateCondBr(EndCond, LoopBB, AfterBB);
+  // Create the unconditional branch that returns to LoopConditionBB to
+  // determine if we should continue looping.
+  Builder->CreateBr(LoopConditionBB);
 
-  // Any new code will be inserted in AfterBB.
-  Builder->SetInsertPoint(AfterBB);
+  // Append EndLoopBB after the loop body. We go to this basic block if the
+  // loop condition says we should not loop anymore.
+  TheFunction->insert(TheFunction->end(), EndLoopBB);
 
-  // Add a new entry to the PHI node for the backedge.
-  Variable->addIncoming(NextVar, LoopEndBB);
+  // Any new code will be inserted after the loop.
+  Builder->SetInsertPoint(EndLoopBB);
 
   // Restore the unshadowed variable.
   if (OldVal)

>From 63d526b50216f891322e5fabe8e6aaee3f2b54ca Mon Sep 17 00:00:00 2001
From: Vladimir Mishel <vmishel at outlook.com>
Date: Mon, 8 Apr 2024 20:06:15 -0700
Subject: [PATCH 2/7] Updated `LangImpl05.rst` to use correct Ch5 code

---
 .../MyFirstLanguageFrontend/LangImpl05.rst    | 196 ++++++++++--------
 1 file changed, 112 insertions(+), 84 deletions(-)

diff --git a/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl05.rst b/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl05.rst
index 0039547e8c7baf..8edd4d6db694d5 100644
--- a/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl05.rst
+++ b/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl05.rst
@@ -598,22 +598,24 @@ this dump is generated with optimizations disabled for clarity):
     define double @printstar(double %n) {
     entry:
       ; initial value = 1.0 (inlined into phi)
-      br label %loop
+      br label %loopcond
 
-    loop:       ; preds = %loop, %entry
+    loopcond:   ; preds = %loop, %entry
       %i = phi double [ 1.000000e+00, %entry ], [ %nextvar, %loop ]
+      ; termination test
+      %cmptmp = fcmp ult double %i, %n
+      %booltmp = uitofp i1 %cmptmp to double
+      %endcond = fcmp one double %booltmp, 0.000000e+00
+      br i1 %endcond, label %loop, label %endloop
+
+    loop:       ; preds = %loopcond
       ; body
       %calltmp = call double @putchard(double 4.200000e+01)
       ; increment
       %nextvar = fadd double %i, 1.000000e+00
+      br label %loopcond
 
-      ; termination test
-      %cmptmp = fcmp ult double %i, %n
-      %booltmp = uitofp i1 %cmptmp to double
-      %loopcond = fcmp one double %booltmp, 0.000000e+00
-      br i1 %loopcond, label %loop, label %afterloop
-
-    afterloop:      ; preds = %loop
+    endloop:    ; preds = %loopcond
       ; loop always returns 0.0
       ret double 0.000000e+00
     }
@@ -637,75 +639,114 @@ expression for the loop value:
         return nullptr;
 
 With this out of the way, the next step is to set up the LLVM basic
-block for the start of the loop body. In the case above, the whole loop
-body is one block, but remember that the body code itself could consist
+blocks that make up the for-loop statement. In the case above, the whole
+loop body is one block, but remember that the body code itself could consist
 of multiple blocks (e.g. if it contains an if/then/else or a for/in
 expression).
 
 .. code-block:: c++
 
-      // Make the new basic block for the loop header, inserting after current
-      // block.
+      // Make new basic blocks for pre-loop, loop condition, loop body and end-loop
+      // code. 
       Function *TheFunction = Builder->GetInsertBlock()->getParent();
-      BasicBlock *PreheaderBB = Builder->GetInsertBlock();
-      BasicBlock *LoopBB =
-          BasicBlock::Create(*TheContext, "loop", TheFunction);
+      BasicBlock *PreLoopBB = Builder->GetInsertBlock();
+      BasicBlock *LoopConditionBB = BasicBlock::Create(*TheContext, "loopcond",
+          TheFunction);
+      BasicBlock *LoopBB = BasicBlock::Create(*TheContext, "loop");
+      BasicBlock *EndLoopBB = BasicBlock::Create(*TheContext, "endloop");
 
-      // Insert an explicit fall through from the current block to the LoopBB.
-      Builder->CreateBr(LoopBB);
+      // Insert an explicit fall through from current block to LoopConditionBB.
+      Builder->CreateBr(LoopConditionBB);
 
 This code is similar to what we saw for if/then/else. Because we will
 need it to create the Phi node, we remember the block that falls through
-into the loop. Once we have that, we create the actual block that starts
-the loop and create an unconditional branch for the fall-through between
-the two blocks.
+into the loop condition check. Once we have that, we create the actual block
+that determines if control should enter the loop and attach it directly to the
+end of our parent function. The other two blocks (``LoopBB`` and ``EndLoopBB``)
+are created, but aren't inserted into the function, similarly to our previous
+work on if/then/else.
+
+We also create an unconditional branch into the loop condition block from the
+pre-loop code.
 
 .. code-block:: c++
 
-      // Start insertion in LoopBB.
-      Builder->SetInsertPoint(LoopBB);
+      // Start insertion in LoopConditionBB.
+      Builder->SetInsertPoint(LoopConditionBB);
 
       // Start the PHI node with an entry for Start.
-      PHINode *Variable = Builder->CreatePHI(Type::getDoubleTy(*TheContext),
-                                             2, VarName);
-      Variable->addIncoming(StartVal, PreheaderBB);
+      PHINode *Variable =
+          Builder->CreatePHI(Type::getDoubleTy(*TheContext), 2, VarName);
+      Variable->addIncoming(StartVal, PreLoopBB);
 
 Now that the "preheader" for the loop is set up, we switch to emitting
-code for the loop body. To begin with, we move the insertion point and
-create the PHI node for the loop induction variable. Since we already
+code for the loop condition check. To begin with, we move the insertion point
+and create the Phi node for the loop induction variable. Since we already
 know the incoming value for the starting value, we add it to the Phi
-node. Note that the Phi will eventually get a second value for the
+node. Note that the Phi node will eventually get a second value for the
 backedge, but we can't set it up yet (because it doesn't exist!).
 
 .. code-block:: c++
 
-      // Within the loop, the variable is defined equal to the PHI node.  If it
+      // Within the loop, the variable is defined equal to the PHI node. If it
       // shadows an existing variable, we have to restore it, so save it now.
       Value *OldVal = NamedValues[VarName];
       NamedValues[VarName] = Variable;
 
-      // Emit the body of the loop.  This, like any other expr, can change the
-      // current BB.  Note that we ignore the value computed by the body, but don't
-      // allow an error.
-      if (!Body->codegen())
+      // Compute the end condition.
+      Value *EndCond = End->codegen();
+      if (!EndCond)
         return nullptr;
 
-Now the code starts to get more interesting. Our 'for' loop introduces a
-new variable to the symbol table. This means that our symbol table can
-now contain either function arguments or loop variables. To handle this,
-before we codegen the body of the loop, we add the loop variable as the
-current value for its name. Note that it is possible that there is a
-variable of the same name in the outer scope. It would be easy to make
+Our for-loop introduces a new variable to the symbol table. This means that
+our symbol table can now contain either function arguments or loop variables.
+To handle this, before we codegen the remainder of the loop, we add the loop
+variable as the current value for its name. Note that it is possible there
+is a variable of the same name in the outer scope. It would be easy to make
 this an error (emit an error and return null if there is already an
 entry for VarName) but we choose to allow shadowing of variables. In
 order to handle this correctly, we remember the Value that we are
 potentially shadowing in ``OldVal`` (which will be null if there is no
-shadowed variable).
+shadowed variable). This allows the loop body (which we will codegen soon) to
+use the loop variable: any references to it will naturally find it in the 
+symbol table.
+
+Once the loop variable is set into the symbol table, we codegen the condition
+that determines if we can enter into the loop (or continue looping, depending
+on if we are arriving from the "preheader" or the loop body).
+
+.. code-block:: c++
+
+      // Convert condition to a bool by comparing non-equal to 0.0.
+      EndCond = Builder->CreateFCmpONE(
+          EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "endcond");
+
+      // Insert the conditional branch that either continues the loop, or exits the
+      // loop.
+      Builder->CreateCondBr(EndCond, LoopBB, EndLoopBB);
 
-Once the loop variable is set into the symbol table, the code
-recursively codegen's the body. This allows the body to use the loop
-variable: any references to it will naturally find it in the symbol
-table.
+As with if/then/else, after emitting the condition, we compare that value to
+zero to get a truth value as a 1-bit (bool) value. Next we emit the conditional
+branch that chooses if we enter the loop body, or move on to the post-loop code.
+
+.. code-block:: c++
+
+      // Attach the basic block that will soon hold the loop body to the end of the
+      // parent function.
+      TheFunction->insert(TheFunction->end(), LoopBB);
+
+      // Emit the loop body within the LoopBB. This, like any other expr, can change
+      // the current BB. Note that we ignore the value computed by the body, but
+      // don't allow an error.
+      Builder->SetInsertPoint(LoopBB);
+      if (!Body->codegen()) {
+        return nullptr;
+      }
+
+Next, we insert our basic block that will soon hold the loop body to the end of 
+the specified function. We recursively codegen the body, remembering to update
+the IRBuilder's insert point to the basic block that is supposed to hold the
+loop body beforehand.
 
 .. code-block:: c++
 
@@ -723,51 +764,42 @@ table.
       Value *NextVar = Builder->CreateFAdd(Variable, StepVal, "nextvar");
 
 Now that the body is emitted, we compute the next value of the iteration
-variable by adding the step value, or 1.0 if it isn't present.
-'``NextVar``' will be the value of the loop variable on the next
-iteration of the loop.
+variable by adding the step value, or 1.0 if it isn't present. ``NextVar``
+will be the value of the loop variable on the next iteration of the loop.
 
 .. code-block:: c++
 
-      // Compute the end condition.
-      Value *EndCond = End->codegen();
-      if (!EndCond)
-        return nullptr;
+      // Add a new entry to the PHI node for the backedge.
+      LoopBB = Builder->GetInsertBlock();
+      Variable->addIncoming(NextVar, LoopBB);
 
-      // Convert condition to a bool by comparing non-equal to 0.0.
-      EndCond = Builder->CreateFCmpONE(
-          EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "loopcond");
+      // Create the unconditional branch that returns to LoopConditionBB to
+      // determine if we should continue looping.
+      Builder->CreateBr(LoopConditionBB);
 
-Finally, we evaluate the exit value of the loop, to determine whether
-the loop should exit. This mirrors the condition evaluation for the
-if/then/else statement.
+Here, similarly to how we did it in our implementation of the if/then/else
+statement, we get an up-to-date value for ``LoopBB`` (because it's possible
+that the loop body has changed the basic block that the Builder is emitting
+into) and use it to add a backedge to our Phi node. This backedge denotes
+the value of the incremented loop variable. We also create an unconditional
+branch back to the basic block that performs the check if we should continue
+looping. This completes the loop body code.
 
 .. code-block:: c++
 
-      // Create the "after loop" block and insert it.
-      BasicBlock *LoopEndBB = Builder->GetInsertBlock();
-      BasicBlock *AfterBB =
-          BasicBlock::Create(*TheContext, "afterloop", TheFunction);
+      // Append EndLoopBB after the loop body. We go to this basic block if the
+      // loop condition says we should not loop anymore.
+      TheFunction->insert(TheFunction->end(), EndLoopBB);
 
-      // Insert the conditional branch into the end of LoopEndBB.
-      Builder->CreateCondBr(EndCond, LoopBB, AfterBB);
+      // Any new code will be inserted after the loop.
+      Builder->SetInsertPoint(EndLoopBB);
 
-      // Any new code will be inserted in AfterBB.
-      Builder->SetInsertPoint(AfterBB);
-
-With the code for the body of the loop complete, we just need to finish
-up the control flow for it. This code remembers the end block (for the
-phi node), then creates the block for the loop exit ("afterloop"). Based
-on the value of the exit condition, it creates a conditional branch that
-chooses between executing the loop again and exiting the loop. Any
-future code is emitted in the "afterloop" block, so it sets the
-insertion position to it.
+Finally, we append the post-loop basic block created earlier (denoted by 
+``EndLoopBB``) to the parent function, and update the IRBuilder's insert point
+such that any new subsequent code is generated in that post-loop basic block.
 
 .. code-block:: c++
 
-      // Add a new entry to the PHI node for the backedge.
-      Variable->addIncoming(NextVar, LoopEndBB);
-
       // Restore the unshadowed variable.
       if (OldVal)
         NamedValues[VarName] = OldVal;
@@ -778,12 +810,9 @@ insertion position to it.
       return Constant::getNullValue(Type::getDoubleTy(*TheContext));
     }
 
-The final code handles various cleanups: now that we have the "NextVar"
-value, we can add the incoming value to the loop PHI node. After that,
-we remove the loop variable from the symbol table, so that it isn't in
-scope after the for loop. Finally, code generation of the for loop
-always returns 0.0, so that is what we return from
-``ForExprAST::codegen()``.
+The final bit of code handles some clean-ups: We remove the loop variable from
+the symbol table so that it isn't in scope after the for-loop, and return a 0.0
+value.
 
 With this, we conclude the "adding control flow to Kaleidoscope" chapter
 of the tutorial. In this chapter we added two control flow constructs,
@@ -811,4 +840,3 @@ Here is the code:
    :language: c++
 
 `Next: Extending the language: user-defined operators <LangImpl06.html>`_
-

>From 81f62daec97b9dc1b1a2a22da314e2ef2a40e12b Mon Sep 17 00:00:00 2001
From: Vladimir Mishel <vmishel at outlook.com>
Date: Mon, 8 Apr 2024 20:30:54 -0700
Subject: [PATCH 3/7] Updated Kaleidoscope Ch-6 code listing

---
 llvm/examples/Kaleidoscope/Chapter6/toy.cpp | 95 ++++++++++++---------
 1 file changed, 56 insertions(+), 39 deletions(-)

diff --git a/llvm/examples/Kaleidoscope/Chapter6/toy.cpp b/llvm/examples/Kaleidoscope/Chapter6/toy.cpp
index ebe4322287b21f..c29f1e719990e3 100644
--- a/llvm/examples/Kaleidoscope/Chapter6/toy.cpp
+++ b/llvm/examples/Kaleidoscope/Chapter6/toy.cpp
@@ -794,53 +794,78 @@ Value *IfExprAST::codegen() {
 }
 
 // Output for-loop as:
+// preloop:
 //   ...
 //   start = startexpr
-//   goto loop
+//   goto loopcondition
+// loopcondition:
+//   variable = phi [start, loopheader], [nextvariable, loop]
+//   endcond = endexpr
+//   br endcond, loop, endloop
 // loop:
-//   variable = phi [start, loopheader], [nextvariable, loopend]
 //   ...
 //   bodyexpr
 //   ...
-// loopend:
 //   step = stepexpr
 //   nextvariable = variable + step
-//   endcond = endexpr
-//   br endcond, loop, endloop
-// outloop:
+//   goto loopcondition
+// endloop:
+//   ...
 Value *ForExprAST::codegen() {
   // Emit the start code first, without 'variable' in scope.
   Value *StartVal = Start->codegen();
   if (!StartVal)
     return nullptr;
 
-  // Make the new basic block for the loop header, inserting after current
-  // block.
+  // Make new basic blocks for pre-loop, loop condition, loop body and end-loop
+  // code. 
   Function *TheFunction = Builder->GetInsertBlock()->getParent();
-  BasicBlock *PreheaderBB = Builder->GetInsertBlock();
-  BasicBlock *LoopBB = BasicBlock::Create(*TheContext, "loop", TheFunction);
+  BasicBlock *PreLoopBB = Builder->GetInsertBlock();
+  BasicBlock *LoopConditionBB = BasicBlock::Create(*TheContext, "loopcond",
+      TheFunction);
+  BasicBlock *LoopBB = BasicBlock::Create(*TheContext, "loop");
+  BasicBlock *EndLoopBB = BasicBlock::Create(*TheContext, "endloop");
 
-  // Insert an explicit fall through from the current block to the LoopBB.
-  Builder->CreateBr(LoopBB);
+  // Insert an explicit fall through from current block to LoopConditionBB.
+  Builder->CreateBr(LoopConditionBB);
 
-  // Start insertion in LoopBB.
-  Builder->SetInsertPoint(LoopBB);
+  // Start insertion in LoopConditionBB.
+  Builder->SetInsertPoint(LoopConditionBB);
 
   // Start the PHI node with an entry for Start.
   PHINode *Variable =
       Builder->CreatePHI(Type::getDoubleTy(*TheContext), 2, VarName);
-  Variable->addIncoming(StartVal, PreheaderBB);
+  Variable->addIncoming(StartVal, PreLoopBB);
 
-  // Within the loop, the variable is defined equal to the PHI node.  If it
+  // Within the loop, the variable is defined equal to the PHI node. If it
   // shadows an existing variable, we have to restore it, so save it now.
   Value *OldVal = NamedValues[VarName];
   NamedValues[VarName] = Variable;
 
-  // Emit the body of the loop.  This, like any other expr, can change the
-  // current BB.  Note that we ignore the value computed by the body, but don't
-  // allow an error.
-  if (!Body->codegen())
+  // Compute the end condition.
+  Value *EndCond = End->codegen();
+  if (!EndCond)
+    return nullptr;
+
+  // Convert condition to a bool by comparing non-equal to 0.0.
+  EndCond = Builder->CreateFCmpONE(
+      EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "endcond");
+
+  // Insert the conditional branch that either continues the loop, or exits the
+  // loop.
+  Builder->CreateCondBr(EndCond, LoopBB, EndLoopBB);
+
+  // Attach the basic block that will soon hold the loop body to the end of the
+  // parent function.
+  TheFunction->insert(TheFunction->end(), LoopBB);
+
+  // Emit the loop body within the LoopBB. This, like any other expr, can change
+  // the current BB. Note that we ignore the value computed by the body, but
+  // don't allow an error.
+  Builder->SetInsertPoint(LoopBB);
+  if (!Body->codegen()) {
     return nullptr;
+  }
 
   // Emit the step value.
   Value *StepVal = nullptr;
@@ -855,28 +880,20 @@ Value *ForExprAST::codegen() {
 
   Value *NextVar = Builder->CreateFAdd(Variable, StepVal, "nextvar");
 
-  // Compute the end condition.
-  Value *EndCond = End->codegen();
-  if (!EndCond)
-    return nullptr;
-
-  // Convert condition to a bool by comparing non-equal to 0.0.
-  EndCond = Builder->CreateFCmpONE(
-      EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "loopcond");
-
-  // Create the "after loop" block and insert it.
-  BasicBlock *LoopEndBB = Builder->GetInsertBlock();
-  BasicBlock *AfterBB =
-      BasicBlock::Create(*TheContext, "afterloop", TheFunction);
+  // Add a new entry to the PHI node for the backedge.
+  LoopBB = Builder->GetInsertBlock();
+  Variable->addIncoming(NextVar, LoopBB);
 
-  // Insert the conditional branch into the end of LoopEndBB.
-  Builder->CreateCondBr(EndCond, LoopBB, AfterBB);
+  // Create the unconditional branch that returns to LoopConditionBB to
+  // determine if we should continue looping.
+  Builder->CreateBr(LoopConditionBB);
 
-  // Any new code will be inserted in AfterBB.
-  Builder->SetInsertPoint(AfterBB);
+  // Append EndLoopBB after the loop body. We go to this basic block if the
+  // loop condition says we should not loop anymore.
+  TheFunction->insert(TheFunction->end(), EndLoopBB);
 
-  // Add a new entry to the PHI node for the backedge.
-  Variable->addIncoming(NextVar, LoopEndBB);
+  // Any new code will be inserted after the loop.
+  Builder->SetInsertPoint(EndLoopBB);
 
   // Restore the unshadowed variable.
   if (OldVal)

>From 19fdb0f3b19dcbef565d22568250a11078dc3357 Mon Sep 17 00:00:00 2001
From: Vladimir Mishel <vmishel at outlook.com>
Date: Mon, 8 Apr 2024 21:57:32 -0700
Subject: [PATCH 4/7] Changed some comments in Ch5 for clarity

---
 .../docs/tutorial/MyFirstLanguageFrontend/LangImpl05.rst | 5 ++---
 llvm/examples/Kaleidoscope/Chapter5/toy.cpp              | 9 ++++-----
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl05.rst b/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl05.rst
index 8edd4d6db694d5..128d178d42bdd2 100644
--- a/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl05.rst
+++ b/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl05.rst
@@ -646,8 +646,7 @@ expression).
 
 .. code-block:: c++
 
-      // Make new basic blocks for pre-loop, loop condition, loop body and end-loop
-      // code. 
+      // Make new basic blocks for loop condition, loop body and end-loop code.
       Function *TheFunction = Builder->GetInsertBlock()->getParent();
       BasicBlock *PreLoopBB = Builder->GetInsertBlock();
       BasicBlock *LoopConditionBB = BasicBlock::Create(*TheContext, "loopcond",
@@ -664,7 +663,7 @@ into the loop condition check. Once we have that, we create the actual block
 that determines if control should enter the loop and attach it directly to the
 end of our parent function. The other two blocks (``LoopBB`` and ``EndLoopBB``)
 are created, but aren't inserted into the function, similarly to our previous
-work on if/then/else.
+work on if/then/else. These will be used to complete our loop IR later on.
 
 We also create an unconditional branch into the loop condition block from the
 pre-loop code.
diff --git a/llvm/examples/Kaleidoscope/Chapter5/toy.cpp b/llvm/examples/Kaleidoscope/Chapter5/toy.cpp
index d76834d1830cf3..74f4a91e6ff7b0 100644
--- a/llvm/examples/Kaleidoscope/Chapter5/toy.cpp
+++ b/llvm/examples/Kaleidoscope/Chapter5/toy.cpp
@@ -685,8 +685,8 @@ Value *IfExprAST::codegen() {
 // preloop:
 //   ...
 //   start = startexpr
-//   goto loopcondition
-// loopcondition:
+//   goto loopcond
+// loopcond:
 //   variable = phi [start, loopheader], [nextvariable, loop]
 //   endcond = endexpr
 //   br endcond, loop, endloop
@@ -696,7 +696,7 @@ Value *IfExprAST::codegen() {
 //   ...
 //   step = stepexpr
 //   nextvariable = variable + step
-//   goto loopcondition
+//   goto loopcond
 // endloop:
 //   ...
 Value *ForExprAST::codegen() {
@@ -705,8 +705,7 @@ Value *ForExprAST::codegen() {
   if (!StartVal)
     return nullptr;
 
-  // Make new basic blocks for pre-loop, loop condition, loop body and end-loop
-  // code. 
+  // Make new basic blocks for loop condition, loop body and end-loop code. 
   Function *TheFunction = Builder->GetInsertBlock()->getParent();
   BasicBlock *PreLoopBB = Builder->GetInsertBlock();
   BasicBlock *LoopConditionBB = BasicBlock::Create(*TheContext, "loopcond",

>From 7426bf29e7efc0da1d5793ec1bd92047d8e7a37a Mon Sep 17 00:00:00 2001
From: Vladimir Mishel <vmishel at outlook.com>
Date: Mon, 8 Apr 2024 22:43:52 -0700
Subject: [PATCH 5/7] Fixed ForExprAST::codegen in Ch7 and updated rst

---
 .../MyFirstLanguageFrontend/LangImpl07.rst    | 16 ++-
 llvm/examples/Kaleidoscope/Chapter7/toy.cpp   | 97 +++++++++++--------
 2 files changed, 62 insertions(+), 51 deletions(-)

diff --git a/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl07.rst b/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl07.rst
index 8fd4c39d3ff47b..0bb0a0835b8376 100644
--- a/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl07.rst
+++ b/llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl07.rst
@@ -384,17 +384,16 @@ the unabridged code):
 
       // Store the value into the alloca.
       Builder->CreateStore(StartVal, Alloca);
-      ...
 
-      // Compute the end condition.
-      Value *EndCond = End->codegen();
-      if (!EndCond)
-        return nullptr;
+      // If the loop variable shadows an existing variable, we have to restore it,
+      // so save it now. Set VarName to refer to our recently created alloca.
+      AllocaInst *OldVal = NamedValues[VarName];
+      NamedValues[VarName] = Alloca;
+      ...
 
-      // Reload, increment, and restore the alloca.  This handles the case where
-      // the body of the loop mutates the variable.
+      // Load, increment and store the new loop variable.
       Value *CurVar = Builder->CreateLoad(Alloca->getAllocatedType(), Alloca,
-                                          VarName.c_str());
+                                          VarName);
       Value *NextVar = Builder->CreateFAdd(CurVar, StepVal, "nextvar");
       Builder->CreateStore(NextVar, Alloca);
       ...
@@ -883,4 +882,3 @@ Here is the code:
    :language: c++
 
 `Next: Compiling to Object Code <LangImpl08.html>`_
-
diff --git a/llvm/examples/Kaleidoscope/Chapter7/toy.cpp b/llvm/examples/Kaleidoscope/Chapter7/toy.cpp
index 68208c4f3394ab..a5b8aad3ca2417 100644
--- a/llvm/examples/Kaleidoscope/Chapter7/toy.cpp
+++ b/llvm/examples/Kaleidoscope/Chapter7/toy.cpp
@@ -905,20 +905,21 @@ Value *IfExprAST::codegen() {
 //   ...
 //   start = startexpr
 //   store start -> var
-//   goto loop
+//   goto loopcond
+// loopcond:
+//   endcond = endexpr
+//   br endcond, loop, endloop
 // loop:
 //   ...
 //   bodyexpr
 //   ...
-// loopend:
 //   step = stepexpr
-//   endcond = endexpr
-//
 //   curvar = load var
 //   nextvar = curvar + step
 //   store nextvar -> var
-//   br endcond, loop, endloop
-// outloop:
+//   goto loopcond
+// endloop:
+//   ...
 Value *ForExprAST::codegen() {
   Function *TheFunction = Builder->GetInsertBlock()->getParent();
 
@@ -933,26 +934,47 @@ Value *ForExprAST::codegen() {
   // Store the value into the alloca.
   Builder->CreateStore(StartVal, Alloca);
 
-  // Make the new basic block for the loop header, inserting after current
-  // block.
-  BasicBlock *LoopBB = BasicBlock::Create(*TheContext, "loop", TheFunction);
+  // If the loop variable shadows an existing variable, we have to restore it,
+  // so save it now. Set VarName to refer to our recently created alloca.
+  AllocaInst *OldVal = NamedValues[VarName];
+  NamedValues[VarName] = Alloca;
 
-  // Insert an explicit fall through from the current block to the LoopBB.
-  Builder->CreateBr(LoopBB);
+  // Make new basic blocks for loop condition, loop body and end-loop code.
+  BasicBlock *LoopConditionBB = BasicBlock::Create(*TheContext, "loopcond",
+      TheFunction);
+  BasicBlock *LoopBB = BasicBlock::Create(*TheContext, "loop");
+  BasicBlock *EndLoopBB = BasicBlock::Create(*TheContext, "endloop");
 
-  // Start insertion in LoopBB.
-  Builder->SetInsertPoint(LoopBB);
+  // Insert an explicit fall through from current block to LoopConditionBB.
+  Builder->CreateBr(LoopConditionBB);
 
-  // Within the loop, the variable is defined equal to the PHI node.  If it
-  // shadows an existing variable, we have to restore it, so save it now.
-  AllocaInst *OldVal = NamedValues[VarName];
-  NamedValues[VarName] = Alloca;
+  // Start insertion in LoopConditionBB.
+  Builder->SetInsertPoint(LoopConditionBB);
+
+  // Compute the end condition.
+  Value *EndCond = End->codegen();
+  if (!EndCond)
+    return nullptr;
 
-  // Emit the body of the loop.  This, like any other expr, can change the
-  // current BB.  Note that we ignore the value computed by the body, but don't
-  // allow an error.
-  if (!Body->codegen())
+  // Convert condition to a bool by comparing non-equal to 0.0.
+  EndCond = Builder->CreateFCmpONE(
+      EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "endcond");
+
+  // Insert the conditional branch that either continues the loop, or exits the
+  // loop.
+  Builder->CreateCondBr(EndCond, LoopBB, EndLoopBB);
+
+  // Attach the basic block that will soon hold the loop body to the end of the
+  // parent function.
+  TheFunction->insert(TheFunction->end(), LoopBB);
+
+  // Emit the loop body within the LoopBB. This, like any other expr, can change
+  // the current BB. Note that we ignore the value computed by the body, but
+  // don't allow an error.
+  Builder->SetInsertPoint(LoopBB);
+  if (!Body->codegen()) {
     return nullptr;
+  }
 
   // Emit the step value.
   Value *StepVal = nullptr;
@@ -965,31 +987,22 @@ Value *ForExprAST::codegen() {
     StepVal = ConstantFP::get(*TheContext, APFloat(1.0));
   }
 
-  // Compute the end condition.
-  Value *EndCond = End->codegen();
-  if (!EndCond)
-    return nullptr;
-
-  // Reload, increment, and restore the alloca.  This handles the case where
-  // the body of the loop mutates the variable.
-  Value *CurVar =
-      Builder->CreateLoad(Alloca->getAllocatedType(), Alloca, VarName.c_str());
+  // Load, increment and store the new loop variable.
+  Value *CurVar = Builder->CreateLoad(Alloca->getAllocatedType(), Alloca,
+                                      VarName);
   Value *NextVar = Builder->CreateFAdd(CurVar, StepVal, "nextvar");
   Builder->CreateStore(NextVar, Alloca);
 
-  // Convert condition to a bool by comparing non-equal to 0.0.
-  EndCond = Builder->CreateFCmpONE(
-      EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "loopcond");
-
-  // Create the "after loop" block and insert it.
-  BasicBlock *AfterBB =
-      BasicBlock::Create(*TheContext, "afterloop", TheFunction);
+  // Create the unconditional branch that returns to LoopConditionBB to
+  // determine if we should continue looping.
+  Builder->CreateBr(LoopConditionBB);
 
-  // Insert the conditional branch into the end of LoopEndBB.
-  Builder->CreateCondBr(EndCond, LoopBB, AfterBB);
+  // Append EndLoopBB after the loop body. We go to this basic block if the
+  // loop condition says we should not loop anymore.
+  TheFunction->insert(TheFunction->end(), EndLoopBB);
 
-  // Any new code will be inserted in AfterBB.
-  Builder->SetInsertPoint(AfterBB);
+  // Any new code will be inserted after the loop.
+  Builder->SetInsertPoint(EndLoopBB);
 
   // Restore the unshadowed variable.
   if (OldVal)
@@ -1104,7 +1117,7 @@ Function *FunctionAST::codegen() {
     verifyFunction(*TheFunction);
 
     // Run the optimizer on the function.
-    TheFPM->run(*TheFunction, *TheFAM);
+    // TheFPM->run(*TheFunction, *TheFAM);
 
     return TheFunction;
   }

>From 792faa513154c6cdda136a6471365794d49c74dd Mon Sep 17 00:00:00 2001
From: Vladimir Mishel <vmishel at outlook.com>
Date: Mon, 8 Apr 2024 22:46:11 -0700
Subject: [PATCH 6/7] Uncommented function optimization pass

---
 llvm/examples/Kaleidoscope/Chapter7/toy.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/examples/Kaleidoscope/Chapter7/toy.cpp b/llvm/examples/Kaleidoscope/Chapter7/toy.cpp
index a5b8aad3ca2417..11b63091c47889 100644
--- a/llvm/examples/Kaleidoscope/Chapter7/toy.cpp
+++ b/llvm/examples/Kaleidoscope/Chapter7/toy.cpp
@@ -1117,7 +1117,7 @@ Function *FunctionAST::codegen() {
     verifyFunction(*TheFunction);
 
     // Run the optimizer on the function.
-    // TheFPM->run(*TheFunction, *TheFAM);
+    TheFPM->run(*TheFunction, *TheFAM);
 
     return TheFunction;
   }

>From d006bd10a17801954495bd351eeec8a6e6acf976 Mon Sep 17 00:00:00 2001
From: Vladimir Mishel <vmishel at outlook.com>
Date: Tue, 9 Apr 2024 15:52:35 -0700
Subject: [PATCH 7/7] Updated Kaleidoscope code for Ch8 and Ch9

---
 llvm/examples/Kaleidoscope/Chapter8/toy.cpp | 95 ++++++++++++---------
 llvm/examples/Kaleidoscope/Chapter9/toy.cpp | 95 ++++++++++++---------
 2 files changed, 108 insertions(+), 82 deletions(-)

diff --git a/llvm/examples/Kaleidoscope/Chapter8/toy.cpp b/llvm/examples/Kaleidoscope/Chapter8/toy.cpp
index ae2f9c7059e5fb..4b2d2aeebff64c 100644
--- a/llvm/examples/Kaleidoscope/Chapter8/toy.cpp
+++ b/llvm/examples/Kaleidoscope/Chapter8/toy.cpp
@@ -893,20 +893,21 @@ Value *IfExprAST::codegen() {
 //   ...
 //   start = startexpr
 //   store start -> var
-//   goto loop
+//   goto loopcond
+// loopcond:
+//   endcond = endexpr
+//   br endcond, loop, endloop
 // loop:
 //   ...
 //   bodyexpr
 //   ...
-// loopend:
 //   step = stepexpr
-//   endcond = endexpr
-//
 //   curvar = load var
 //   nextvar = curvar + step
 //   store nextvar -> var
-//   br endcond, loop, endloop
-// outloop:
+//   goto loopcond
+// endloop:
+//   ...
 Value *ForExprAST::codegen() {
   Function *TheFunction = Builder->GetInsertBlock()->getParent();
 
@@ -921,26 +922,47 @@ Value *ForExprAST::codegen() {
   // Store the value into the alloca.
   Builder->CreateStore(StartVal, Alloca);
 
-  // Make the new basic block for the loop header, inserting after current
-  // block.
-  BasicBlock *LoopBB = BasicBlock::Create(*TheContext, "loop", TheFunction);
+  // If the loop variable shadows an existing variable, we have to restore it,
+  // so save it now. Set VarName to refer to our recently created alloca.
+  AllocaInst *OldVal = NamedValues[VarName];
+  NamedValues[VarName] = Alloca;
 
-  // Insert an explicit fall through from the current block to the LoopBB.
-  Builder->CreateBr(LoopBB);
+  // Make new basic blocks for loop condition, loop body and end-loop code.
+  BasicBlock *LoopConditionBB = BasicBlock::Create(*TheContext, "loopcond",
+      TheFunction);
+  BasicBlock *LoopBB = BasicBlock::Create(*TheContext, "loop");
+  BasicBlock *EndLoopBB = BasicBlock::Create(*TheContext, "endloop");
 
-  // Start insertion in LoopBB.
-  Builder->SetInsertPoint(LoopBB);
+  // Insert an explicit fall through from current block to LoopConditionBB.
+  Builder->CreateBr(LoopConditionBB);
 
-  // Within the loop, the variable is defined equal to the PHI node.  If it
-  // shadows an existing variable, we have to restore it, so save it now.
-  AllocaInst *OldVal = NamedValues[VarName];
-  NamedValues[VarName] = Alloca;
+  // Start insertion in LoopConditionBB.
+  Builder->SetInsertPoint(LoopConditionBB);
+
+  // Compute the end condition.
+  Value *EndCond = End->codegen();
+  if (!EndCond)
+    return nullptr;
 
-  // Emit the body of the loop.  This, like any other expr, can change the
-  // current BB.  Note that we ignore the value computed by the body, but don't
-  // allow an error.
-  if (!Body->codegen())
+  // Convert condition to a bool by comparing non-equal to 0.0.
+  EndCond = Builder->CreateFCmpONE(
+      EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "endcond");
+
+  // Insert the conditional branch that either continues the loop, or exits the
+  // loop.
+  Builder->CreateCondBr(EndCond, LoopBB, EndLoopBB);
+
+  // Attach the basic block that will soon hold the loop body to the end of the
+  // parent function.
+  TheFunction->insert(TheFunction->end(), LoopBB);
+
+  // Emit the loop body within the LoopBB. This, like any other expr, can change
+  // the current BB. Note that we ignore the value computed by the body, but
+  // don't allow an error.
+  Builder->SetInsertPoint(LoopBB);
+  if (!Body->codegen()) {
     return nullptr;
+  }
 
   // Emit the step value.
   Value *StepVal = nullptr;
@@ -953,31 +975,22 @@ Value *ForExprAST::codegen() {
     StepVal = ConstantFP::get(*TheContext, APFloat(1.0));
   }
 
-  // Compute the end condition.
-  Value *EndCond = End->codegen();
-  if (!EndCond)
-    return nullptr;
-
-  // Reload, increment, and restore the alloca.  This handles the case where
-  // the body of the loop mutates the variable.
-  Value *CurVar = Builder->CreateLoad(Type::getDoubleTy(*TheContext), Alloca,
-                                      VarName.c_str());
+  // Load, increment and store the new loop variable.
+  Value *CurVar = Builder->CreateLoad(Alloca->getAllocatedType(), Alloca,
+                                      VarName);
   Value *NextVar = Builder->CreateFAdd(CurVar, StepVal, "nextvar");
   Builder->CreateStore(NextVar, Alloca);
 
-  // Convert condition to a bool by comparing non-equal to 0.0.
-  EndCond = Builder->CreateFCmpONE(
-      EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "loopcond");
-
-  // Create the "after loop" block and insert it.
-  BasicBlock *AfterBB =
-      BasicBlock::Create(*TheContext, "afterloop", TheFunction);
+  // Create the unconditional branch that returns to LoopConditionBB to
+  // determine if we should continue looping.
+  Builder->CreateBr(LoopConditionBB);
 
-  // Insert the conditional branch into the end of LoopEndBB.
-  Builder->CreateCondBr(EndCond, LoopBB, AfterBB);
+  // Append EndLoopBB after the loop body. We go to this basic block if the
+  // loop condition says we should not loop anymore.
+  TheFunction->insert(TheFunction->end(), EndLoopBB);
 
-  // Any new code will be inserted in AfterBB.
-  Builder->SetInsertPoint(AfterBB);
+  // Any new code will be inserted after the loop.
+  Builder->SetInsertPoint(EndLoopBB);
 
   // Restore the unshadowed variable.
   if (OldVal)
diff --git a/llvm/examples/Kaleidoscope/Chapter9/toy.cpp b/llvm/examples/Kaleidoscope/Chapter9/toy.cpp
index 51457a3c22ade3..a17830e240d294 100644
--- a/llvm/examples/Kaleidoscope/Chapter9/toy.cpp
+++ b/llvm/examples/Kaleidoscope/Chapter9/toy.cpp
@@ -1063,20 +1063,21 @@ Value *IfExprAST::codegen() {
 //   ...
 //   start = startexpr
 //   store start -> var
-//   goto loop
+//   goto loopcond
+// loopcond:
+//   endcond = endexpr
+//   br endcond, loop, endloop
 // loop:
 //   ...
 //   bodyexpr
 //   ...
-// loopend:
 //   step = stepexpr
-//   endcond = endexpr
-//
 //   curvar = load var
 //   nextvar = curvar + step
 //   store nextvar -> var
-//   br endcond, loop, endloop
-// outloop:
+//   goto loopcond
+// endloop:
+//   ...
 Value *ForExprAST::codegen() {
   Function *TheFunction = Builder->GetInsertBlock()->getParent();
 
@@ -1093,26 +1094,47 @@ Value *ForExprAST::codegen() {
   // Store the value into the alloca.
   Builder->CreateStore(StartVal, Alloca);
 
-  // Make the new basic block for the loop header, inserting after current
-  // block.
-  BasicBlock *LoopBB = BasicBlock::Create(*TheContext, "loop", TheFunction);
+  // If the loop variable shadows an existing variable, we have to restore it,
+  // so save it now. Set VarName to refer to our recently created alloca.
+  AllocaInst *OldVal = NamedValues[VarName];
+  NamedValues[VarName] = Alloca;
 
-  // Insert an explicit fall through from the current block to the LoopBB.
-  Builder->CreateBr(LoopBB);
+  // Make new basic blocks for loop condition, loop body and end-loop code.
+  BasicBlock *LoopConditionBB = BasicBlock::Create(*TheContext, "loopcond",
+      TheFunction);
+  BasicBlock *LoopBB = BasicBlock::Create(*TheContext, "loop");
+  BasicBlock *EndLoopBB = BasicBlock::Create(*TheContext, "endloop");
 
-  // Start insertion in LoopBB.
-  Builder->SetInsertPoint(LoopBB);
+  // Insert an explicit fall through from current block to LoopConditionBB.
+  Builder->CreateBr(LoopConditionBB);
 
-  // Within the loop, the variable is defined equal to the PHI node.  If it
-  // shadows an existing variable, we have to restore it, so save it now.
-  AllocaInst *OldVal = NamedValues[VarName];
-  NamedValues[VarName] = Alloca;
+  // Start insertion in LoopConditionBB.
+  Builder->SetInsertPoint(LoopConditionBB);
+
+  // Compute the end condition.
+  Value *EndCond = End->codegen();
+  if (!EndCond)
+    return nullptr;
 
-  // Emit the body of the loop.  This, like any other expr, can change the
-  // current BB.  Note that we ignore the value computed by the body, but don't
-  // allow an error.
-  if (!Body->codegen())
+  // Convert condition to a bool by comparing non-equal to 0.0.
+  EndCond = Builder->CreateFCmpONE(
+      EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "endcond");
+
+  // Insert the conditional branch that either continues the loop, or exits the
+  // loop.
+  Builder->CreateCondBr(EndCond, LoopBB, EndLoopBB);
+
+  // Attach the basic block that will soon hold the loop body to the end of the
+  // parent function.
+  TheFunction->insert(TheFunction->end(), LoopBB);
+
+  // Emit the loop body within the LoopBB. This, like any other expr, can change
+  // the current BB. Note that we ignore the value computed by the body, but
+  // don't allow an error.
+  Builder->SetInsertPoint(LoopBB);
+  if (!Body->codegen()) {
     return nullptr;
+  }
 
   // Emit the step value.
   Value *StepVal = nullptr;
@@ -1125,31 +1147,22 @@ Value *ForExprAST::codegen() {
     StepVal = ConstantFP::get(*TheContext, APFloat(1.0));
   }
 
-  // Compute the end condition.
-  Value *EndCond = End->codegen();
-  if (!EndCond)
-    return nullptr;
-
-  // Reload, increment, and restore the alloca.  This handles the case where
-  // the body of the loop mutates the variable.
-  Value *CurVar = Builder->CreateLoad(Type::getDoubleTy(*TheContext), Alloca,
-                                      VarName.c_str());
+  // Load, increment and store the new loop variable.
+  Value *CurVar = Builder->CreateLoad(Alloca->getAllocatedType(), Alloca,
+                                      VarName);
   Value *NextVar = Builder->CreateFAdd(CurVar, StepVal, "nextvar");
   Builder->CreateStore(NextVar, Alloca);
 
-  // Convert condition to a bool by comparing non-equal to 0.0.
-  EndCond = Builder->CreateFCmpONE(
-      EndCond, ConstantFP::get(*TheContext, APFloat(0.0)), "loopcond");
-
-  // Create the "after loop" block and insert it.
-  BasicBlock *AfterBB =
-      BasicBlock::Create(*TheContext, "afterloop", TheFunction);
+  // Create the unconditional branch that returns to LoopConditionBB to
+  // determine if we should continue looping.
+  Builder->CreateBr(LoopConditionBB);
 
-  // Insert the conditional branch into the end of LoopEndBB.
-  Builder->CreateCondBr(EndCond, LoopBB, AfterBB);
+  // Append EndLoopBB after the loop body. We go to this basic block if the
+  // loop condition says we should not loop anymore.
+  TheFunction->insert(TheFunction->end(), EndLoopBB);
 
-  // Any new code will be inserted in AfterBB.
-  Builder->SetInsertPoint(AfterBB);
+  // Any new code will be inserted after the loop.
+  Builder->SetInsertPoint(EndLoopBB);
 
   // Restore the unshadowed variable.
   if (OldVal)



More information about the llvm-commits mailing list