Skip to content

Commit 5ad7edb

Browse files
committed
Avoid materializing unused tuple fields
1 parent 0e83860 commit 5ad7edb

2 files changed

Lines changed: 164 additions & 0 deletions

File tree

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateTuples.java

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,17 @@ private static ImType getFirstType(ImType t) {
411411
private static void toTupleExpressions(ImStmts body, ImTranslator translator, ImFunction f) {
412412
Replacer replacer = new Replacer();
413413
body.accept(new Element.DefaultVisitor() {
414+
@Override
415+
public void visit(ImTupleSelection selection) {
416+
ImExpr selectedStorage = selectTupleStorageComponent(selection, translator, f);
417+
if (selectedStorage != null) {
418+
replacer.replace(selection, selectedStorage);
419+
selectedStorage.accept(this);
420+
return;
421+
}
422+
super.visit(selection);
423+
}
424+
414425
@Override
415426
public void visit(ImNull n) {
416427
// Expand null<⦅T1, T2, ...⦆> ==> <null<T1>, null<T2>, ...>
@@ -566,6 +577,86 @@ public void visit(ImMethodCall mc) {
566577
});
567578
}
568579

580+
/**
581+
* Select tuple storage before expanding it. Expanding first turns one array/member read into
582+
* reads of every scalar backing variable, which then have to be preserved through discard
583+
* calls because those reads can fail. A source-level field read only needs the selected
584+
* backing component.
585+
*/
586+
private static @org.eclipse.jdt.annotation.Nullable ImExpr selectTupleStorageComponent(
587+
ImTupleSelection selection, ImTranslator translator, ImFunction f) {
588+
List<Integer> componentPath = new ArrayList<>();
589+
ImExpr storage = selection;
590+
while (storage instanceof ImTupleSelection current) {
591+
if (current.isUsedAsLValue()) {
592+
return null;
593+
}
594+
componentPath.add(current.getTupleIndex());
595+
storage = current.getTupleExpr();
596+
}
597+
Collections.reverse(componentPath);
598+
599+
ImExpr expanded;
600+
ImStmts prelude = JassIm.ImStmts();
601+
if (storage instanceof ImVarAccess access && access.attrTyp() instanceof ImTupleType) {
602+
VarsForTupleResult selected = selectTupleComponent(
603+
translator.getVarsForTuple(access.getVar()), componentPath);
604+
if (selected == null) {
605+
return null;
606+
}
607+
expanded = selected.<ImExpr>map(
608+
parts -> JassIm.ImTupleExpr(parts.collect(Collectors.toCollection(JassIm::ImExprs))),
609+
JassIm::ImVarAccess);
610+
} else if (storage instanceof ImVarArrayAccess access
611+
&& access.attrTyp() instanceof ImTupleType) {
612+
ImExprs indexes = captureIndexesOnceIfNeeded(access.getIndexes(), prelude, f);
613+
VarsForTupleResult selected = selectTupleComponent(
614+
translator.getVarsForTuple(access.getVar()), componentPath);
615+
if (selected == null) {
616+
return null;
617+
}
618+
expanded = selected.<ImExpr>map(
619+
parts -> JassIm.ImTupleExpr(parts.collect(Collectors.toCollection(JassIm::ImExprs))),
620+
var -> JassIm.ImVarArrayAccess(access.getTrace(), var, indexes.copy()));
621+
} else if (storage instanceof ImMemberAccess access
622+
&& access.attrTyp() instanceof ImTupleType) {
623+
boolean indexesAreEffectful = access.getIndexes().stream()
624+
.anyMatch(SideEffectAnalyzer::quickcheckHasSideeffects);
625+
ImExpr receiver = captureOnceIfNeeded(access.getReceiver(), "tupleReceiver", prelude,
626+
f, indexesAreEffectful);
627+
ImExprs indexes = captureIndexesOnceIfNeeded(access.getIndexes(), prelude, f);
628+
VarsForTupleResult selected = selectTupleComponent(
629+
translator.getVarsForTuple(access.getVar()), componentPath);
630+
if (selected == null) {
631+
return null;
632+
}
633+
expanded = selected.<ImExpr>map(
634+
parts -> JassIm.ImTupleExpr(parts.collect(Collectors.toCollection(JassIm::ImExprs))),
635+
var -> JassIm.ImMemberAccess(access.getTrace(), receiver.copy(),
636+
access.getTypeArguments().copy(), var, indexes.copy()));
637+
} else {
638+
return null;
639+
}
640+
641+
if (prelude.isEmpty()) {
642+
return expanded;
643+
}
644+
return JassIm.ImStatementExpr(prelude, expanded);
645+
}
646+
647+
private static @org.eclipse.jdt.annotation.Nullable VarsForTupleResult selectTupleComponent(
648+
VarsForTupleResult tuple, List<Integer> componentPath) {
649+
VarsForTupleResult selected = tuple;
650+
for (int index : componentPath) {
651+
if (!(selected instanceof ImTranslator.TupleResult tupleResult)
652+
|| index < 0 || index >= tupleResult.getItems().size()) {
653+
return null;
654+
}
655+
selected = tupleResult.getItems().get(index);
656+
}
657+
return selected;
658+
}
659+
569660
private static ImExpr captureOnceIfNeeded(ImExpr expr, String name, ImStmts stmts, ImFunction f,
570661
boolean forceCapture) {
571662
if (!forceCapture && !SideEffectAnalyzer.quickcheckHasSideeffects(expr)) {

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,16 @@ private String compiledLua(String testName) throws IOException {
4343
return Files.toString(new File("test-output/lua/LuaBackendAuditTests_" + testName + ".lua"), Charsets.UTF_8);
4444
}
4545

46+
private String luaFunctionBody(String compiled, String functionName) {
47+
java.util.regex.Matcher matcher = java.util.regex.Pattern.compile(
48+
"function " + java.util.regex.Pattern.quote(functionName)
49+
+ "\\([^)]*\\)\\s*\\R(.*?)\\Rend",
50+
java.util.regex.Pattern.DOTALL)
51+
.matcher(compiled);
52+
assertTrue("expected generated Lua function " + functionName, matcher.find());
53+
return matcher.group(1);
54+
}
55+
4656
private String compileOptimizedLua(String testName, String... lines) {
4757
RunArgs runArgs = new RunArgs().with("-lua", "-inline", "-localOptimizations");
4858
return compileLuaWithRunArgs(testName, runArgs, false, lines);
@@ -209,6 +219,69 @@ public void tuplesAreScalarizedWithoutLuaAllocations() throws IOException {
209219
assertFalse("tuple arrays must be split into scalar arrays", compiled.contains("__wurst_arrIndex("));
210220
}
211221

222+
@Test
223+
public void tupleFieldReadsOnlyLoadTheSelectedStorageComponent() throws IOException {
224+
String[] source = {
225+
"package Test",
226+
"native testSuccess()",
227+
"tuple vec3(real x, real y, real z)",
228+
"tuple segment(vec3 start, vec3 finish)",
229+
"vec3 array points",
230+
"segment array segments",
231+
"int indexCalls",
232+
"@noinline function nextIndex() returns int",
233+
" indexCalls++",
234+
" return 2",
235+
"@noinline function readAt(int index) returns real",
236+
" return points[index].z",
237+
"@noinline function readAtNext() returns real",
238+
" return points[nextIndex()].z",
239+
"@noinline function readNested(int index) returns real",
240+
" return segments[index].finish.y",
241+
"class Entity",
242+
" vec3 pos",
243+
"@noinline function readPos(Entity entity) returns real",
244+
" return entity.pos.z",
245+
"init",
246+
" points[2] = vec3(1., 2., 3.)",
247+
" segments[2] = segment(vec3(4., 5., 6.), vec3(7., 8., 9.))",
248+
" let entity = new Entity()",
249+
" entity.pos = vec3(4., 5., 6.)",
250+
" if readAt(2) == 3. and readAtNext() == 3. and indexCalls == 1",
251+
" and readNested(2) == 8. and readPos(entity) == 6.",
252+
" testSuccess()"
253+
};
254+
test().testLua(true).executeProg().lines(source);
255+
256+
String compiled = compileOptimizedLua(
257+
"tupleFieldReadsOnlyLoadTheSelectedStorageComponentOptimized", source);
258+
String plainArrayRead = luaFunctionBody(compiled, "readAt");
259+
String effectfulArrayRead = luaFunctionBody(compiled, "readAtNext");
260+
String nestedArrayRead = luaFunctionBody(compiled, "readNested");
261+
String memberRead = luaFunctionBody(compiled, "readPos");
262+
263+
assertTrue(plainArrayRead.contains("points_z["));
264+
assertFalse(plainArrayRead.contains("points_x["));
265+
assertFalse(plainArrayRead.contains("points_y["));
266+
assertTrue(effectfulArrayRead.contains("points_z["));
267+
assertFalse(effectfulArrayRead.contains("points_x["));
268+
assertFalse(effectfulArrayRead.contains("points_y["));
269+
assertEquals("a selected tuple-array field must evaluate its index exactly once",
270+
1, effectfulArrayRead.split("nextIndex\\(", -1).length - 1);
271+
assertTrue(nestedArrayRead.contains("segments_finish_y["));
272+
assertFalse(nestedArrayRead.contains("segments_start_"));
273+
assertFalse(nestedArrayRead.contains("segments_finish_x["));
274+
assertFalse(nestedArrayRead.contains("segments_finish_z["));
275+
assertTrue(memberRead.contains("Entity_pos_z_storage["));
276+
assertFalse(memberRead.contains("Entity_pos_x_storage["));
277+
assertFalse(memberRead.contains("Entity_pos_y_storage["));
278+
assertFalse("selected tuple storage reads must not need discard helpers",
279+
plainArrayRead.contains("__wurst_tuple_discard_")
280+
|| effectfulArrayRead.contains("__wurst_tuple_discard_")
281+
|| nestedArrayRead.contains("__wurst_tuple_discard_")
282+
|| memberRead.contains("__wurst_tuple_discard_"));
283+
}
284+
212285
@Test
213286
public void tupleMemberArrayAssignmentCapturesIndex() throws IOException {
214287
test().testLua(true).luaOnly(false).executeProg().lines(

0 commit comments

Comments
 (0)