Consider temp registers too as part of SSA because in boolean expressions, a temp is assigned value more than once so although generally temps are SSA already, in this case they are not.

Author: dibyendumajumdar

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

@@ -627,8 +627,6 @@ else if (indexed instanceof Operand.LoadFieldOperand loadFieldOperand) {
     }
 
     private void codeNew(Type type) {
-        // temps are already SSA so we don't need to
-        // do anything special
         var temp = createTemp(type);
         if (type instanceof Type.TypeArray typeArray) {
             code(new Instruction.NewArray(typeArray, temp));
@@ -650,8 +648,8 @@ private void codeStoreAppend() {
     }
 
     private void codeArg(Operand.LocalRegisterOperand target) {
-        target = (Operand.LocalRegisterOperand) issa.write(target);
-        var insn = new Instruction.ArgInstruction(target);
+        var newtarget = (Operand.RegisterOperand) issa.write(target);
+        var insn = new Instruction.ArgInstruction(newtarget);
         issa.recordDef(target, insn);
         code(insn);
     }
@@ -820,11 +818,14 @@ static final class IncrementalSSABraun implements IncrementalSSA {
         // a mapping is maintained for the name to SSA value (virtual register) in each block.
         // we could add this mapping to the BB itself but it seems nicer to keep it separate
         // at least for now.
-        Map<Symbol.VarSymbol, Map<BasicBlock, Register>> currentDef = new HashMap<>();
+        // Note that we use the nonSSAId as proxy for variable name
+        // because this id is unique for non-SSA variables, but for SSA versions this refers
+        // back to the original ID
+        Map<Integer, Map<BasicBlock, Register>> currentDef = new HashMap<>();
         // Flags blocks that are completed in terms of instruction generation
         BitSet sealedBlocks = new BitSet();
         // Tracks Phis that are not finalized because the basic block is not yet sealed
-        Map<BasicBlock, Map<Symbol.VarSymbol, Instruction.Phi>> incompletePhis = new HashMap<>();
+        Map<BasicBlock, Map<Register, Instruction.Phi>> incompletePhis = new HashMap<>();
 
         // Not explicitly stated in the paper but implicit in the algo is
         // the availability of Def-use chains. We have to main this incrementally as we
@@ -846,17 +847,17 @@ private IncrementalSSABraun(CompiledFunction function) {
         /**
          * Associates a new definition (value) to a variable name within a basic block
          */
-        private void writeVariable(Symbol.VarSymbol variable, BasicBlock block, Register value) {
-            currentDef.computeIfAbsent(variable, k -> new HashMap<>()).put(block, value);
+        private void writeVariable(Register variable, BasicBlock block, Register value) {
+            currentDef.computeIfAbsent(variable.nonSSAId(), k -> new HashMap<>()).put(block, value);
         }
 
         /**
          * Looks up the current SSA value (virtual register) associated with a name, inside a block.
          * If no mapping is found, processing depends on status of the block.
-         * @see #readVariableRecursive(Symbol.VarSymbol, BasicBlock) 
+         * @see #readVariableRecursive(Register, BasicBlock)
          */
-        private Register readVariable(Symbol.VarSymbol variable, BasicBlock block) {
-            Map<BasicBlock, Register> defs = currentDef.get(variable);
+        private Register readVariable(Register variable, BasicBlock block) {
+            Map<BasicBlock, Register> defs = currentDef.get(variable.nonSSAId());
             if (defs != null && defs.containsKey(block)) {
                 // local value numbering
                 return defs.get(block);
@@ -874,11 +875,11 @@ private Register readVariable(Symbol.VarSymbol variable, BasicBlock block) {
          * obtained recursively via each predecessor block.
          * In case of 1 predecessor the value is read recursively from that predecessor.
          */
-        private Register readVariableRecursive(Symbol.VarSymbol variable, BasicBlock block) {
+        private Register readVariableRecursive(Register variable, BasicBlock block) {
             Register val;
             if (!sealedBlocks.get(block.bid)) {
                 // incomplete CFG
-                val = function.registerPool.newISSAReg("phi_" + variable.name, variable.type);
+                val = function.registerPool.newISSAReg("phi_" + variable.name(), variable.type);
                 Instruction.Phi phi = new Instruction.Phi(val, new ArrayList<>());
                 recordDef(val, phi);
                 block.add(0, phi);
@@ -890,7 +891,7 @@ else if (block.predecessors.size() == 1) {
             }
             else {
                 // Break potential cycles with operandless phis
-                val = function.registerPool.newISSAReg("phi_" + variable.name, variable.type);
+                val = function.registerPool.newISSAReg("phi_" + variable.name(), variable.type);
                 Instruction.Phi phi = new Instruction.Phi(val, new ArrayList<>());
                 recordDef(val, phi);
                 block.add(0, phi);
@@ -904,7 +905,7 @@ else if (block.predecessors.size() == 1) {
         /**
          * Populate the members of a phi instruction
          */
-        private Register addPhiOperands(Symbol.VarSymbol variable, Instruction.Phi phi) {
+        private Register addPhiOperands(Register variable, Instruction.Phi phi) {
             // Determine operands from predecessors
             for (BasicBlock pred: phi.block.predecessors) {
                 phi.addInput(readVariable(variable,pred));
@@ -984,34 +985,34 @@ private List<Instruction> getUsesExcept(Instruction.Phi phi) {
 
         @Override
         public Operand read(Operand operand) {
-            // We only consider declared local variables
-            // because temps are already SSA in this compiler
-            if (operand instanceof Operand.LocalRegisterOperand localRegisterOperand) {
-                var reg = readVariable(localRegisterOperand.variable, function.currentBlock);
-                operand = new Operand.LocalRegisterOperand(reg, localRegisterOperand.variable);
+            // We have to consider temps too because of boolean expressions
+            // where temps are not SSA
+            if (operand instanceof Operand.RegisterOperand localRegisterOperand) {
+                var reg = readVariable(localRegisterOperand.reg, function.currentBlock);
+                operand = new Operand.RegisterOperand(reg);
             }
             return operand;
         }
         @Override
         public Operand write(Operand operand) {
-            // We only consider declared local variables
-            // because temps are already SSA in this compiler
-            if (operand instanceof Operand.LocalRegisterOperand localRegisterOperand) {
-                var variable = localRegisterOperand.variable;
+            // We have to consider temps too because of boolean expressions
+            // where temps are not SSA
+            if (operand instanceof Operand.RegisterOperand localRegisterOperand) {
+                var variable = localRegisterOperand.reg;
                 Register newValue;
-                Integer version = versioned.get(localRegisterOperand.reg.nonSSAId());
+                Integer version = versioned.get(variable.nonSSAId());
                 // Avoid creating a new value first time because we already
                 // have a pre-created register we can use
                 if (version == null) {
-                    newValue = localRegisterOperand.reg;
-                    versioned.put(localRegisterOperand.reg.nonSSAId(), 1);
+                    newValue = variable;
+                    versioned.put(variable.nonSSAId(), 1);
                 }
                 else {
-                    versioned.put(localRegisterOperand.reg.nonSSAId(), version + 1);
-                    newValue = function.registerPool.ssaReg(localRegisterOperand.reg, version);
+                    versioned.put(variable.nonSSAId(), version + 1);
+                    newValue = function.registerPool.ssaReg(variable, version);
                 }
-                writeVariable(localRegisterOperand.variable, function.currentBlock, newValue);
-                operand = new Operand.LocalRegisterOperand(newValue,variable);
+                writeVariable(variable, function.currentBlock, newValue);
+                operand = new Operand.RegisterOperand(newValue);
             }
             return operand;
         }

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

@@ -83,6 +83,7 @@ public String name() {
      * During SSA form this is not valid for registers that are instances of SSARegister.
      */
     public int nonSSAId() {
+        assert frameSlot >= 0;
         return frameSlot;
     }
     public void updateSlot(int slot) {

optvm/src/test/java/com/compilerprogramming/ezlang/interpreter/TestInterpreter.java

@@ -13,6 +13,7 @@ Value compileAndRun(String src, String mainFunction) {
         return compileAndRun(src, mainFunction, Options.NONE);
     }
     Value compileAndRun(String src, String mainFunction, EnumSet<Options> options) {
+        //options.add(Options.ISSA);
         var compiler = new Compiler();
         var typeDict = compiler.compileSrc(src, options);
         var compiled = compiler.dumpIR(typeDict);