bufixes with goto-successors, empty block removal

src/main/java/codegen/flowgraph/FlowBasicBlock.java

@@ -29,8 +29,12 @@ public class FlowBasicBlock {
         this("");
     }
 
+    /**
+     * Ermittelt ob ein BasicBlock ohne weiteres entfernbar ist.
+     * Der Block darf kein Label haben, damit keine Sprünge ins Leere passieren.
+     */
     public boolean isEmpty() {
-        return this.label.isBlank() && this.instructions.isEmpty();
+        return this.instructions.isEmpty() && this.label.isBlank();
     }
 
     public String getId() {

src/main/java/codegen/flowgraph/FlowGraph.java

@@ -1,19 +1,23 @@
 package codegen.flowgraph;
 
+import util.Logger;
+
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 public class FlowGraph {
 
     private final FlowGraphHead head;
-    private final List<FlowBasicBlock> blocks;
+    private final List<FlowBasicBlock> basicBlocks;
     private final FlowGraphTail tail;
 
     // If a new block has this label, the value in this map is a predecessor
@@ -21,7 +25,7 @@ public class FlowGraph {
 
     public FlowGraph(String bytecodeVersion, String source, String clazz, int stackSize, int localCount) {
         this.head = new FlowGraphHead(bytecodeVersion, source, clazz, stackSize, localCount);
-        this.blocks = new ArrayList<>();
+        this.basicBlocks = new ArrayList<>();
         this.tail = new FlowGraphTail();
         this.predecessorMap = new HashMap<>();
     }
@@ -35,46 +39,24 @@ public class FlowGraph {
         if (this.predecessorMap.containsKey(label)) {
             this.predecessorMap.get(label).addSuccessor(newBlock);
             newBlock.addPredecessor(this.predecessorMap.get(label));
-
-            // this.predecessorMap.remove(label); // Problematic if multiple gotos to same label
         }
 
-        /*
+        newBlock.addPredecessor(this.getCurrentBlock()); // Obvious predecessor of new block
-        TODO: Hier ist ein Bug, welcher im Datenflussgraph zu gotos mit 2 successors führt
+        this.getCurrentBlock().addSuccessor(newBlock); // Obvious successor of current block
-        if (this.getCurrentBlock().isEmpty()) {
-            // Replace empty blocks, we don't need them
 
-            if (this.blocks.size() >= 2) {
+        this.basicBlocks.add(newBlock);
-                // This empty blocks successors become the previous blocks successors after replacment
-
-                this.blocks.get(this.blocks.size() - 2).addSuccessors(this.getCurrentBlock().getSuccessorSet());
-            }
-
-            // Previous blocks predecessors are also the new blocks predecessors
-            newBlock.addPredecessors(this.getCurrentBlock().getPredecessorSet());
-
-            this.blocks.set(this.blocks.size() - 1, newBlock); // Replace
-        } else {
-        */
-            // Append block if last one isn't empty
-
-            newBlock.addPredecessor(this.getCurrentBlock()); // Obvious predecessor of new block
-            this.getCurrentBlock().addSuccessor(newBlock); // Obvious successor of current block
-
-            this.blocks.add(newBlock);
-        //        }
     }
 
     // Jump means end of block
     public void addJump(String jumpInstruction, String label) {
-        this.addInst(jumpInstruction, label);
+        this.addInstruction(jumpInstruction, label);
 
         final FlowBasicBlock newBlock = new FlowBasicBlock();
-        newBlock.addPredecessor(this.getCurrentBlock()); // Obvious predecessor of new block
 
         if (!"goto".equals(jumpInstruction)) {
             // Goto always jumps
 
+            newBlock.addPredecessor(this.getCurrentBlock()); // Obvious predecessor of new block
             this.getCurrentBlock().addSuccessor(newBlock); // Obvious successor of current block
         }
 
@@ -91,22 +73,80 @@ public class FlowGraph {
             this.predecessorMap.put(label, this.getCurrentBlock()); // Current node is predecessor of label-block
         }
 
-        this.blocks.add(newBlock);
+        this.basicBlocks.add(newBlock);
     }
 
-    public void addInst(String instruction, String... args) {
+    public void addInstruction(String instruction, String... args) {
-        if (this.blocks.isEmpty()) {
+        if (this.basicBlocks.isEmpty()) {
-            this.blocks.add(new FlowBasicBlock()); // First block doesn't exist
+            this.basicBlocks.add(new FlowBasicBlock("START")); // First block doesn't exist
         }
 
         this.getCurrentBlock().addInstruction(instruction, args); // Add to last block
     }
 
+    /**
+     * Entfernt leere Blöcke.
+     */
+    public void purgeEmptyBlocks() {
+        Logger.log("\nPurging empty blocks: ");
+
+        final Set<FlowBasicBlock> toRemove = new HashSet<>();
+
+        // Collect removable blocks
+        for (FlowBasicBlock block : this.basicBlocks) {
+            if (block.isEmpty()) {
+                Logger.log("Marking Block " + this.basicBlocks.indexOf(block) + " as removable.");
+                toRemove.add(block);
+            }
+        }
+
+        // Remove blocks + reroute predecessors/successors
+        for (FlowBasicBlock block : toRemove) {
+
+            for (FlowBasicBlock pred : block.getPredecessorSet()) {
+
+                for (FlowBasicBlock succ : block.getSuccessorSet()) {
+
+                    Logger.log("Rerouting Block " + this.basicBlocks.indexOf(pred) + " to Block " + this.basicBlocks.indexOf(succ));
+                    pred.addSuccessor(succ);
+                    succ.addPredecessor(pred);
+                }
+
+                pred.getSuccessorSet().remove(block);
+            }
+
+            for (FlowBasicBlock succ : block.getSuccessorSet()) {
+                succ.getPredecessorSet().remove(block);
+            }
+        }
+
+        this.basicBlocks.removeAll(toRemove);
+    }
+
+    // Getters, Setters
+
+    private FlowBasicBlock getBlockByLabel(String label) {
+        return this.basicBlocks.stream()
+                               .filter(block -> block.getLabel().equals(label))
+                               .findFirst()
+                               .orElse(null);
+    }
+
+    private FlowBasicBlock getCurrentBlock() {
+        return this.basicBlocks.get(this.basicBlocks.size() - 1);
+    }
+
+    public List<FlowBasicBlock> getBasicBlocks() {
+        return this.basicBlocks;
+    }
+
+    // Print + Overrides
+
     public String print() {
-        final String blocksString = this.blocks.stream()
+        final String blocksString = this.basicBlocks.stream()
-                                               .map(FlowBasicBlock::toString)
+                                                    .map(FlowBasicBlock::toString)
-                                               .map(string -> string + "-".repeat(50) + "\n")
+                                                    .map(string -> string + "-".repeat(50) + "\n")
-                                               .collect(Collectors.joining());
+                                                    .collect(Collectors.joining());
 
         return this.head + "-".repeat(100) + "\n"
                + "-".repeat(50) + "\n" + blocksString + "-".repeat(100) + "\n"
@@ -119,10 +159,10 @@ public class FlowGraph {
         dot.append("digraph dfd {\n")
            .append("node[shape=Mrecord]\n");
 
-        for (FlowBasicBlock block : this.blocks) {
+        for (FlowBasicBlock block : this.basicBlocks) {
             dot.append(block.getId())
                .append(" [label=\"{<f0> ")
-               .append(this.blocks.indexOf(block))
+               .append(this.basicBlocks.indexOf(block))
                .append(": ")
                .append(block.getLabel())
                .append("|<f1> ")
@@ -133,13 +173,23 @@ public class FlowGraph {
         dot.append("START[label=\"START\"];\n")
            .append("END[label=\"END\"];\n");
 
-        dot.append("START -> ").append(this.blocks.get(0).getId()).append(";\n");
+        dot.append("START -> ").append(this.basicBlocks.get(0).getId()).append(";\n");
         dot.append(this.getCurrentBlock().getId()).append(" -> END;\n");
 
-        for (FlowBasicBlock block : this.blocks) {
+        for (FlowBasicBlock block : this.basicBlocks) {
+            // Successors
             for (FlowBasicBlock succ : block.getSuccessorSet()) {
                 dot.append(block.getId()).append(" -> ").append(succ.getId()).append(";\n");
             }
+
+            // Predecessors
+            for (FlowBasicBlock pred : block.getPredecessorSet()) {
+                if (!dot.toString().contains(pred.getId() + " -> " + block.getId())) {
+                    // No duplicate arrows
+
+                    dot.append(pred.getId()).append(" -> ").append(block.getId()).append(";\n");
+                }
+            }
         }
 
         dot.append("}");
@@ -165,34 +215,13 @@ public class FlowGraph {
 
     @Override
     public String toString() {
-        final String blocksString = this.blocks.stream()
+        final String blocksString = this.basicBlocks.stream()
-                                               .map(FlowBasicBlock::toString)
+                                                    .map(FlowBasicBlock::toString)
-                                               .collect(Collectors.joining());
+                                                    .collect(Collectors.joining());
 
         return this.head
                + blocksString
                + this.tail;
     }
 
-    private FlowBasicBlock getBlockByLabel(String label) {
-        return this.blocks.stream()
-                          .filter(block -> block.getLabel().equals(label))
-                          .findFirst()
-                          .orElse(null);
-    }
-
-    private FlowBasicBlock getCurrentBlock() {
-        return this.blocks.get(this.blocks.size() - 1);
-    }
-
-    public List<FlowBasicBlock> getBlocks() {
-        return this.blocks;
-    }
-
-    public FlowBasicBlock getBlockById(String id) {
-        return this.blocks.stream()
-                          .filter(block -> block.getId().equals(id))
-                          .findFirst()
-                          .orElse(null);
-    }
 }

src/main/java/codegen/flowgraph/FlowGraphGenerator.java

@@ -17,11 +17,15 @@ import java.util.Map;
 import static java.util.Map.entry;
 import static util.Logger.log;
 
+/**
+ * Erzeugt den SourceCode in FlussGraph-Darstellung.
+ */
 public final class FlowGraphGenerator {
 
     private static final Map<String, Method> methodMap;
 
     static {
+        // Init the method mappings: ASTNode.getName() -> FlowGraphGenerator.method
         Map<String, Method> map;
         try {
             final Class<?> gen = FlowGraphGenerator.class;
@@ -76,7 +80,7 @@ public final class FlowGraphGenerator {
     private static Map<String, Integer> initVarMap(AST tree) {
         final Map<String, Integer> varMap = new HashMap<>();
 
-        // Assign variables to map
+        // Assign variables to map: Symbol -> jasminLocalVarNr.
         int varCount = 0;
         final Deque<ASTNode> stack = new ArrayDeque<>();
         stack.push(tree.getRoot());
@@ -97,14 +101,17 @@ public final class FlowGraphGenerator {
         return Collections.unmodifiableMap(varMap);
     }
 
-    public Map<String, Integer> getVarMap() {
+    /**
-        return this.varMap;
+     * Erzeugt den Flussgraphen für den gespeicherten AST.
-    }
+     * Der Flussgraph ist dabei die Graphenform des generierten SourceCodes:
-
+     * Die Instruktionen sind unterteilt in BasicBlocks, welche über Kanten verbunden sind.
+     */
     public FlowGraph generateGraph() {
         System.out.println(" - Generating Source Graph...");
 
+        // Skip the first 2 identifiers: ClassName, MainArgs
         this.generateNode(this.tree.getRoot().getChildren().get(3).getChildren().get(11));
+        this.graph.purgeEmptyBlocks();
         Logger.call(this.graph::printToImage);
 
         log("\n\nSourceGraph print:\n" + "-".repeat(100) + "\n" + this.graph.print() + "-".repeat(100));
@@ -113,137 +120,168 @@ public final class FlowGraphGenerator {
         return this.graph;
     }
 
-    private void generateNode(ASTNode node) {
+    /**
-        if (methodMap.containsKey(node.getName())) {
+     * Erzeugt den FlussGraphen für die angegebene Wurzel.
+     * Der Wurzelname wird über die methodMap einer Methode zugewiesen.
+     * Diese wird aufgerufen und erzeugt den entsprechenden Teilbaum.
+     */
+    private void generateNode(ASTNode root) {
+        if (methodMap.containsKey(root.getName())) {
             try {
-                methodMap.get(node.getName()).invoke(this, node);
+                methodMap.get(root.getName()).invoke(this, root);
             } catch (IllegalAccessException | InvocationTargetException e) {
                 e.printStackTrace();
             }
         } else {
-            node.getChildren().forEach(this::generateNode);
+            root.getChildren().forEach(this::generateNode);
         }
     }
 
     // ifeq - if value is 0
     // ifne - if value is not 0
-    private void condNode(ASTNode node) {
+
+    /**
+     * Erzeugt den Teilbaum für einen If-Knoten.
+     */
+    private void condNode(ASTNode root) {
         final int currentLabel = this.labelCounter;
         this.labelCounter++;
 
-        // Condition
+        // Condition If ( ... ) {
-        this.generateNode(node.getChildren().get(0));
+        this.generateNode(root.getChildren().get(0));
 
-        // Jump
+        // Jump if condition false
         this.graph.addJump("ifeq", "IFfalse" + currentLabel);
 
-        // IFtrue branch
+        // IFtrue branch (gets executed without jump)
-        this.generateNode(node.getChildren().get(1));
+        this.generateNode(root.getChildren().get(1));
-        this.graph.addJump("goto", "IFend" + currentLabel);
+        this.graph.addJump("goto", "IFend" + currentLabel); // Skip IFfalse branch
 
-        // IFfalse branch
+        // IFfalse branch (gets executed after jump)
         this.graph.addLabel("IFfalse" + currentLabel);
-        if (node.getChildren().size() == 3) {
+        if (root.getChildren().size() == 3) {
             // Else exists
 
-            this.generateNode(node.getChildren().get(2));
+            this.generateNode(root.getChildren().get(2));
         }
 
         // IFend branch
         this.graph.addLabel("IFend" + currentLabel);
     }
 
-    private void loopNode(ASTNode node) {
+    /**
+     * Erzeugt den Teilbaum für einen While-Knoten.
+     */
+    private void loopNode(ASTNode root) {
         final int currentLabel = this.labelCounter;
         this.labelCounter++;
 
+        // LOOPstart label for loop repetition
         this.graph.addLabel("LOOPstart" + currentLabel);
 
-        // Condition
+        // Condition while ( ... ) {
-        this.generateNode(node.getChildren().get(0).getChildren().get(1));
+        this.generateNode(root.getChildren().get(0).getChildren().get(1));
 
-        // Jump
+        // Jump out of loop if condition is false
         this.graph.addJump("ifeq", "LOOPend" + currentLabel);
 
-        // Loop body
+        // Loop body (gets executed without jump)
-        this.generateNode(node.getChildren().get(1));
+        this.generateNode(root.getChildren().get(1));
-        this.graph.addJump("goto", "LOOPstart" + currentLabel);
+        this.graph.addJump("goto", "LOOPstart" + currentLabel); // Repeat loop
 
         // Loop end
         this.graph.addLabel("LOOPend" + currentLabel);
     }
 
-    private void assignNode(ASTNode node) { //! Stack - 1
+    /**
-        this.generateNode(node.getChildren().get(0));
+     * Erzeugt den Teilbaum für Assignment-Knoten.
+     * Die JVM-Stacksize wird dabei um 1 verringert, da istore/astore 1 Argument konsumieren.
+     */
+    private void assignNode(ASTNode root) { //! Stack - 1
+        this.generateNode(root.getChildren().get(0));
 
-        final String type = this.nodeTypeMap.get(node.getChildren().get(0));
+        final String type = this.nodeTypeMap.get(root.getChildren().get(0));
         final String inst = switch (type) {
             case "INTEGER_TYPE", "BOOLEAN_TYPE" -> "istore";
             case "STRING_TYPE" -> "astore";
             default -> throw new IllegalStateException("Unexpected value: " + type);
         };
 
-        log("assign(): " + node.getName() + ": " + node.getValue() + " => " + inst);
+        log("assign(): " + root.getName() + ": " + root.getValue() + " => " + inst);
 
-        this.graph.addInst(inst, this.varMap.get(node.getValue()).toString());
+        this.graph.addInstruction(inst, this.varMap.get(root.getValue()).toString());
     }
 
-    private void exprNode(ASTNode node) {
+    /**
-        if ("INTEGER_TYPE".equals(this.nodeTypeMap.get(node))) {
+     * Wählt die entsprechende Methode für mathematische oder logische Ausdrücke.
-            this.intExpr(node);
+     */
-        } else if ("BOOLEAN_TYPE".equals(this.nodeTypeMap.get(node))) {
+    private void exprNode(ASTNode root) {
-            this.boolExpr(node);
+        if ("INTEGER_TYPE".equals(this.nodeTypeMap.get(root))) {
+            this.intExpr(root);
+        } else if ("BOOLEAN_TYPE".equals(this.nodeTypeMap.get(root))) {
+            this.boolExpr(root);
         }
     }
 
-    private void intExpr(ASTNode node) {
+    /**
+     * Erzeugt den Teilbaum für mathematische Ausdrücke.
+     * Bei unären Operatoren bleibt die Stackgröße konstant (1 konsumiert, 1 Ergebnis),
+     * bei binären Operatoren sinkt die Stackgröße um 1 (2 konsumiert, 1 Ergebnis).
+     */
+    private void intExpr(ASTNode root) {
         String inst = "";
 
-        if (node.getChildren().size() == 1) { //! Stack + 0
+        if (root.getChildren().size() == 1) { //! Stack + 0
             // Unary operator
 
-            this.generateNode(node.getChildren().get(0));
+            this.generateNode(root.getChildren().get(0));
 
-            inst = switch (node.getValue()) {
+            inst = switch (root.getValue()) {
                 case "ADD" -> "";
                 case "SUB" -> "ineg";
-                default -> throw new IllegalStateException("Unexpected value: " + node.getValue());
+                default -> throw new IllegalStateException("Unexpected value: " + root.getValue());
             };
-        } else if (node.getChildren().size() == 2) { //! Stack - 1
+        } else if (root.getChildren().size() == 2) { //! Stack - 1
             // Binary operator
 
-            this.generateNode(node.getChildren().get(0));
+            this.generateNode(root.getChildren().get(0));
-            this.generateNode(node.getChildren().get(1));
+            this.generateNode(root.getChildren().get(1));
 
-            inst = switch (node.getValue()) {
+            inst = switch (root.getValue()) {
                 case "ADD" -> "iadd"; // Integer
                 case "SUB" -> "isub";
                 case "MUL" -> "imul";
                 case "DIV" -> "idiv";
                 case "MOD" -> "irem"; // Remainder operator
-                default -> throw new IllegalStateException("Unexpected value: " + node.getValue());
+                default -> throw new IllegalStateException("Unexpected value: " + root.getValue());
             };
         }
 
-        log("intExpr(): " + node.getName() + ": " + node.getValue() + " => " + inst);
+        log("intExpr(): " + root.getName() + ": " + root.getValue() + " => " + inst);
 
-        this.graph.addInst(inst);
+        this.graph.addInstruction(inst);
     }
 
+    /**
+     * Erzeugt den Teilbaum für logische Ausdrücke.
+     * Bei unären Operatoren wächst der Stack temporär um 1 (NOT pusht eine 1 für xor),
+     * bei binären Operatoren sinkt die Stackgröße um 1 (2 konsumiert, 1 Ergebnis).
+     */
     private void boolExpr(ASTNode node) {
         if (node.getChildren().size() == 1) { //! Stack + 1
             // Unary operator
 
             if (!"NOT".equals(node.getValue())) {
-                // Diese Möglichkeit gibts eigentlich nicht
+                // Possibility doesn't exist, would be frontend-error
+
                 throw new IllegalStateException("Unexpected value: " + node.getValue());
             }
 
             this.generateNode(node.getChildren().get(0));
 
-            // 0  ^1 = 1, 1  ^1 = 0
+            // 0 xor 1 = 1, 1 xor 1 = 0 => not
-            this.graph.addInst("ldc", "1");
+            this.graph.addInstruction("ldc", "1");
-            this.graph.addInst("ixor");
+            this.graph.addInstruction("ixor");
 
         } else if (node.getChildren().size() == 2) { //! Stack - 1
             // Binary operator
@@ -266,9 +304,10 @@ public final class FlowGraphGenerator {
                 default -> throw new IllegalStateException("Unexpected value: " + type);
             };
 
+            // The comparison operations need to jump
             switch (node.getValue()) {
-                case "AND" -> this.graph.addInst("iand"); // Boolean
+                case "AND" -> this.graph.addInstruction("iand"); // Boolean
-                case "OR" -> this.graph.addInst("ior");
+                case "OR" -> this.graph.addInstruction("ior");
                 case "EQUAL" -> this.genComparisonInst(cmpeq, "EQ", currentLabel);
                 case "NOT_EQUAL" -> this.genComparisonInst(cmpne, "NE", currentLabel);
                 case "LESS" -> this.genComparisonInst("if_icmplt", "LT", currentLabel);
@@ -280,12 +319,19 @@ public final class FlowGraphGenerator {
         }
     }
 
+    /**
+     * Erzeugt die Instruktionen für eine Vergleichsoperation.
+     *
+     * @param cmpInst      Die Vergleichsanweisung
+     * @param labelPre     Das Labelpräfix, abhängig von der Art des Vergleichs
+     * @param currentLabel Der aktuelle Labelcounter
+     */
     private void genComparisonInst(String cmpInst, String labelPre, int currentLabel) {
         this.graph.addJump(cmpInst, labelPre + "true" + currentLabel); // If not equal jump to NEtrue
-        this.graph.addInst("ldc", "0"); // If false load 0
+        this.graph.addInstruction("ldc", "0"); // If false load 0
         this.graph.addJump("goto", labelPre + "end" + currentLabel); // If false skip to true
         this.graph.addLabel(labelPre + "true" + currentLabel);
-        this.graph.addInst("ldc", "1"); // If true load 1
+        this.graph.addInstruction("ldc", "1"); // If true load 1
         this.graph.addLabel(labelPre + "end" + currentLabel);
     }
 
@@ -295,7 +341,7 @@ public final class FlowGraphGenerator {
         log("intStringLiteral(): " + node.getName() + ": " + node.getValue() + " => ldc");
 
         // bipush only pushes 1 byte as int
-        this.graph.addInst("ldc", node.getValue());
+        this.graph.addInstruction("ldc", node.getValue());
     }
 
     private void boolLiteralNode(ASTNode node) { //! Stack + 1
@@ -303,7 +349,7 @@ public final class FlowGraphGenerator {
 
         final String val = "true".equals(node.getValue()) ? "1" : "0";
 
-        this.graph.addInst("ldc", val);
+        this.graph.addInstruction("ldc", val);
     }
 
     private void identifierNode(ASTNode node) { //! Stack + 1
@@ -316,11 +362,11 @@ public final class FlowGraphGenerator {
 
         log("identifier(): " + node.getName() + ": " + node.getValue() + " => " + inst);
 
-        this.graph.addInst(inst, this.varMap.get(node.getValue()).toString());
+        this.graph.addInstruction(inst, this.varMap.get(node.getValue()).toString());
     }
 
     private void printlnNode(ASTNode node) { //! Stack + 1
-        this.graph.addInst("getstatic", "java/lang/System/out", "Ljava/io/PrintStream;");
+        this.graph.addInstruction("getstatic", "java/lang/System/out", "Ljava/io/PrintStream;");
 
         final ASTNode expr = node.getChildren().get(1).getChildren().get(1);
         final String type = switch (this.nodeTypeMap.get(expr)) {
@@ -334,6 +380,12 @@ public final class FlowGraphGenerator {
 
         log("println(): " + expr.getName() + ": " + expr.getValue() + " => " + type);
 
-        this.graph.addInst("invokevirtual", "java/io/PrintStream/println(" + type + ")V");
+        this.graph.addInstruction("invokevirtual", "java/io/PrintStream/println(" + type + ")V");
+    }
+
+    // Getters, Setters
+
+    public Map<String, Integer> getVarMap() {
+        return this.varMap;
     }
 }