Avoids swap problem when eliminating phi functions and therefore gets

back global value numbering optimization

teavm-core/src/main/java/org/teavm/model/util/GraphColorer.java

@@ -15,9 +15,7 @@
  */
 package org.teavm.model.util;
 
-import java.util.ArrayList;
 import java.util.BitSet;
-import java.util.List;
 import org.teavm.common.Graph;
 
 /**
@@ -25,42 +23,20 @@ import org.teavm.common.Graph;
  * @author Alexey Andreev
  */
 class GraphColorer {
-    public void colorize(Graph graph, int[] classes, int[] colors) {
+    public void colorize(Graph graph, int[] colors) {
-        List<List<Integer>> classMembers = new ArrayList<>();
-        for (int i = 0; i < classes.length; ++i) {
-            int cls = classes[i];
-            while (cls >= classMembers.size()) {
-                classMembers.add(new ArrayList<Integer>());
-            }
-            classMembers.get(cls).add(i);
-        }
-        for (int i = 0; i < colors.length; ++i) {
-            if (colors[i] >= 0) {
-                int cls = classes[i];
-                for (int member : classMembers.get(cls)) {
-                    colors[member] = colors[i];
-                }
-            }
-        }
         BitSet usedColors = new BitSet();
         for (int v : getOrdering(graph)) {
             if (colors[v] >= 0) {
                 continue;
             }
-            int cls = classes[v];
             usedColors.clear();
             usedColors.set(0);
-            for (int member : classMembers.get(cls)) {
+            for (int succ : graph.outgoingEdges(v)) {
-                for (int succ : graph.outgoingEdges(member)) {
                 if (colors[succ] >= 0) {
                     usedColors.set(colors[succ]);
                 }
             }
-            }
+            colors[v] = usedColors.nextClearBit(0);
-            int color = usedColors.nextClearBit(0);
-            for (int member : classMembers.get(cls)) {
-                colors[member] = color;
-            }
         }
     }
 

teavm-core/src/main/java/org/teavm/model/util/MutableGraphEdge.java

@@ -57,4 +57,9 @@ class MutableGraphEdge {
     public MutableGraphEdge getBack() {
         return back;
     }
+
+    @Override
+    public String toString() {
+        return String.valueOf(second.getTag());
+    }
 }

teavm-core/src/main/java/org/teavm/model/util/RegisterAllocator.java

@@ -18,6 +18,7 @@ package org.teavm.model.util;
 import java.util.*;
 import org.teavm.common.DisjointSet;
 import org.teavm.common.Graph;
+import org.teavm.common.GraphBuilder;
 import org.teavm.model.*;
 import org.teavm.model.instructions.AssignInstruction;
 import org.teavm.model.instructions.EmptyInstruction;
@@ -29,35 +30,28 @@ import org.teavm.model.instructions.JumpInstruction;
  */
 public class RegisterAllocator {
     public void allocateRegisters(MethodReader method, Program program) {
-        List<PhiArgumentCopy> phiArgsCopies = insertPhiArgumentsCopies(program);
+        insertPhiArgumentsCopies(program);
         InterferenceGraphBuilder interferenceBuilder = new InterferenceGraphBuilder();
         LivenessAnalyzer liveness = new LivenessAnalyzer();
         liveness.analyze(program);
         Graph interferenceGraph = interferenceBuilder.build(program, method.parameterCount(), liveness);
         DisjointSet congruenceClasses = buildPhiCongruenceClasses(program);
         List<MutableGraphNode> classInterferenceGraph = makeMutableGraph(interferenceGraph, congruenceClasses);
-        removeRedundantCopies(program, phiArgsCopies, classInterferenceGraph, congruenceClasses);
+        removeRedundantCopies(program, classInterferenceGraph, congruenceClasses);
         int[] classArray = congruenceClasses.pack(program.variableCount());
+        renameVariables(program, classArray);
         int[] colors = new int[program.variableCount()];
         Arrays.fill(colors, -1);
         for (int i = 0; i <= method.parameterCount(); ++i) {
             colors[i] = i;
         }
         GraphColorer colorer = new GraphColorer();
-        colorer.colorize(interferenceGraph, classArray, colors);
+        colorer.colorize(renameInterferenceGraph(interferenceGraph, classArray), colors);
         for (int i = 0; i < colors.length; ++i) {
             program.variableAt(i).setRegister(colors[i]);
         }
     }
 
-    private static class PhiArgumentCopy {
-        Incoming incoming;
-        int original;
-        int index;
-        BasicBlock block;
-        int var;
-    }
-
     private static List<MutableGraphNode> makeMutableGraph(Graph graph, DisjointSet classes) {
         List<MutableGraphNode> mutableGraph = new ArrayList<>();
         for (int i = 0; i < graph.size(); ++i) {
@@ -77,32 +71,37 @@ public class RegisterAllocator {
         return mutableGraph;
     }
 
-    private List<PhiArgumentCopy> insertPhiArgumentsCopies(Program program) {
+    private void insertPhiArgumentsCopies(Program program) {
-        List<PhiArgumentCopy> copies = new ArrayList<>();
         for (int i = 0; i < program.basicBlockCount(); ++i) {
             Map<BasicBlock, BasicBlock> blockMap = new HashMap<>();
-            for (final Phi phi : program.basicBlockAt(i).getPhis()) {
+            for (Phi phi : program.basicBlockAt(i).getPhis()) {
                 for (Incoming incoming : phi.getIncomings()) {
-                    PhiArgumentCopy copy = new PhiArgumentCopy();
+                    insertCopy(incoming, blockMap);
-                    copy.incoming = incoming;
+                }
-                    copy.original = incoming.getValue().getIndex();
+            }
+            for (Phi phi : program.basicBlockAt(i).getPhis()) {
+                for (Incoming incoming : phi.getIncomings()) {
+                    insertCopy(incoming, blockMap);
+                }
+            }
+        }
+    }
+
+    private void insertCopy(Incoming incoming, Map<BasicBlock, BasicBlock> blockMap) {
+        final Phi phi = incoming.getPhi();
+        Program program = phi.getBasicBlock().getProgram();
         AssignInstruction copyInstruction = new AssignInstruction();
-                    copyInstruction.setReceiver(program.createVariable());
+        Variable firstCopy = program.createVariable();
+        copyInstruction.setReceiver(firstCopy);
         copyInstruction.setAssignee(incoming.getValue());
-                    copy.var = copyInstruction.getReceiver().getIndex();
         BasicBlock source = blockMap.get(incoming.getSource());
         if (source == null) {
             source = incoming.getSource();
         } else {
             incoming.setSource(source);
         }
-                    if (incoming.getSource().getLastInstruction() instanceof JumpInstruction) {
+        if (!(incoming.getSource().getLastInstruction() instanceof JumpInstruction)) {
-                        copy.index = incoming.getSource().getInstructions().size() - 1;
-                        copy.block = incoming.getSource();
-                        copy.block.getInstructions().add(copy.index, copyInstruction);
-                    } else {
             final BasicBlock copyBlock = program.createBasicBlock();
-                        copyBlock.getInstructions().add(copyInstruction);
             JumpInstruction jumpInstruction = new JumpInstruction();
             jumpInstruction.setTarget(phi.getBasicBlock());
             copyBlock.getInstructions().add(jumpInstruction);
@@ -117,42 +116,42 @@ public class RegisterAllocator {
             });
             blockMap.put(source, copyBlock);
             incoming.setSource(copyBlock);
-                        copy.index = 0;
+            source = copyBlock;
-                        copy.block = incoming.getSource();
         }
+        source.getInstructions().add(source.getInstructions().size() - 1, copyInstruction);
         incoming.setValue(copyInstruction.getReceiver());
-                    copies.add(copy);
-                }
-            }
-        }
-        return copies;
     }
 
-    private void removeRedundantCopies(Program program, List<PhiArgumentCopy> copies,
+    private void removeRedundantCopies(Program program, List<MutableGraphNode> interferenceGraph,
-            List<MutableGraphNode> interferenceGraph, DisjointSet congruenceClasses) {
+            DisjointSet congruenceClasses) {
-        for (PhiArgumentCopy copy : copies) {
+        for (int i = 0; i < program.basicBlockCount(); ++i) {
+            BasicBlock block = program.basicBlockAt(i);
+            for (int j = 0; j < block.getInstructions().size(); ++j) {
+                Instruction insn = block.getInstructions().get(j);
+                if (!(insn instanceof AssignInstruction)) {
+                    continue;
+                }
+                AssignInstruction assignment = (AssignInstruction)insn;
                 boolean interfere = false;
-            int varClass = congruenceClasses.find(copy.var);
+                int copyClass = congruenceClasses.find(assignment.getReceiver().getIndex());
-            int origClass = congruenceClasses.find(copy.original);
+                int origClass = congruenceClasses.find(assignment.getAssignee().getIndex());
-            for (MutableGraphEdge edge : interferenceGraph.get(copy.original).getEdges()) {
+                for (MutableGraphEdge edge : interferenceGraph.get(origClass).getEdges()) {
                     if (edge.getFirst() == edge.getSecond()) {
                         continue;
                     }
                     int neighbour = congruenceClasses.find(edge.getSecond().getTag());
-                if (neighbour == varClass || neighbour == origClass) {
+                    if (neighbour == copyClass || neighbour == origClass) {
                         interfere = true;
                         break;
                     }
                 }
                 if (!interfere) {
-                int newClass = congruenceClasses.union(varClass, origClass);
+                    int newClass = congruenceClasses.union(copyClass, origClass);
-                copy.block.getInstructions().set(copy.index, new EmptyInstruction());
+                    block.getInstructions().set(j, new EmptyInstruction());
-                copy.incoming.setValue(program.variableAt(copy.original));
+                    MutableGraphNode newNode = interferenceGraph.get(origClass);
-                for (MutableGraphEdge edge : interferenceGraph.get(varClass).getEdges()
+                    if (newClass == interferenceGraph.size()) {
-                        .toArray(new MutableGraphEdge[0])) {
+                        newNode = new MutableGraphNode(interferenceGraph.size());
-                    if (edge.getFirst() != null) {
+                        interferenceGraph.add(newNode);
-                        edge.setFirst(interferenceGraph.get(newClass));
-                    }
                     }
                     for (MutableGraphEdge edge : interferenceGraph.get(origClass).getEdges()
                             .toArray(new MutableGraphEdge[0])) {
@@ -160,9 +159,46 @@ public class RegisterAllocator {
                             edge.setFirst(interferenceGraph.get(newClass));
                         }
                     }
+                    for (MutableGraphEdge edge : interferenceGraph.get(copyClass).getEdges()
+                            .toArray(new MutableGraphEdge[0])) {
+                        if (edge.getFirst() != null) {
+                            edge.setFirst(interferenceGraph.get(newClass));
                         }
                     }
                 }
+            }
+        }
+    }
+
+    private void renameVariables(final Program program, final int[] varMap) {
+        InstructionVariableMapper mapper = new InstructionVariableMapper() {
+            @Override protected Variable map(Variable var) {
+                return program.variableAt(varMap[var.getIndex()]);
+            }
+        };
+        for (int i = 0; i < program.basicBlockCount(); ++i) {
+            BasicBlock block = program.basicBlockAt(i);
+            for (Instruction insn : block.getInstructions()) {
+                insn.acceptVisitor(mapper);
+            }
+            for (Phi phi : block.getPhis()) {
+                phi.setReceiver(program.variableAt(varMap[phi.getReceiver().getIndex()]));
+                for (Incoming incoming : phi.getIncomings()) {
+                    incoming.setValue(program.variableAt(varMap[incoming.getValue().getIndex()]));
+                }
+            }
+        }
+    }
+
+    private Graph renameInterferenceGraph(Graph graph, final int[] varMap) {
+        GraphBuilder renamedGraph = new GraphBuilder();
+        for (int i = 0; i < graph.size(); ++i) {
+            for (int j : graph.outgoingEdges(i)) {
+                renamedGraph.addEdge(varMap[i], varMap[j]);
+            }
+        }
+        return renamedGraph.build();
+    }
 
     private DisjointSet buildPhiCongruenceClasses(Program program) {
         DisjointSet classes = new DisjointSet();

teavm-core/src/main/java/org/teavm/optimization/ClassSetOptimizer.java

@@ -36,7 +36,8 @@ public class ClassSetOptimizer {
     }
 
     private List<MethodOptimization> getOptimizations() {
-        return Arrays.<MethodOptimization>asList(new UnusedVariableElimination());
+        return Arrays.<MethodOptimization>asList(new CommonSubexpressionElimination(),
+                new UnusedVariableElimination());
     }
 
     public void optimizeAll(ListableClassHolderSource classSource) {