Skip to content

Commit 8304045

Browse files
dibyendumajumdardibyendumajumdar
authored
Merge pull request #8 from CompilerProgramming/ssatest
Bug fixes and test for lost copy & swap problems
2 parents 24317cf + b3eb039 commit 8304045

File tree

4 files changed

+202
-37
lines changed

4 files changed

+202
-37
lines changed

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public class CompiledFunction {
1515
public BasicBlock entry;
1616
public BasicBlock exit;
1717
private int bid = 0;
18-
private BasicBlock currentBlock;
18+
public BasicBlock currentBlock;
1919
private BasicBlock currentBreakTarget;
2020
private BasicBlock currentContinueTarget;
2121
private Type.TypeFunction functionType;
@@ -52,6 +52,17 @@ public CompiledFunction(Symbol.FunctionTypeSymbol functionSymbol) {
5252
this.frameSlots = registerPool.numRegisters();
5353
}
5454

55+
public CompiledFunction(Type.TypeFunction functionType) {
56+
this.functionType = (Type.TypeFunction) functionType;
57+
this.registerPool = new RegisterPool("%ret", functionType == null?null:functionType.returnType);
58+
this.bid = 0;
59+
this.entry = this.currentBlock = createBlock();
60+
this.exit = createBlock();
61+
this.currentBreakTarget = null;
62+
this.currentContinueTarget = null;
63+
this.frameSlots = registerPool.numRegisters();
64+
}
65+
5566
private void generateArgInstructions(Scope scope) {
5667
if (scope.isFunctionParameterScope) {
5768
for (Symbol symbol: scope.getLocalSymbols()) {
@@ -84,7 +95,7 @@ private void setVirtualRegisters(Scope scope) {
8495
}
8596
}
8697

87-
private BasicBlock createBlock() {
98+
public BasicBlock createBlock() {
8899
return new BasicBlock(bid++);
89100
}
90101

@@ -111,7 +122,7 @@ else if (virtualStack.size() > 1)
111122
jumpTo(exit);
112123
}
113124

114-
private void code(Instruction instruction) {
125+
public void code(Instruction instruction) {
115126
currentBlock.add(instruction);
116127
}
117128

@@ -213,13 +224,13 @@ private boolean isBlockTerminated(BasicBlock block) {
213224
block.instructions.getLast().isTerminal());
214225
}
215226

216-
private void jumpTo(BasicBlock block) {
227+
public void jumpTo(BasicBlock block) {
217228
assert !isBlockTerminated(currentBlock);
218229
currentBlock.add(new Instruction.Jump(block));
219230
currentBlock.addSuccessor(block);
220231
}
221232

222-
private void startBlock(BasicBlock block) {
233+
public void startBlock(BasicBlock block) {
223234
if (!isBlockTerminated(currentBlock)) {
224235
jumpTo(block);
225236
}
@@ -541,11 +552,12 @@ private boolean vstackEmpty() {
541552
return virtualStack.isEmpty();
542553
}
543554

544-
public void toStr(StringBuilder sb, boolean verbose) {
555+
public StringBuilder toStr(StringBuilder sb, boolean verbose) {
545556
if (verbose) {
546557
sb.append(this.functionType.describe()).append("\n");
547558
registerPool.toStr(sb);
548559
}
549560
BasicBlock.toStr(sb, entry, new BitSet(), verbose);
561+
return sb;
550562
}
551563
}

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

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ private void removePhis() {
3434

3535
/* Algorithm for iterating through blocks to perform phi replacement */
3636
private void insertCopies(BasicBlock block) {
37-
List<Register> pushed = new ArrayList<>();
37+
List<Integer> pushed = new ArrayList<>();
3838
for (Instruction i: block.instructions) {
3939
// replace all uses u with stacks[i]
4040
if (i.usesVars()) {
@@ -45,8 +45,8 @@ private void insertCopies(BasicBlock block) {
4545
for (BasicBlock c: block.dominatedChildren) {
4646
insertCopies(c);
4747
}
48-
for (Register name: pushed) {
49-
stacks[name.id].pop();
48+
for (Integer name: pushed) {
49+
stacks[name].pop();
5050
}
5151
}
5252

@@ -67,8 +67,8 @@ private void replaceUses(Instruction i) {
6767
}
6868

6969
static class CopyItem {
70-
Register src;
71-
Register dest;
70+
final Register src;
71+
final Register dest;
7272
boolean removed;
7373

7474
public CopyItem(Register src, Register dest) {
@@ -78,7 +78,7 @@ public CopyItem(Register src, Register dest) {
7878
}
7979
}
8080

81-
private void scheduleCopies(BasicBlock block, List<Register> pushed) {
81+
private void scheduleCopies(BasicBlock block, List<Integer> pushed) {
8282
/* Pass 1 - Initialize data structures */
8383
/* In this pass we count the number of times a name is used by other phi-nodes */
8484
List<CopyItem> copySet = new ArrayList<>();
@@ -100,7 +100,7 @@ private void scheduleCopies(BasicBlock block, List<Register> pushed) {
100100
/* In this pass we build a worklist of names that are not used in other phi nodes */
101101
List<CopyItem> workList = new ArrayList<>();
102102
for (CopyItem copyItem: copySet) {
103-
if (!usedByAnother.get(copyItem.dest.id)) {
103+
if (usedByAnother.get(copyItem.dest.id) != true) {
104104
copyItem.removed = true;
105105
workList.add(copyItem);
106106
}
@@ -112,17 +112,17 @@ private void scheduleCopies(BasicBlock block, List<Register> pushed) {
112112
/* Each time we insert a copy operation we add the source of that op to the worklist */
113113
while (!workList.isEmpty() || !copySet.isEmpty()) {
114114
while (!workList.isEmpty()) {
115-
CopyItem copyItem = workList.remove(0);
116-
Register src = copyItem.src;
117-
Register dest = copyItem.dest;
115+
final CopyItem copyItem = workList.remove(0);
116+
final Register src = copyItem.src;
117+
final Register dest = copyItem.dest;
118118
if (block.liveOut.get(dest.id)) {
119119
/* Insert a copy from dest to a new temp t at phi node defining dest */
120-
Register t = insertCopy(block, dest);
120+
final Register t = addMoveToTempAfterPhi(block, dest);
121121
stacks[dest.id].push(t);
122-
pushed.add(t);
122+
pushed.add(dest.id);
123123
}
124-
/* Insert a copy operation from map[src] to dst at end of BB */
125-
appendCopy(block, map.get(src.id), dest);
124+
/* Insert a copy operation from map[src] to dest at end of BB */
125+
addMoveAtBBEnd(block, map.get(src.id), dest);
126126
map.put(src.id, dest);
127127
/* If src is the name of a dest in copySet add item to worklist */
128128
CopyItem item = isDest(copySet, src);
@@ -133,7 +133,7 @@ private void scheduleCopies(BasicBlock block, List<Register> pushed) {
133133
if (!copySet.isEmpty()) {
134134
CopyItem copyItem = copySet.remove(0);
135135
/* Insert a copy from dst to new temp at the end of Block */
136-
Register t = appendCopy(block, copyItem.dest);
136+
Register t = addMoveToTempAtBBEnd(block, copyItem.dest);
137137
map.put(copyItem.dest.id, t);
138138
workList.add(copyItem);
139139
}
@@ -142,30 +142,33 @@ private void scheduleCopies(BasicBlock block, List<Register> pushed) {
142142

143143
private void insertAtEnd(BasicBlock bb, Instruction i) {
144144
assert bb.instructions.size() > 0;
145+
// Last instruction is a branch - so new instruction will
146+
// go before that
145147
int pos = bb.instructions.size()-1;
146148
bb.instructions.add(pos, i);
147149
}
148150

149151
private void insertAfterPhi(BasicBlock bb, Register phiDef, Instruction newInst) {
150152
assert bb.instructions.size() > 0;
151-
int pos = 0;
152-
while (pos < bb.instructions.size()) {
153-
Instruction i = bb.instructions.get(pos++);
154-
if (i instanceof Instruction.Phi phi &&
155-
phi.def().id == phiDef.id) {
156-
break;
153+
int insertionPos = -1;
154+
for (int pos = 0; pos < bb.instructions.size(); pos++) {
155+
Instruction i = bb.instructions.get(pos);
156+
if (i instanceof Instruction.Phi phi) {
157+
if (phi.def().id == phiDef.id) {
158+
insertionPos = pos+1; // After phi
159+
break;
160+
}
157161
}
158162
}
159-
if (pos == bb.instructions.size()) {
163+
if (insertionPos < 0) {
160164
throw new IllegalStateException();
161165
}
162-
bb.instructions.add(pos, newInst);
166+
bb.instructions.add(insertionPos, newInst);
163167
}
164168

165-
166169
/* Insert a copy from dest to new temp at end of BB, and return temp */
167-
private Register appendCopy(BasicBlock block, Register dest) {
168-
var temp = function.registerPool.newReg(dest.name(), dest.type);
170+
private Register addMoveToTempAtBBEnd(BasicBlock block, Register dest) {
171+
var temp = function.registerPool.newTempReg(dest.name(), dest.type);
169172
var inst = new Instruction.Move(new Operand.RegisterOperand(dest), new Operand.RegisterOperand(temp));
170173
insertAtEnd(block, inst);
171174
return temp;
@@ -181,16 +184,16 @@ private CopyItem isDest(List<CopyItem> copySet, Register src) {
181184
}
182185

183186
/* Insert a copy from src to dst at end of BB */
184-
private void appendCopy(BasicBlock block, Register src, Register dest) {
187+
private void addMoveAtBBEnd(BasicBlock block, Register src, Register dest) {
185188
var inst = new Instruction.Move(new Operand.RegisterOperand(src), new Operand.RegisterOperand(dest));
186189
insertAtEnd(block, inst);
187190
}
188191

189192
/* Insert a copy dest to a new temp at phi node defining dest, return temp */
190-
private Register insertCopy(BasicBlock block, Register dst) {
191-
var temp = function.registerPool.newReg(dst.name(), dst.type);
192-
var inst = new Instruction.Move(new Operand.RegisterOperand(dst), new Operand.RegisterOperand(temp));
193-
insertAfterPhi(block, dst, inst);
193+
private Register addMoveToTempAfterPhi(BasicBlock block, Register dest) {
194+
var temp = function.registerPool.newTempReg(dest.name(), dest.type);
195+
var inst = new Instruction.Move(new Operand.RegisterOperand(dest), new Operand.RegisterOperand(temp));
196+
insertAfterPhi(block, dest, inst);
194197
return temp;
195198
}
196199

‎optvm/src/main/java/com/compilerprogramming/ezlang/compiler/RegisterPool.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ public Register newTempReg(Type type) {
3838
registers.add(reg);
3939
return reg;
4040
}
41+
public Register newTempReg(String baseName, Type type) {
42+
var id = registers.size();
43+
var name = baseName+"_"+id;
44+
var reg = new Register(id, name, type);
45+
registers.add(reg);
46+
return reg;
47+
}
4148

4249
public Register.SSARegister ssaReg(Register original, int version) {
4350
var id = registers.size();

‎optvm/src/test/java/com/compilerprogramming/ezlang/compiler/TestSSATransform.java‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
package com.compilerprogramming.ezlang.compiler;
22

33
import com.compilerprogramming.ezlang.types.Symbol;
4+
import com.compilerprogramming.ezlang.types.Type;
5+
import com.compilerprogramming.ezlang.types.TypeDictionary;
46
import org.junit.Assert;
57
import org.junit.Test;
68

9+
import java.util.Arrays;
710
import java.util.BitSet;
811

912
public class TestSSATransform {
@@ -625,4 +628,144 @@ func bar(arg: Int)->Int {
625628
goto L1
626629
""", result);
627630
}
631+
632+
/**
633+
* This test case is based on the example snippet from Briggs paper
634+
* illustrating the lost copy problem.
635+
*/
636+
private CompiledFunction buildLostCopyTest() {
637+
TypeDictionary typeDictionary = new TypeDictionary();
638+
Type.TypeFunction functionType = new Type.TypeFunction("foo");
639+
functionType.addArg(new Symbol.ParameterSymbol("p", typeDictionary.INT));
640+
functionType.setReturnType(typeDictionary.INT);
641+
CompiledFunction function = new CompiledFunction(functionType);
642+
RegisterPool regPool = function.registerPool;
643+
Register p = regPool.newReg("p", typeDictionary.INT);
644+
Register x1 = regPool.newReg("x1", typeDictionary.INT);
645+
function.code(new Instruction.ArgInstruction(new Operand.LocalRegisterOperand(p)));
646+
function.code(new Instruction.Move(
647+
new Operand.ConstantOperand(1, typeDictionary.INT),
648+
new Operand.RegisterOperand(x1)));
649+
BasicBlock B2 = function.createBlock();
650+
function.startBlock(B2);
651+
Register x3 = regPool.newReg("x3", typeDictionary.INT);
652+
Register x2 = regPool.newReg("x2", typeDictionary.INT);
653+
function.code(new Instruction.Phi(x2, Arrays.asList(x1, x3)));
654+
function.code(new Instruction.Binary("+",
655+
new Operand.RegisterOperand(x3),
656+
new Operand.RegisterOperand(x2),
657+
new Operand.ConstantOperand(1, typeDictionary.INT)));
658+
function.code(new Instruction.ConditionalBranch(B2,
659+
new Operand.RegisterOperand(p), B2, function.exit));
660+
function.startBlock(function.exit);
661+
function.code(new Instruction.Return(new Operand.RegisterOperand(x2), regPool.returnRegister));
662+
function.isSSA = true;
663+
return function;
664+
}
665+
666+
@Test
667+
public void testLostCopyProblem() {
668+
CompiledFunction function = buildLostCopyTest();
669+
String expected = """
670+
L0:
671+
arg p
672+
x1 = 1
673+
goto L2
674+
L2:
675+
x2 = phi(x1, x3)
676+
x3 = x2+1
677+
if p goto L2 else goto L1
678+
L1:
679+
%ret = x2
680+
""";
681+
Assert.assertEquals(expected, function.toStr(new StringBuilder(), false).toString());
682+
new ExitSSA(function);
683+
expected = """
684+
L0:
685+
arg p
686+
x1 = 1
687+
x2 = x1
688+
goto L2
689+
L2:
690+
x2_5 = x2
691+
x3 = x2+1
692+
x2 = x3
693+
if p goto L2 else goto L1
694+
L1:
695+
%ret = x2_5
696+
""";
697+
Assert.assertEquals(expected, function.toStr(new StringBuilder(), false).toString());
698+
}
699+
700+
/**
701+
* This test case is based on the example snippet from Briggs paper
702+
* illustrating the swap problem.
703+
*/
704+
private CompiledFunction buildSwapTest() {
705+
TypeDictionary typeDictionary = new TypeDictionary();
706+
Type.TypeFunction functionType = new Type.TypeFunction("foo");
707+
functionType.addArg(new Symbol.ParameterSymbol("p", typeDictionary.INT));
708+
CompiledFunction function = new CompiledFunction(functionType);
709+
RegisterPool regPool = function.registerPool;
710+
Register p = regPool.newReg("p", typeDictionary.INT);
711+
Register a1 = regPool.newReg("a1", typeDictionary.INT);
712+
Register a2 = regPool.newReg("a2", typeDictionary.INT);
713+
Register a3 = regPool.newReg("a3", typeDictionary.INT);
714+
Register b1 = regPool.newReg("b1", typeDictionary.INT);
715+
Register b2 = regPool.newReg("b2", typeDictionary.INT);
716+
function.code(new Instruction.ArgInstruction(new Operand.LocalRegisterOperand(p)));
717+
function.code(new Instruction.Move(
718+
new Operand.ConstantOperand(42, typeDictionary.INT),
719+
new Operand.RegisterOperand(a1)));
720+
function.code(new Instruction.Move(
721+
new Operand.ConstantOperand(24, typeDictionary.INT),
722+
new Operand.RegisterOperand(b1)));
723+
BasicBlock B2 = function.createBlock();
724+
function.startBlock(B2);
725+
function.code(new Instruction.Phi(a2, Arrays.asList(a1, b2)));
726+
function.code(new Instruction.Phi(b2, Arrays.asList(b1, a2)));
727+
function.code(new Instruction.ConditionalBranch(B2,
728+
new Operand.RegisterOperand(p), B2, function.exit));
729+
function.startBlock(function.exit);
730+
function.isSSA = true;
731+
return function;
732+
}
733+
734+
@Test
735+
public void testSwapProblem() {
736+
CompiledFunction function = buildSwapTest();
737+
String expected = """
738+
L0:
739+
arg p
740+
a1 = 42
741+
b1 = 24
742+
goto L2
743+
L2:
744+
a2 = phi(a1, b2)
745+
b2 = phi(b1, a2)
746+
if p goto L2 else goto L1
747+
L1:
748+
""";
749+
Assert.assertEquals(expected, function.toStr(new StringBuilder(), false).toString());
750+
new ExitSSA(function);
751+
expected = """
752+
L0:
753+
arg p
754+
a1 = 42
755+
b1 = 24
756+
a2 = a1
757+
b2 = b1
758+
goto L2
759+
L2:
760+
a2_6 = a2
761+
a2 = b2
762+
b2 = a2_6
763+
b2_7 = b2
764+
b2 = b2
765+
if p goto L2 else goto L1
766+
L1:
767+
""";
768+
Assert.assertEquals(expected, function.toStr(new StringBuilder(), false).toString());
769+
}
770+
628771
}

0 commit comments

Comments
 (0)