Skip to content

Commit bf7feec

Browse files
FrottyFrotty
committed
reduce temp var creation during flatten step, re-enable test
1 parent 4a371a7 commit bf7feec

3 files changed

Lines changed: 164 additions & 5 deletions

File tree

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/Flatten.java‎

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public ImLExpr getExpr() {
178178

179179
public static class MultiResult {
180180

181-
final List<ImStmt> stmts;
181+
public final List<ImStmt> stmts;
182182
final List<ImExpr> exprs;
183183

184184
public MultiResult(List<ImStmt> stmts, List<ImExpr> exprs) {
@@ -488,6 +488,49 @@ public static Result flatten(ImCompiletimeExpr e, ImTranslator translator, ImFun
488488
}
489489

490490

491+
private static boolean isEffectivelyPure(ImExpr expr) {
492+
if (expr.attrPurity() instanceof Pure) {
493+
return true;
494+
}
495+
return expr instanceof ImConst;
496+
}
497+
498+
private static boolean isUnchangedVarAccess(ImExpr expr, List<? extends Result> laterResults, int startIndex) {
499+
if (!(expr instanceof ImVarAccess)) {
500+
return false;
501+
}
502+
503+
ImVar target = ((ImVarAccess) expr).getVar();
504+
for (int i = startIndex; i < laterResults.size(); i++) {
505+
for (ImStmt stmt : laterResults.get(i).stmts) {
506+
if (writesVar(stmt, target)) {
507+
return false;
508+
}
509+
}
510+
}
511+
512+
return true;
513+
}
514+
515+
private static boolean writesVar(ImStmt stmt, ImVar var) {
516+
class WriteDetector extends ImStmt.DefaultVisitor {
517+
boolean writes = false;
518+
519+
@Override
520+
public void visit(ImSet e) {
521+
super.visit(e);
522+
if (e.getLeft() instanceof ImVarAccess
523+
&& ((ImVarAccess) e.getLeft()).getVar() == var) {
524+
writes = true;
525+
}
526+
}
527+
}
528+
529+
WriteDetector detector = new WriteDetector();
530+
stmt.accept(detector);
531+
return detector.writes;
532+
}
533+
491534
private static MultiResult flattenExprs(ImTranslator t, ImFunction f, ImExpr... exprs) {
492535
return flattenExprs(t, f, Arrays.asList(exprs));
493536
}
@@ -510,7 +553,8 @@ private static MultiResult flattenExprs(ImTranslator t, ImFunction f, List<ImExp
510553
Result r = results.get(i);
511554

512555
stmts.addAll(r.stmts);
513-
if (r.expr.attrPurity() instanceof Pure
556+
if (isEffectivelyPure(r.expr)
557+
|| isUnchangedVarAccess(r.expr, results, i + 1)
514558
|| i >= withStmts) {
515559
newExprs.add(r.expr);
516560
} else {
@@ -541,7 +585,8 @@ private static MultiResultL flattenExprsL(ImTranslator t, ImFunction f, List<ImL
541585
ResultL r = results.get(i);
542586

543587
stmts.addAll(r.stmts);
544-
if (r.expr.attrPurity() instanceof Pure
588+
if (isEffectivelyPure(r.expr)
589+
|| isUnchangedVarAccess(r.expr, results, i + 1)
545590
|| i >= withStmts) {
546591
newExprs.add(r.getExpr());
547592
} else {

‎de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FlattenTests.java‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
package tests.wurstscript.tests;
2+
3+
import de.peeeq.wurstscript.RunArgs;
4+
import de.peeeq.wurstscript.ast.Ast;
5+
import de.peeeq.wurstscript.jassIm.*;
6+
import de.peeeq.wurstscript.translation.imtranslation.CallType;
7+
import de.peeeq.wurstscript.translation.imtranslation.Flatten;
8+
import de.peeeq.wurstscript.translation.imtranslation.Flatten.MultiResult;
9+
import de.peeeq.wurstscript.translation.imtranslation.ImTranslator;
10+
import org.testng.annotations.Test;
11+
12+
import java.lang.reflect.Method;
13+
import java.util.Arrays;
14+
import java.util.Collections;
15+
16+
import static org.testng.Assert.*;
17+
18+
public class FlattenTests {
19+
20+
@Test
21+
public void avoid_extra_temp_for_var_access() throws Exception {
22+
ImTranslator translator = new ImTranslator(Ast.WurstModel(), true, new RunArgs());
23+
de.peeeq.wurstscript.ast.Element trace = Ast.NoExpr();
24+
ImVar sourceVar = JassIm.ImVar(trace, JassIm.ImSimpleType("int"), "source", false);
25+
ImVar targetVar = JassIm.ImVar(trace, JassIm.ImSimpleType("int"), "target", false);
26+
27+
ImExpr simpleAccess = JassIm.ImVarAccess(sourceVar);
28+
ImExpr expressionWithStatements = JassIm.ImStatementExpr(
29+
JassIm.ImStmts(JassIm.ImSet(trace, JassIm.ImVarAccess(targetVar), JassIm.ImIntVal(1))),
30+
JassIm.ImVarAccess(targetVar));
31+
32+
ImFunction function = JassIm.ImFunction(trace, "test", JassIm.ImTypeVars(), JassIm.ImVars(), JassIm.ImVoid(),
33+
JassIm.ImVars(sourceVar, targetVar), JassIm.ImStmts(), Collections.emptyList());
34+
35+
Method flattenExprs = Flatten.class.getDeclaredMethod("flattenExprs", ImTranslator.class, ImFunction.class, java.util.List.class);
36+
flattenExprs.setAccessible(true);
37+
38+
MultiResult result = (MultiResult) flattenExprs.invoke(null, translator, function, Arrays.asList(simpleAccess, expressionWithStatements));
39+
40+
assertEquals(function.getLocals().size(), 2, "no extra locals should be introduced");
41+
assertSame(simpleAccess, result.expr(0), "the original access should be reused");
42+
}
43+
44+
@Test
45+
public void add_temp_for_impure_expr_with_followup_statements() throws Exception {
46+
ImTranslator translator = new ImTranslator(Ast.WurstModel(), true, new RunArgs());
47+
de.peeeq.wurstscript.ast.Element trace = Ast.NoExpr();
48+
ImVar sourceVar = JassIm.ImVar(trace, JassIm.ImSimpleType("int"), "source", false);
49+
ImVar targetVar = JassIm.ImVar(trace, JassIm.ImSimpleType("int"), "target", false);
50+
51+
ImFunction impureCallee = JassIm.ImFunction(trace, "impure", JassIm.ImTypeVars(), JassIm.ImVars(),
52+
JassIm.ImSimpleType("int"), JassIm.ImVars(), JassIm.ImStmts(), Collections.emptyList());
53+
ImExpr impureCall = JassIm.ImFunctionCall(trace, impureCallee, JassIm.ImTypeArguments(), JassIm.ImExprs(),
54+
false, CallType.NORMAL);
55+
56+
ImExpr expressionWithStatements = JassIm.ImStatementExpr(
57+
JassIm.ImStmts(JassIm.ImSet(trace, JassIm.ImVarAccess(targetVar), JassIm.ImIntVal(1))),
58+
JassIm.ImVarAccess(targetVar));
59+
60+
ImFunction function = JassIm.ImFunction(trace, "test", JassIm.ImTypeVars(), JassIm.ImVars(), JassIm.ImVoid(),
61+
JassIm.ImVars(sourceVar, targetVar), JassIm.ImStmts(), Collections.emptyList());
62+
63+
Method flattenExprs = Flatten.class.getDeclaredMethod("flattenExprs", ImTranslator.class, ImFunction.class, java.util.List.class);
64+
flattenExprs.setAccessible(true);
65+
66+
MultiResult result = (MultiResult) flattenExprs.invoke(null, translator, function, Arrays.asList(impureCall, expressionWithStatements));
67+
68+
assertEquals(function.getLocals().size(), 3, "a temporary should be added for the impure expression");
69+
assertEquals(result.stmts.size(), 2, "flattening should produce a temp assignment followed by the statement expr body");
70+
71+
ImExpr flattenedImpure = result.expr(0);
72+
assertTrue(flattenedImpure instanceof ImVarAccess, "impure call should be replaced with temp access");
73+
ImVar tempVar = ((ImVarAccess) flattenedImpure).getVar();
74+
assertNotSame(tempVar, sourceVar);
75+
assertNotSame(tempVar, targetVar);
76+
77+
ImStmt tempAssignment = result.stmts.get(0);
78+
assertTrue(tempAssignment instanceof ImSet, "first statement should assign the impure result to the temp var");
79+
ImLExpr left = ((ImSet) tempAssignment).getLeft();
80+
assertTrue(left instanceof ImVarAccess);
81+
assertSame(((ImVarAccess) left).getVar(), tempVar);
82+
}
83+
84+
@Test
85+
public void add_temp_when_followup_writes_accessed_var() throws Exception {
86+
ImTranslator translator = new ImTranslator(Ast.WurstModel(), true, new RunArgs());
87+
de.peeeq.wurstscript.ast.Element trace = Ast.NoExpr();
88+
ImVar shared = JassIm.ImVar(trace, JassIm.ImSimpleType("int"), "shared", false);
89+
90+
ImExpr initialRead = JassIm.ImVarAccess(shared);
91+
ImExpr statementWithWrite = JassIm.ImStatementExpr(
92+
JassIm.ImStmts(JassIm.ImSet(trace, JassIm.ImVarAccess(shared), JassIm.ImIntVal(4))),
93+
JassIm.ImIntVal(2));
94+
95+
ImFunction function = JassIm.ImFunction(trace, "test", JassIm.ImTypeVars(), JassIm.ImVars(), JassIm.ImVoid(),
96+
JassIm.ImVars(shared), JassIm.ImStmts(), Collections.emptyList());
97+
98+
Method flattenExprs = Flatten.class.getDeclaredMethod("flattenExprs", ImTranslator.class, ImFunction.class, java.util.List.class);
99+
flattenExprs.setAccessible(true);
100+
101+
MultiResult result = (MultiResult) flattenExprs.invoke(null, translator, function, Arrays.asList(initialRead, statementWithWrite));
102+
103+
assertEquals(function.getLocals().size(), 2, "flattening should introduce a temp for the first argument");
104+
assertEquals(result.stmts.size(), 2, "flattening should store the first arg before executing later statements");
105+
106+
ImExpr firstExpr = result.expr(0);
107+
assertTrue(firstExpr instanceof ImVarAccess, "first expression should be rewritten to a temp access");
108+
ImVar tempVar = ((ImVarAccess) firstExpr).getVar();
109+
assertNotSame(tempVar, shared);
110+
111+
ImStmt firstStmt = result.stmts.get(0);
112+
assertTrue(firstStmt instanceof ImSet, "first statement should capture the original value");
113+
assertSame(((ImVarAccess) ((ImSet) firstStmt).getLeft()).getVar(), tempVar);
114+
}
115+
}

‎de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,6 @@ public void test_tempVarRemover() throws IOException {
427427
}
428428

429429
@Test
430-
@Ignore // This test was for a rewrite that caused an infinite loop in the optimizer.
431430
public void test_mult2rewrite() throws IOException {
432431
test().lines(
433432
"package test",
@@ -442,7 +441,7 @@ public void test_mult2rewrite() throws IOException {
442441
"endpackage");
443442
String output = Files.toString(new File("./test-output/OptimizerTests_test_mult2rewrite_inlopt.j"), Charsets.UTF_8);
444443

445-
assertTrue(!output.contains("blub_a") && !(output.contains("blub_b") && !output.contains("blub_c")));
444+
assertTrue(output.contains("blub_a") && !(output.contains("blub_b") && !output.contains("blub_c")));
446445
}
447446

448447
@Test

0 commit comments

Comments
 (0)