Merge pull request #23 from CompilerProgramming/sccp

Sparse Conditional Constant Propagation - apply the changes after analysis

Author: dibyendumajumdar

optvm/src/main/java/com/compilerprogramming/ezlang/compiler/BasicBlock.java

@@ -98,10 +98,19 @@ public void add(Instruction instruction) {
         instructions.add(instruction);
         instruction.block = this;
     }
+    public void deleteInstruction(Instruction instruction) {
+        instructions.remove(instruction);
+    }
     public void addSuccessor(BasicBlock successor) {
+        assert successors.contains(successor) == false;
         successors.add(successor);
+        assert successor.predecessors.contains(this) == false;
         successor.predecessors.add(this);
     }
+    public void removeSuccessor(BasicBlock successor) {
+        successors.remove(successor);
+        successor.predecessors.remove(this);
+    }
 
     /**
      * Initially the phi has the form
@@ -132,15 +141,25 @@ public List<Instruction.Phi> phis() {
         }
         return list;
     }
-    public int whichPred(BasicBlock s) {
+    public int whichPred(BasicBlock pred) {
         int i = 0;
-        for (BasicBlock p: s.predecessors) {
-            if (p == this)
+        for (BasicBlock p: predecessors) {
+            if (p == pred)
                 return i;
             i++;
         }
         throw new IllegalStateException();
     }
+    public int whichSucc(BasicBlock succ) {
+        int i = 0;
+        for (BasicBlock s: successors) {
+            if (s == succ)
+                return i;
+            i++;
+        }
+        throw new IllegalStateException();
+    }
+
     public static StringBuilder toStr(StringBuilder sb, BasicBlock bb, BitSet visited, boolean dumpLiveness)
     {
         if (visited.get(bb.bid))

optvm/src/main/java/com/compilerprogramming/ezlang/compiler/DominatorTree.java

@@ -193,7 +193,9 @@ private void calculateDominanceFrontiers() {
             if (b.predecessors.size() >= 2) {
                 for (BasicBlock p : b.predecessors) {
                     BasicBlock runner = p;
-                    while (runner != b.idom) {
+                    // re runner != null: Dominance frontier calc fails in infinite loop
+                    // scenario - need to check what the correct solution is
+                    while (runner != b.idom && runner != null) {
                         runner.dominationFrontier.add(b);
                         runner = runner.idom;
                     }

optvm/src/main/java/com/compilerprogramming/ezlang/compiler/EnterSSA.java

@@ -153,9 +153,9 @@ void search(BasicBlock block) {
         }
         // Update phis in successor blocks
         for (BasicBlock s: block.successors) {
-            int j = block.whichPred(s);
+            int j = s.whichPred(block);
             for (Instruction.Phi phi: s.phis()) {
-                Register oldReg = phi.input(j);
+                Register oldReg = phi.inputAsRegister(j);
                 phi.replaceInput(j, stacks[oldReg.nonSSAId()].top());
             }
         }

optvm/src/main/java/com/compilerprogramming/ezlang/compiler/ExitSSA.java

@@ -52,6 +52,11 @@ private void insertCopies(BasicBlock block) {
      * replace all uses u with stacks[i]
      */
     private void replaceUses(Instruction i) {
+        if (i instanceof Instruction.Phi)
+            // FIXME check this can never be valid
+            // tests 8/9 in TestInterpreter invoke on Phi but
+            // replacements are same as existing inputs
+            return;
         var oldUses = i.uses();
         Register[] newUses = new Register[oldUses.size()];
         for (int u = 0; u < oldUses.size(); u++) {
@@ -65,13 +70,15 @@ private void replaceUses(Instruction i) {
     }
 
     static class CopyItem {
-        final Register src;
+        final Operand src;
         final Register dest;
+        final BasicBlock destBlock;
         boolean removed;
 
-        public CopyItem(Register src, Register dest) {
+        public CopyItem(Operand src, Register dest, BasicBlock destBlock) {
             this.src = src;
             this.dest = dest;
+            this.destBlock = destBlock;
             this.removed = false;
         }
     }
@@ -83,14 +90,17 @@ private void scheduleCopies(BasicBlock block, List<Integer> pushed) {
         Map<Integer, Register> map = new HashMap<>();
         BitSet usedByAnother = new BitSet(function.registerPool.numRegisters()*2);
         for (BasicBlock s: block.successors) {
-            int j = block.whichPred(s);
+            int j = s.whichPred(block);
             for (Instruction.Phi phi: s.phis()) {
                 Register dst = phi.value();
-                Register src = phi.input(j);   // jth operand of phi node
-                copySet.add(new CopyItem(src, dst));
-                map.put(src.id, src);
+                Operand srcOperand = phi.input(j); // jth operand of phi node
+                if (srcOperand instanceof Operand.RegisterOperand srcRegisterOperand) {
+                    Register src = srcRegisterOperand.reg;
+                    map.put(src.id, src);
+                    usedByAnother.set(src.id);
+                }
+                copySet.add(new CopyItem(srcOperand, dst, s));
                 map.put(dst.id, dst);
-                usedByAnother.set(src.id);
             }
         }
 
@@ -111,8 +121,9 @@ private void scheduleCopies(BasicBlock block, List<Integer> pushed) {
         while (!workList.isEmpty() || !copySet.isEmpty()) {
             while (!workList.isEmpty()) {
                 final CopyItem copyItem = workList.remove(0);
-                final Register src = copyItem.src;
+                final Operand src = copyItem.src;
                 final Register dest = copyItem.dest;
+                final BasicBlock destBlock = copyItem.destBlock;
                 /* Engineering a Compiler: We can avoid the lost copy
                    problem by checking the liveness of the target name
                    for each copy that we try to insert. When we discover
@@ -122,18 +133,23 @@ private void scheduleCopies(BasicBlock block, List<Integer> pushed) {
                  */
                 if (block.liveOut.get(dest.id)) {
                     /* Insert a copy from dest to a new temp t at phi node defining dest */
-                    final Register t = addMoveToTempAfterPhi(block, dest);
+                    final Register t = addMoveToTempAfterPhi(destBlock, dest);
                     stacks[dest.id].push(t);
                     pushed.add(dest.id);
                 }
                 /* Insert a copy operation from map[src] to dest at end of BB */
-                addMoveAtBBEnd(block, map.get(src.id), dest);
-                map.put(src.id, dest);
-                /* If src is the name of a dest in copySet add item to worklist */
-                /* see comment on phi cycles below. */
-                CopyItem item = isCycle(copySet, src);
-                if (item != null) {
-                    workList.add(item);
+                if (src instanceof Operand.RegisterOperand srcRegisterOperand) {
+                    addMoveAtBBEnd(block, map.get(srcRegisterOperand.reg.id), dest);
+                    map.put(srcRegisterOperand.reg.id, dest);
+                    /* If src is the name of a dest in copySet add item to worklist */
+                    /* see comment on phi cycles below. */
+                    CopyItem item = isCycle(copySet, srcRegisterOperand.reg);
+                    if (item != null) {
+                        workList.add(item);
+                    }
+                }
+                else if (src instanceof Operand.ConstantOperand srcConstantOperand) {
+                    addMoveAtBBEnd(block, srcConstantOperand, dest);
                 }
             }
             /* Engineering a Compiler: To solve the swap problem
@@ -204,7 +220,10 @@ private void addMoveAtBBEnd(BasicBlock block, Register src, Register dest) {
         var inst = new Instruction.Move(new Operand.RegisterOperand(src), new Operand.RegisterOperand(dest));
         insertAtEnd(block, inst);
     }
-
+    private void addMoveAtBBEnd(BasicBlock block, Operand.ConstantOperand src, Register dest) {
+        var inst = new Instruction.Move(src, new Operand.RegisterOperand(dest));
+        insertAtEnd(block, inst);
+    }
     /* Insert a copy dest to a new temp at phi node defining dest, return temp */
     private Register addMoveToTempAfterPhi(BasicBlock block, Register dest) {
         var temp = function.registerPool.newTempReg(dest.name(), dest.type);

optvm/src/main/java/com/compilerprogramming/ezlang/compiler/Instruction.java

@@ -27,19 +27,21 @@ public abstract class Instruction {
     static final int I_FIELD_SET = 16;
 
     public final int opcode;
-    public Operand.RegisterOperand def;
-    public Operand[] uses;
+    protected Operand.RegisterOperand def;
+    protected Operand[] uses;
     public BasicBlock block;
 
     public Instruction(int opcode, Operand... uses) {
         this.opcode = opcode;
         this.def = null;
-        this.uses = uses;
+        this.uses = new Operand[uses.length];
+        System.arraycopy(uses, 0, this.uses, 0, uses.length);
     }
     public Instruction(int opcode, Operand.RegisterOperand def, Operand... uses) {
         this.opcode = opcode;
         this.def = def;
-        this.uses = uses;
+        this.uses = new Operand[uses.length];
+        System.arraycopy(uses, 0, this.uses, 0, uses.length);
     }
 
     public boolean isTerminal() { return false; }
@@ -87,7 +89,14 @@ public boolean replaceUse(Register source, Register target) {
         }
         return replaced;
     }
-
+    public void replaceWithConstant(Register register, Operand.ConstantOperand constantOperand) {
+        for (int i = 0; i < uses.length; i++) {
+            Operand operand = uses[i];
+            if (operand != null && operand instanceof Operand.RegisterOperand registerOperand && registerOperand.reg.id == register.id) {
+                uses[i] = constantOperand;
+            }
+        }
+    }
     public static class NoOp extends Instruction {
         public NoOp() {
             super(I_NOOP);
@@ -340,21 +349,35 @@ public StringBuilder toStr(StringBuilder sb) {
     /**
      * Phi does not generate uses or defs directly, instead
      * they are treated as a special case.
-     * To avoid bugs we do not use the def or uses.
+     * To avoid bugs we disable the standard interfaces
      */
     public static class Phi extends Instruction {
-        public Register value;
-        public final Register[] inputs;
+        private Register value;
         public Phi(Register value, List<Register> inputs) {
             super(I_PHI);
             this.value = value;
-            this.inputs = inputs.toArray(new Register[inputs.size()]);
+            this.uses = new Operand[inputs.size()];
+            for (int i = 0; i < inputs.size(); i++) {
+                this.uses[i] = new Operand.RegisterOperand(inputs.get(i));
+            }
         }
         public void replaceInput(int i, Register newReg) {
-            inputs[i] = newReg;
+            uses[i].replaceRegister(newReg);
+        }
+        /**
+         * This will fail in input was replaced by a constant
+         */
+        public Register inputAsRegister(int i) {
+            return ((Operand.RegisterOperand) uses[i]).reg;
+        }
+        public Operand input(int i) {
+            return uses[i];
+        }
+        public boolean isRegisterInput(int i) {
+            return uses[i] instanceof Operand.RegisterOperand;
         }
-        public Register input(int i) {
-            return inputs[i];
+        public Register[] inputRegisters() {
+            return super.uses().toArray(new Register[super.uses().size()]);
         }
         @Override
         public Register def() {
@@ -368,18 +391,36 @@ public void replaceDef(Register newReg) {
         public boolean definesVar() {
             return false;
         }
+        @Override
+        public List<Register> uses() {
+            return Collections.emptyList();
+        }
+        @Override
+        public void replaceUses(Register[] newUses) {
+            throw new UnsupportedOperationException();
+        }
+        @Override
+        public boolean replaceUse(Register source, Register target) {
+            throw new UnsupportedOperationException();
+        }
         public Register value() {
             return value;
         }
         public void replaceValue(Register newReg) {
             this.value = newReg;
         }
+        public void removeInput(int i) {
+            var newUses = new Operand[uses.length - 1];
+            System.arraycopy(uses, 0, newUses, 0, i);
+            System.arraycopy(uses, i + 1, newUses, i, uses.length - i - 1);
+            this.uses = newUses;
+        }
         @Override
         public StringBuilder toStr(StringBuilder sb) {
             sb.append(value().name()).append(" = phi(");
-            for (int i = 0; i < inputs.length; i++) {
+            for (int i = 0; i < uses.length; i++) {
                 if (i > 0) sb.append(", ");
-                sb.append(inputs[i].name());
+                sb.append(uses[i].toString());
             }
             sb.append(")");
             return sb;

optvm/src/main/java/com/compilerprogramming/ezlang/compiler/InterferenceGraph.java

@@ -63,6 +63,10 @@ public void rename(Integer source, Integer target) {
         // Move all interferences
         var fromSet = edges.remove(source);
         var toSet = edges.get(target);
+        if (toSet == null) {
+            //throw new RuntimeException("Cannot find edge " + target + " from " + source);
+            return; // FIXME this is workaround to handle sceanrio where target is arg register but we need a better way
+        }
         toSet.addAll(fromSet);
         // If any node interfered with from
         // it should now interfere with to

optvm/src/main/java/com/compilerprogramming/ezlang/compiler/Liveness.java

@@ -69,7 +69,9 @@ private void init(List<BasicBlock> blocks) {
                 if (instruction instanceof Instruction.Phi phi) {
                     for (int i = 0; i < block.predecessors.size(); i++) {
                         BasicBlock pred = block.predecessors.get(i);
-                        Register use = phi.input(i);
+                        if (!phi.isRegisterInput(i))
+                            continue;
+                        Register use = phi.inputAsRegister(i);
                         // We can have a block referring it its own phis
                         // if there is loop back and there are cycles
                         // such as e.g. the swap copy problem

optvm/src/main/java/com/compilerprogramming/ezlang/compiler/Optimizer.java

@@ -4,7 +4,7 @@ public class Optimizer {
 
     public void optimize(CompiledFunction function) {
         new EnterSSA(function);
-        new SparseConditionalConstantPropagation().constantPropagation(function);
+        new SparseConditionalConstantPropagation().constantPropagation(function).apply();
         new ExitSSA(function);
         new ChaitinGraphColoringRegisterAllocator().assignRegisters(function, 64);
     }

optvm/src/main/java/com/compilerprogramming/ezlang/compiler/SSAEdges.java

@@ -58,7 +58,7 @@ private static void recordUses(CompiledFunction function, Map<Register, SSADef>
         for (BasicBlock block : function.getBlocks()) {
             for (Instruction instruction : block.instructions) {
                 if (instruction instanceof Instruction.Phi phi) {
-                    recordUses(defUseChains, phi.inputs, block, instruction);
+                    recordUses(defUseChains, phi.inputRegisters(), block, instruction);
                 }
                 else {
                     List<Register> uses = instruction.uses();