From 0af2b65298e13ce4e64be6ab7af4b7ccc8e58864 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sun, 26 Jul 2026 17:56:36 +0200 Subject: [PATCH 1/6] consider locality for optimizations --- .../optimizer/BranchMerger.java | 3 + .../optimizer/ConstantAndCopyPropagation.java | 15 +- .../optimizer/FunctionSplitter.java | 6 +- .../optimizer/LocalMerger.java | 12 + .../optimizer/LocalPlayerContextAnalyzer.java | 285 ++++++++++++++++++ .../optimizer/SimpleRewrites.java | 20 +- .../optimizer/TempMerger.java | 8 + .../translation/imoptimizer/ImInliner.java | 12 + .../UselessFunctionCallsRemover.java | 7 +- .../tests/LuaBackendAuditTests.java | 25 ++ .../wurstscript/tests/OptimizerTests.java | 170 +++++++++++ 11 files changed, 554 insertions(+), 9 deletions(-) create mode 100644 de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/BranchMerger.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/BranchMerger.java index 6c8aa134f..722cef1da 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/BranchMerger.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/BranchMerger.java @@ -13,6 +13,7 @@ */ public class BranchMerger implements OptimizerPass { private SideEffectAnalyzer sideEffectAnalyzer; + private LocalPlayerContextAnalyzer localPlayerContextAnalyzer; public int branchesMerged = 0; @Override @@ -20,6 +21,7 @@ public int optimize(ImTranslator trans) { branchesMerged = 0; ImProg prog = trans.getImProg(); this.sideEffectAnalyzer = new SideEffectAnalyzer(prog); + this.localPlayerContextAnalyzer = new LocalPlayerContextAnalyzer(prog); for (ImFunction func : prog.getFunctions()) { optimizeFunc(func); @@ -52,6 +54,7 @@ public void visit(ImStmts stmts) { // if first statement in both branches is the same // and has no side-effects that could affect the if-condition: if (firstStmtThen.structuralEquals(firstStmtElse) + && !localPlayerContextAnalyzer.isLocalPlayerDependent(ifStmt.getCondition()) && !sideEffectAnalyzer.mightAffect(firstStmtThen, ifStmt.getCondition())) { // remove statements ifStmt.getThenBlock().remove(0); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ConstantAndCopyPropagation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ConstantAndCopyPropagation.java index 32d6141be..74c058fa7 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ConstantAndCopyPropagation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ConstantAndCopyPropagation.java @@ -19,9 +19,11 @@ public class ConstantAndCopyPropagation implements OptimizerPass { private int totalPropagated = 0; + private @Nullable LocalPlayerContextAnalyzer localPlayerContextAnalyzer; public int optimize(ImTranslator trans) { ImProg prog = trans.getImProg(); + localPlayerContextAnalyzer = new LocalPlayerContextAnalyzer(prog); totalPropagated = 0; for (ImFunction func : ImHelper.calculateFunctionsOfProg(prog)) { @@ -144,6 +146,11 @@ void optimizeFunc(ImFunction func) { rewriteCode(cfg, knowledge); } + void optimizeFunc(ImFunction func, LocalPlayerContextAnalyzer analyzer) { + localPlayerContextAnalyzer = analyzer; + optimizeFunc(func); + } + private void rewriteCode(ControlFlowGraph cfg, Map knowledge) { for (Node node : cfg.getNodes()) { ImStmt stmt = node.getStmt(); @@ -311,7 +318,13 @@ private void analyzeComponent(List scc, Map knowledge) { // Constant folding is intentionally centralized in SimpleRewrites. // This pass performs propagation only to keep fold semantics in one place. - if (right instanceof ImConst) { + if (localPlayerContextAnalyzer != null + && localPlayerContextAnalyzer.isLocalPlayerDependent(right)) { + // Keep GetLocalPlayer-derived values at their original + // variable boundary. This is deliberately conservative: + // later rewrites must still be able to see that boundary. + newValue = null; + } else if (right instanceof ImConst) { newValue = Value.tryValue(right); } else if (right instanceof ImVarAccess) { ImVar varRight = ((ImVarAccess) right).getVar(); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/FunctionSplitter.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/FunctionSplitter.java index 55a91330d..423e92eba 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/FunctionSplitter.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/FunctionSplitter.java @@ -39,9 +39,11 @@ private void optimize() { Preconditions.checkArgument(func.getReturnType() instanceof ImVoid, "func must return void"); // run some basic optimizations first: func.flatten(tr); - new ConstantAndCopyPropagation().optimizeFunc(func); + LocalPlayerContextAnalyzer localPlayerContextAnalyzer = + new LocalPlayerContextAnalyzer(tr.getImProg()); + new ConstantAndCopyPropagation().optimizeFunc(func, localPlayerContextAnalyzer); // new TempMerger().optimizeFunc(func); - new LocalMerger().optimizeFunc(func); + new LocalMerger().optimizeFunc(func, localPlayerContextAnalyzer); Set usedVars = UsedVariables.calculate(func); func.getLocals().removeIf(v -> !usedVars.contains(v)); func.flatten(tr); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java index 038305651..39a8a461b 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java @@ -16,10 +16,12 @@ public class LocalMerger implements OptimizerPass { private int totalLocalsMerged = 0; + private LocalPlayerContextAnalyzer localPlayerContextAnalyzer; @Override public int optimize(ImTranslator trans) { ImProg prog = trans.getImProg(); + localPlayerContextAnalyzer = new LocalPlayerContextAnalyzer(prog); totalLocalsMerged = 0; for (ImFunction func : de.peeeq.wurstscript.translation.imtranslation.ImHelper.calculateFunctionsOfProg(prog)) { if (!func.isNative() && !func.isBj()) { @@ -38,6 +40,11 @@ void optimizeFunc(ImFunction func) { mergeLocals(livenessInfo, func); } + void optimizeFunc(ImFunction func, LocalPlayerContextAnalyzer analyzer) { + localPlayerContextAnalyzer = analyzer; + optimizeFunc(func); + } + private boolean canMerge(ImType a, ImType b) { return a.equalsType(b); } private void mergeLocals(Map> livenessInfo, ImFunction func) { @@ -63,6 +70,11 @@ private void mergeLocals(Map> livenessInfo, ImFunction func) for (ImVar color : colors) { if (!canMerge(color.getType(), v.getType())) continue; + if (localPlayerContextAnalyzer != null + && (localPlayerContextAnalyzer.isLocalPlayerDependent(v) + || localPlayerContextAnalyzer.isLocalPlayerDependent(color))) { + continue; + } boolean conflict = false; for (ImVar neigh : interference.get(v)) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java new file mode 100644 index 000000000..44443d5b1 --- /dev/null +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java @@ -0,0 +1,285 @@ +package de.peeeq.wurstscript.intermediatelang.optimizer; + +import de.peeeq.wurstscript.jassIm.*; +import de.peeeq.wurstscript.translation.imtranslation.ImHelper; + +import java.util.Collections; +import java.util.IdentityHashMap; +import java.util.Set; + +/** + * Conservative, flow-insensitive analysis for values and functions which may + * depend on {@code GetLocalPlayer()}. + * + * Optimizers use this analysis as a barrier. False positives only cost an + * optimization; false negatives could move synchronized work into a + * client-local control-flow region. + */ +public final class LocalPlayerContextAnalyzer { + + private final Set localPlayerDependentVars = + Collections.newSetFromMap(new IdentityHashMap<>()); + private final Set localPlayerDependentReturns = + Collections.newSetFromMap(new IdentityHashMap<>()); + private final Set functionsUsingLocalPlayer = + Collections.newSetFromMap(new IdentityHashMap<>()); + private final Set functionsDirectlyUsingLocalPlayer = + Collections.newSetFromMap(new IdentityHashMap<>()); + + public LocalPlayerContextAnalyzer(ImProg prog) { + analyze(prog); + } + + public boolean isLocalPlayerDependent(Element element) { + if (element == null) { + return false; + } + if (element instanceof ImVarAccess) { + return localPlayerDependentVars.contains(((ImVarAccess) element).getVar()); + } + if (element instanceof ImVarArrayAccess) { + ImVarArrayAccess access = (ImVarArrayAccess) element; + if (localPlayerDependentVars.contains(access.getVar())) { + return true; + } + } + if (element instanceof ImMemberAccess) { + ImMemberAccess access = (ImMemberAccess) element; + if (localPlayerDependentVars.contains(access.getVar())) { + return true; + } + } + if (element instanceof ImFunctionCall) { + ImFunctionCall call = (ImFunctionCall) element; + if (isGetLocalPlayer(call.getFunc()) + || localPlayerDependentReturns.contains(call.getFunc())) { + return true; + } + // Conservatively assume a return value can depend on any argument. + if (isLocalPlayerDependent(call.getArguments())) { + return true; + } + } + if (element instanceof ImMethodCall) { + ImMethodCall call = (ImMethodCall) element; + if (methodReturnsLocalPlayerDependentValue(call.getMethod()) + || isLocalPlayerDependent(call.getReceiver()) + || isLocalPlayerDependent(call.getArguments())) { + return true; + } + } + for (int i = 0; i < element.size(); i++) { + if (isLocalPlayerDependent(element.get(i))) { + return true; + } + } + return false; + } + + public boolean functionUsesLocalPlayer(ImFunction function) { + return function != null + && (isGetLocalPlayer(function) || functionsUsingLocalPlayer.contains(function)); + } + + public boolean functionInliningIsLocalPlayerSensitive(ImFunction function) { + return function != null + && (isGetLocalPlayer(function) + || functionsDirectlyUsingLocalPlayer.contains(function) + || localPlayerDependentReturns.contains(function)); + } + + public boolean isLocalPlayerDependent(ImVar variable) { + return variable != null && localPlayerDependentVars.contains(variable); + } + + public boolean isLocalPlayerSource(ImFunction function) { + return isGetLocalPlayer(function); + } + + private void analyze(ImProg prog) { + boolean changed; + do { + int varsBefore = localPlayerDependentVars.size(); + int returnsBefore = localPlayerDependentReturns.size(); + int functionsBefore = functionsUsingLocalPlayer.size(); + + for (ImFunction function : ImHelper.calculateFunctionsOfProg(prog)) { + analyzeFunction(function); + } + + changed = varsBefore != localPlayerDependentVars.size() + || returnsBefore != localPlayerDependentReturns.size() + || functionsBefore != functionsUsingLocalPlayer.size(); + } while (changed); + } + + private void analyzeFunction(ImFunction function) { + if (isGetLocalPlayer(function)) { + localPlayerDependentReturns.add(function); + functionsUsingLocalPlayer.add(function); + return; + } + if (function.isNative()) { + return; + } + if (containsDirectGetLocalPlayerCall(function.getBody())) { + functionsDirectlyUsingLocalPlayer.add(function); + } + + function.getBody().accept(new ImStmt.DefaultVisitor() { + @Override + public void visit(ImSet set) { + super.visit(set); + if (isLocalPlayerDependent(set.getRight())) { + addAssignedVariables(set.getLeft()); + } + } + + @Override + public void visit(ImReturn returnStmt) { + super.visit(returnStmt); + if (returnStmt.getReturnValue() instanceof ImExpr + && isLocalPlayerDependent((ImExpr) returnStmt.getReturnValue())) { + localPlayerDependentReturns.add(function); + } + } + + @Override + public void visit(ImFunctionCall call) { + super.visit(call); + int count = Math.min(call.getArguments().size(), call.getFunc().getParameters().size()); + for (int i = 0; i < count; i++) { + if (isLocalPlayerDependent(call.getArguments().get(i))) { + localPlayerDependentVars.add(call.getFunc().getParameters().get(i)); + } + } + } + + @Override + public void visit(ImMethodCall call) { + super.visit(call); + if (isLocalPlayerDependent(call.getReceiver()) + || isLocalPlayerDependent(call.getArguments())) { + markMethodParametersLocalPlayerDependent(call.getMethod()); + } + } + }); + + if (containsLocalPlayerUse(function.getBody())) { + functionsUsingLocalPlayer.add(function); + } + } + + private boolean containsLocalPlayerUse(Element element) { + if (element instanceof ImFunctionCall) { + ImFunction called = ((ImFunctionCall) element).getFunc(); + if (isGetLocalPlayer(called) || functionsUsingLocalPlayer.contains(called)) { + return true; + } + } else if (element instanceof ImMethodCall) { + if (methodUsesLocalPlayer(((ImMethodCall) element).getMethod())) { + return true; + } + } else if (element instanceof ImVarAccess + && localPlayerDependentVars.contains(((ImVarAccess) element).getVar())) { + return true; + } else if (element instanceof ImVarArrayAccess + && localPlayerDependentVars.contains(((ImVarArrayAccess) element).getVar())) { + return true; + } else if (element instanceof ImMemberAccess + && localPlayerDependentVars.contains(((ImMemberAccess) element).getVar())) { + return true; + } + for (int i = 0; i < element.size(); i++) { + if (containsLocalPlayerUse(element.get(i))) { + return true; + } + } + return false; + } + + private boolean methodReturnsLocalPlayerDependentValue(ImMethod method) { + if (method == null || method.getImplementation() == null) { + return true; + } + if (localPlayerDependentReturns.contains(method.getImplementation())) { + return true; + } + for (ImMethod subMethod : method.getSubMethods()) { + if (methodReturnsLocalPlayerDependentValue(subMethod)) { + return true; + } + } + return false; + } + + private boolean methodUsesLocalPlayer(ImMethod method) { + if (method == null || method.getImplementation() == null) { + return true; + } + if (functionsUsingLocalPlayer.contains(method.getImplementation())) { + return true; + } + for (ImMethod subMethod : method.getSubMethods()) { + if (methodUsesLocalPlayer(subMethod)) { + return true; + } + } + return false; + } + + private void markMethodParametersLocalPlayerDependent(ImMethod method) { + if (method == null || method.getImplementation() == null) { + return; + } + localPlayerDependentVars.addAll(method.getImplementation().getParameters()); + for (ImMethod subMethod : method.getSubMethods()) { + markMethodParametersLocalPlayerDependent(subMethod); + } + } + + private boolean containsDirectGetLocalPlayerCall(Element element) { + if (element instanceof ImFunctionCall + && isGetLocalPlayer(((ImFunctionCall) element).getFunc())) { + return true; + } + for (int i = 0; i < element.size(); i++) { + if (containsDirectGetLocalPlayerCall(element.get(i))) { + return true; + } + } + return false; + } + + private void addAssignedVariables(ImLExpr left) { + if (left instanceof ImVarAccess) { + localPlayerDependentVars.add(((ImVarAccess) left).getVar()); + } else if (left instanceof ImVarArrayAccess) { + localPlayerDependentVars.add(((ImVarArrayAccess) left).getVar()); + } else if (left instanceof ImMemberAccess) { + localPlayerDependentVars.add(((ImMemberAccess) left).getVar()); + } else if (left instanceof ImTupleSelection) { + ImExpr tupleExpr = ((ImTupleSelection) left).getTupleExpr(); + if (tupleExpr instanceof ImLExpr) { + addAssignedVariables((ImLExpr) tupleExpr); + } + } else if (left instanceof ImTupleExpr) { + for (ImExpr expr : ((ImTupleExpr) left).getExprs()) { + if (expr instanceof ImLExpr) { + addAssignedVariables((ImLExpr) expr); + } + } + } else if (left instanceof ImStatementExpr) { + ImExpr expr = ((ImStatementExpr) left).getExpr(); + if (expr instanceof ImLExpr) { + addAssignedVariables((ImLExpr) expr); + } + } + } + + private static boolean isGetLocalPlayer(ImFunction function) { + return function != null + && function.isNative() + && "GetLocalPlayer".equals(function.getName()); + } +} diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/SimpleRewrites.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/SimpleRewrites.java index 6d8d522d5..37d028906 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/SimpleRewrites.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/SimpleRewrites.java @@ -205,10 +205,10 @@ private void optimizeOpCall(ImOperatorCall opc) { opc.replaceBy(JassIm.ImBoolVal(result)); } else if (left instanceof ImBoolVal) { boolean b1 = ((ImBoolVal) left).getValB(); - wasViable = replaceBoolTerm(opc, right, b1); + wasViable = replaceBoolTerm(opc, right, b1, true); } else if (right instanceof ImBoolVal) { boolean b2 = ((ImBoolVal) right).getValB(); - wasViable = replaceBoolTerm(opc, left, b2); + wasViable = replaceBoolTerm(opc, left, b2, false); } else if (isNumberLiteral(left) && isNumberLiteral(right)) { // If any side is real (or the op is a real op), fold as real; otherwise fold as int. boolean foldAsReal = @@ -613,10 +613,16 @@ private boolean optimizeIntInt(ImOperatorCall opc, boolean wasViable, ImIntVal l return wasViable; } - private boolean replaceBoolTerm(ImOperatorCall opc, ImExpr expr, boolean b2) { + private boolean replaceBoolTerm(ImOperatorCall opc, ImExpr expr, boolean constant, boolean constantOnLeft) { switch (opc.getOp()) { case OR: - if (b2) { + if (constant) { + if (!constantOnLeft) { + // x or true still evaluates x. Replacing the expression + // with true would discard calls, traps, allocations, and + // local-player-dependent reads. + return false; + } opc.replaceBy(JassIm.ImBoolVal(true)); } else { expr.setParent(null); @@ -624,10 +630,14 @@ private boolean replaceBoolTerm(ImOperatorCall opc, ImExpr expr, boolean b2) { } break; case AND: - if (b2) { + if (constant) { expr.setParent(null); opc.replaceBy(expr); } else { + if (!constantOnLeft) { + // x and false still evaluates x. + return false; + } opc.replaceBy(JassIm.ImBoolVal(false)); } break; diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/TempMerger.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/TempMerger.java index bedce28df..373699d53 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/TempMerger.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/TempMerger.java @@ -22,6 +22,7 @@ public class TempMerger implements OptimizerPass { private int totalMerged = 0; + private @Nullable LocalPlayerContextAnalyzer localPlayerContextAnalyzer; @Override @@ -35,6 +36,7 @@ public String getName() { @Override public int optimize(ImTranslator trans) { ImProg prog = trans.getImProg(); + localPlayerContextAnalyzer = new LocalPlayerContextAnalyzer(prog); totalMerged = 0; trans.assertProperties(AssertProperty.FLAT, AssertProperty.NOTUPLES); prog.clearAttributes(); @@ -392,6 +394,12 @@ private boolean isMergable(ImVar left, ImExpr e) { return false; } + if (localPlayerContextAnalyzer != null + && (localPlayerContextAnalyzer.isLocalPlayerDependent(left) + || localPlayerContextAnalyzer.isLocalPlayerDependent(e))) { + return false; + } + // --- FIX START --- // Never inline expressions containing function calls. // This prevents TempMerger from fighting with the Flatten pass, which diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java index 40640b61f..fde47196b 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java @@ -4,6 +4,7 @@ import com.google.common.collect.Maps; import com.google.common.collect.Sets; import de.peeeq.wurstscript.WLogger; +import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerContextAnalyzer; import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.translation.imtranslation.*; import de.peeeq.wurstscript.types.TypesHelper; @@ -30,6 +31,7 @@ public class ImInliner { private final Set done = Sets.newLinkedHashSet(); private final Map containsFuncRefCache = Maps.newLinkedHashMap(); private final double inlineTreshold = 50; + private LocalPlayerContextAnalyzer localPlayerContextAnalyzer; static { dontInline.add("SetPlayerAllianceStateAllyBJ"); @@ -44,6 +46,7 @@ public ImInliner(ImTranslator translator) { public void doInlining() { prog.flatten(translator); + localPlayerContextAnalyzer = new LocalPlayerContextAnalyzer(prog); collectInlinableFunctions(); rateInlinableFunctions(); inlineFunctions(); @@ -119,6 +122,9 @@ private String skipReason(ImFunction caller, ImFunctionCall call, ImFunction f) if (translator.isLuaTarget() && containsFuncRef(f)) { return "lua_callback_funcref_barrier"; } + if (localPlayerContextAnalyzer.functionInliningIsLocalPlayerSensitive(f)) { + return "local_player_context_barrier"; + } if (!inlinableFunctions.contains(f)) { return "not_in_inlinable_set"; } @@ -340,6 +346,12 @@ private boolean shouldInline(ImFunction caller, ImFunctionCall call, ImFunction // Keeping them as standalone calls avoids callback context/vararg scope breakage. return false; } + if (localPlayerContextAnalyzer.functionInliningIsLocalPlayerSensitive(f)) { + // Keep the call boundary around GetLocalPlayer-dependent code. + // Inlining is normally context-preserving, but future local + // rewrites must not gain an opportunity to move its body. + return false; + } if (isLuaTypeCastingCompatFunction(f)) { // In Lua these compat wrappers are rewritten to object index helpers. // If they are inlined beforehand, old TypeCasting bodies leak through. diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/UselessFunctionCallsRemover.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/UselessFunctionCallsRemover.java index fb0e060b3..f473dba55 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/UselessFunctionCallsRemover.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/UselessFunctionCallsRemover.java @@ -1,6 +1,7 @@ package de.peeeq.wurstscript.translation.imoptimizer; import de.peeeq.wurstscript.WurstOperator; +import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerContextAnalyzer; import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.translation.imtranslation.ImHelper; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; @@ -18,10 +19,12 @@ */ public class UselessFunctionCallsRemover implements OptimizerPass { public int totalCallsRemoved = 0; + private LocalPlayerContextAnalyzer localPlayerContextAnalyzer; public int optimize(ImTranslator trans) { totalCallsRemoved = 0; ImProg prog = trans.getImProg(); + localPlayerContextAnalyzer = new LocalPlayerContextAnalyzer(prog); for (ImFunction func : prog.getFunctions()) { optimizeFunc(func, trans); } @@ -105,7 +108,9 @@ private void optimizeElement(Element e) { } private boolean isNativeWithoutSideEffect(ImFunction func) { - return func.isNative() && isFunctionWithoutSideEffect(func.getName()); + return func.isNative() + && !localPlayerContextAnalyzer.isLocalPlayerSource(func) + && isFunctionWithoutSideEffect(func.getName()); } /** diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index 56ae55bc8..c07f83e76 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -53,6 +53,31 @@ private String compileLuaWithRunArgs(String testName, RunArgs runArgs, String... return result.toString(); } + @Test + public void localPlayerEffectfulBooleanOperandSurvivesOptimization() { + String compiled = compileOptimizedLua( + "localPlayerEffectfulBooleanOperandSurvivesOptimization", + "type player extends handle", + "package Test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "integer calls = 0", + "@noinline function localProbe() returns boolean", + " calls++", + " return GetLocalPlayer() == Player(0)", + "init", + " if localProbe() or true", + " print(calls)" + ); + + int definitionOrCall = compiled.indexOf("localProbe()"); + assertTrue("optimized Lua must emit the local-player probe", + definitionOrCall >= 0); + assertTrue("optimized Lua must retain the call to the local-player probe", + compiled.indexOf("localProbe()", definitionOrCall + 1) >= 0); + } + /** * A bare {@code return} inside a vararg loop used to truncate the literal * {@code end} that closed the loop, producing unparseable Lua diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index f00f4f765..e675e28fd 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -1755,4 +1755,174 @@ public void realRealMixed_equality_roundTripGuard() throws Exception { assertFalse(out.matches("(?s).*E[-+]?\\d+.*")); } + @Test + public void effectfulBooleanOperandsMustNotBeDiscarded() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "integer calls = 0", + "@noinline function probeOr() returns boolean", + " calls++", + " return GetLocalPlayer() == Player(0)", + "@noinline function probeAnd() returns boolean", + " calls++", + " return GetLocalPlayer() == Player(0)", + "init", + " if probeOr() or true", + " print(calls)", + " if probeAnd() and false", + " print(calls)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_effectfulBooleanOperandsMustNotBeDiscarded_opt.j"), + Charsets.UTF_8); + assertTrue(optimized.contains("if probeOr() or true"), + "x or true must still evaluate effectful x"); + assertTrue(optimized.contains("if probeAnd() and false"), + "x and false must still evaluate effectful x"); + } + + @Test + public void directGetLocalPlayerConditionMustNotBeDiscarded() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "init", + " if (GetLocalPlayer() == Player(0)) or true", + " print(1)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_directGetLocalPlayerConditionMustNotBeDiscarded_opt.j"), + Charsets.UTF_8); + assertTrue(optimized.contains("GetLocalPlayer()"), + "local-player-dependent expressions must not be discarded"); + } + + @Test + public void synchronizedValueMustNotMoveIntoLocalPlayerBranch() throws Exception { + test().lines( + "type player extends handle", + "type unit extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "@extern native CreateUnit(player p, integer id, real x, real y, real face) returns unit", + "native print(unit u)", + "init", + " unit u = CreateUnit(Player(0), 'hfoo', 0., 0., 0.)", + " player localPlayer = GetLocalPlayer()", + " player playerZero = Player(0)", + " if localPlayer == playerZero", + " print(u)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_synchronizedValueMustNotMoveIntoLocalPlayerBranch_inlopt.j"), + Charsets.UTF_8); + int createUnit = optimized.indexOf("CreateUnit("); + int localCondition = optimized.indexOf("if "); + int use = optimized.indexOf("print(u)"); + assertTrue(createUnit >= 0 && localCondition > createUnit && use > localCondition, + "CreateUnit must remain in synchronized context before the local-player branch"); + } + + @Test + public void branchMergerMustNotHoistAcrossStoredLocalPlayerCondition() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "integer result = 0", + "init", + " player localPlayer = GetLocalPlayer()", + " player alias = localPlayer", + " player playerZero = Player(0)", + " if alias == playerZero", + " result = 7", + " else", + " result = 7", + " print(result)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_branchMergerMustNotHoistAcrossStoredLocalPlayerCondition_opt.j"), + Charsets.UTF_8); + assertTrue(countOccurrences(optimized, "test_result = 7") >= 2, + "identical branches controlled by local-player data must remain separate"); + } + + @Test + public void branchMergerMustTrackLocalPlayerThroughFunctionParameters() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "player remembered", + "integer result = 0", + "@noinline function remember(player p)", + " remembered = p", + "init", + " remember(GetLocalPlayer())", + " player playerZero = Player(0)", + " if remembered == playerZero", + " result = 9", + " else", + " result = 9", + " print(result)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_branchMergerMustTrackLocalPlayerThroughFunctionParameters_opt.j"), + Charsets.UTF_8); + assertTrue(countOccurrences(optimized, "test_result = 9") >= 2, + "GetLocalPlayer taint must flow through call arguments and parameters"); + } + + @Test + public void functionUsingGetLocalPlayerMustNotBeInlined() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@inline function currentPlayer() returns player", + " return GetLocalPlayer()", + "@inline function forwardedPlayer() returns player", + " return currentPlayer()", + "native consume(player p)", + "init", + " consume(currentPlayer())", + " consume(forwardedPlayer())" + ); + + String inlined = Files.toString( + new File("test-output/OptimizerTests_functionUsingGetLocalPlayerMustNotBeInlined_inl.j"), + Charsets.UTF_8); + assertTrue(inlined.contains("call consume(currentPlayer())"), + "functions using GetLocalPlayer must remain explicit calls"); + assertTrue(inlined.contains("call consume(forwardedPlayer())"), + "transitive GetLocalPlayer wrappers must remain explicit calls"); + } + + private static int countOccurrences(String text, String needle) { + int count = 0; + int from = 0; + while ((from = text.indexOf(needle, from)) >= 0) { + count++; + from += needle.length(); + } + return count; + } + } From e492cc9d629495fd350c20bab9834c93d37b8784 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sun, 26 Jul 2026 19:16:25 +0200 Subject: [PATCH 2/6] review fixes --- .../optimizer/BranchMerger.java | 7 +- .../optimizer/ConstantAndCopyPropagation.java | 7 +- .../optimizer/LocalMerger.java | 7 +- .../LocalPlayerAwareOptimizerPass.java | 18 + .../optimizer/LocalPlayerContextAnalyzer.java | 329 +++++++++++------- .../translation/imoptimizer/ImOptimizer.java | 21 +- .../UselessFunctionCallsRemover.java | 7 +- .../wurstscript/tests/OptimizerTests.java | 82 +++++ 8 files changed, 328 insertions(+), 150 deletions(-) create mode 100644 de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerAwareOptimizerPass.java diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/BranchMerger.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/BranchMerger.java index 722cef1da..9548782cd 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/BranchMerger.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/BranchMerger.java @@ -1,7 +1,6 @@ package de.peeeq.wurstscript.intermediatelang.optimizer; import de.peeeq.wurstscript.jassIm.*; -import de.peeeq.wurstscript.translation.imoptimizer.OptimizerPass; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; import java.util.ListIterator; @@ -11,17 +10,17 @@ *

* the input must be a flattened program */ -public class BranchMerger implements OptimizerPass { +public class BranchMerger implements LocalPlayerAwareOptimizerPass { private SideEffectAnalyzer sideEffectAnalyzer; private LocalPlayerContextAnalyzer localPlayerContextAnalyzer; public int branchesMerged = 0; @Override - public int optimize(ImTranslator trans) { + public int optimize(ImTranslator trans, LocalPlayerContextAnalyzer analyzer) { branchesMerged = 0; ImProg prog = trans.getImProg(); this.sideEffectAnalyzer = new SideEffectAnalyzer(prog); - this.localPlayerContextAnalyzer = new LocalPlayerContextAnalyzer(prog); + this.localPlayerContextAnalyzer = analyzer; for (ImFunction func : prog.getFunctions()) { optimizeFunc(func); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ConstantAndCopyPropagation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ConstantAndCopyPropagation.java index 74c058fa7..e0319732b 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ConstantAndCopyPropagation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ConstantAndCopyPropagation.java @@ -5,7 +5,6 @@ import de.peeeq.wurstscript.WurstOperator; import de.peeeq.wurstscript.intermediatelang.optimizer.ControlFlowGraph.Node; import de.peeeq.wurstscript.jassIm.*; -import de.peeeq.wurstscript.translation.imoptimizer.OptimizerPass; import de.peeeq.wurstscript.translation.imtranslation.ImHelper; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; import de.peeeq.wurstscript.types.TypesHelper; @@ -17,13 +16,13 @@ import static de.peeeq.wurstscript.WurstOperator.*; -public class ConstantAndCopyPropagation implements OptimizerPass { +public class ConstantAndCopyPropagation implements LocalPlayerAwareOptimizerPass { private int totalPropagated = 0; private @Nullable LocalPlayerContextAnalyzer localPlayerContextAnalyzer; - public int optimize(ImTranslator trans) { + public int optimize(ImTranslator trans, LocalPlayerContextAnalyzer analyzer) { ImProg prog = trans.getImProg(); - localPlayerContextAnalyzer = new LocalPlayerContextAnalyzer(prog); + localPlayerContextAnalyzer = analyzer; totalPropagated = 0; for (ImFunction func : ImHelper.calculateFunctionsOfProg(prog)) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java index 39a8a461b..f2e02943e 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java @@ -3,7 +3,6 @@ import de.peeeq.datastructures.GraphInterpreter; import de.peeeq.wurstscript.intermediatelang.optimizer.ControlFlowGraph.Node; import de.peeeq.wurstscript.jassIm.*; -import de.peeeq.wurstscript.translation.imoptimizer.OptimizerPass; import de.peeeq.wurstscript.translation.imtranslation.ImHelper; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; import de.peeeq.wurstscript.types.TypesHelper; @@ -14,14 +13,14 @@ import java.util.*; -public class LocalMerger implements OptimizerPass { +public class LocalMerger implements LocalPlayerAwareOptimizerPass { private int totalLocalsMerged = 0; private LocalPlayerContextAnalyzer localPlayerContextAnalyzer; @Override - public int optimize(ImTranslator trans) { + public int optimize(ImTranslator trans, LocalPlayerContextAnalyzer analyzer) { ImProg prog = trans.getImProg(); - localPlayerContextAnalyzer = new LocalPlayerContextAnalyzer(prog); + localPlayerContextAnalyzer = analyzer; totalLocalsMerged = 0; for (ImFunction func : de.peeeq.wurstscript.translation.imtranslation.ImHelper.calculateFunctionsOfProg(prog)) { if (!func.isNative() && !func.isBj()) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerAwareOptimizerPass.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerAwareOptimizerPass.java new file mode 100644 index 000000000..de95b9c5e --- /dev/null +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerAwareOptimizerPass.java @@ -0,0 +1,18 @@ +package de.peeeq.wurstscript.intermediatelang.optimizer; + +import de.peeeq.wurstscript.translation.imoptimizer.OptimizerPass; +import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; + +/** + * An optimizer pass which can share a local-player dependency analysis with + * adjacent passes that preserve the analysis' conservative facts. + */ +public interface LocalPlayerAwareOptimizerPass extends OptimizerPass { + + int optimize(ImTranslator trans, LocalPlayerContextAnalyzer analyzer); + + @Override + default int optimize(ImTranslator trans) { + return optimize(trans, new LocalPlayerContextAnalyzer(trans.getImProg())); + } +} diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java index 44443d5b1..3d8edd37a 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java @@ -3,9 +3,15 @@ import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.translation.imtranslation.ImHelper; +import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.Collections; +import java.util.Deque; import java.util.IdentityHashMap; +import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.function.Consumer; /** * Conservative, flow-insensitive analysis for values and functions which may @@ -25,6 +31,17 @@ public final class LocalPlayerContextAnalyzer { Collections.newSetFromMap(new IdentityHashMap<>()); private final Set functionsDirectlyUsingLocalPlayer = Collections.newSetFromMap(new IdentityHashMap<>()); + private final Set indexedElements = + Collections.newSetFromMap(new IdentityHashMap<>()); + private final Set activeFacts = + Collections.newSetFromMap(new IdentityHashMap<>()); + private final Map> dependents = new IdentityHashMap<>(); + private final Map variableFacts = new IdentityHashMap<>(); + private final Map returnFacts = new IdentityHashMap<>(); + private final Map useFacts = new IdentityHashMap<>(); + private final Set sourceFacts = + Collections.newSetFromMap(new IdentityHashMap<>()); + private final Fact unknownDispatchSource = new Fact(FactKind.SOURCE, null); public LocalPlayerContextAnalyzer(ImProg prog) { analyze(prog); @@ -34,6 +51,9 @@ public boolean isLocalPlayerDependent(Element element) { if (element == null) { return false; } + if (indexedElements.contains(element)) { + return activeFacts.contains(element); + } if (element instanceof ImVarAccess) { return localPlayerDependentVars.contains(((ImVarAccess) element).getVar()); } @@ -56,17 +76,13 @@ public boolean isLocalPlayerDependent(Element element) { return true; } // Conservatively assume a return value can depend on any argument. - if (isLocalPlayerDependent(call.getArguments())) { - return true; - } + return isLocalPlayerDependent(call.getArguments()); } if (element instanceof ImMethodCall) { ImMethodCall call = (ImMethodCall) element; - if (methodReturnsLocalPlayerDependentValue(call.getMethod()) + return methodReturnsLocalPlayerDependentValue(call.getMethod()) || isLocalPlayerDependent(call.getReceiver()) - || isLocalPlayerDependent(call.getArguments())) { - return true; - } + || isLocalPlayerDependent(call.getArguments()); } for (int i = 0; i < element.size(); i++) { if (isLocalPlayerDependent(element.get(i))) { @@ -97,105 +113,18 @@ public boolean isLocalPlayerSource(ImFunction function) { } private void analyze(ImProg prog) { - boolean changed; - do { - int varsBefore = localPlayerDependentVars.size(); - int returnsBefore = localPlayerDependentReturns.size(); - int functionsBefore = functionsUsingLocalPlayer.size(); - - for (ImFunction function : ImHelper.calculateFunctionsOfProg(prog)) { - analyzeFunction(function); + sourceFacts.add(unknownDispatchSource); + for (ImFunction function : ImHelper.calculateFunctionsOfProg(prog)) { + returnFact(function); + useFact(function); + if (isGetLocalPlayer(function)) { + addLocalPlayerSource(function); + } else if (!function.isNative()) { + indexElement(function.getBody(), function); + addDependency(function.getBody(), useFact(function)); } - - changed = varsBefore != localPlayerDependentVars.size() - || returnsBefore != localPlayerDependentReturns.size() - || functionsBefore != functionsUsingLocalPlayer.size(); - } while (changed); - } - - private void analyzeFunction(ImFunction function) { - if (isGetLocalPlayer(function)) { - localPlayerDependentReturns.add(function); - functionsUsingLocalPlayer.add(function); - return; - } - if (function.isNative()) { - return; - } - if (containsDirectGetLocalPlayerCall(function.getBody())) { - functionsDirectlyUsingLocalPlayer.add(function); - } - - function.getBody().accept(new ImStmt.DefaultVisitor() { - @Override - public void visit(ImSet set) { - super.visit(set); - if (isLocalPlayerDependent(set.getRight())) { - addAssignedVariables(set.getLeft()); - } - } - - @Override - public void visit(ImReturn returnStmt) { - super.visit(returnStmt); - if (returnStmt.getReturnValue() instanceof ImExpr - && isLocalPlayerDependent((ImExpr) returnStmt.getReturnValue())) { - localPlayerDependentReturns.add(function); - } - } - - @Override - public void visit(ImFunctionCall call) { - super.visit(call); - int count = Math.min(call.getArguments().size(), call.getFunc().getParameters().size()); - for (int i = 0; i < count; i++) { - if (isLocalPlayerDependent(call.getArguments().get(i))) { - localPlayerDependentVars.add(call.getFunc().getParameters().get(i)); - } - } - } - - @Override - public void visit(ImMethodCall call) { - super.visit(call); - if (isLocalPlayerDependent(call.getReceiver()) - || isLocalPlayerDependent(call.getArguments())) { - markMethodParametersLocalPlayerDependent(call.getMethod()); - } - } - }); - - if (containsLocalPlayerUse(function.getBody())) { - functionsUsingLocalPlayer.add(function); } - } - - private boolean containsLocalPlayerUse(Element element) { - if (element instanceof ImFunctionCall) { - ImFunction called = ((ImFunctionCall) element).getFunc(); - if (isGetLocalPlayer(called) || functionsUsingLocalPlayer.contains(called)) { - return true; - } - } else if (element instanceof ImMethodCall) { - if (methodUsesLocalPlayer(((ImMethodCall) element).getMethod())) { - return true; - } - } else if (element instanceof ImVarAccess - && localPlayerDependentVars.contains(((ImVarAccess) element).getVar())) { - return true; - } else if (element instanceof ImVarArrayAccess - && localPlayerDependentVars.contains(((ImVarArrayAccess) element).getVar())) { - return true; - } else if (element instanceof ImMemberAccess - && localPlayerDependentVars.contains(((ImMemberAccess) element).getVar())) { - return true; - } - for (int i = 0; i < element.size(); i++) { - if (containsLocalPlayerUse(element.get(i))) { - return true; - } - } - return false; + propagateFacts(); } private boolean methodReturnsLocalPlayerDependentValue(ImMethod method) { @@ -213,70 +142,202 @@ private boolean methodReturnsLocalPlayerDependentValue(ImMethod method) { return false; } - private boolean methodUsesLocalPlayer(ImMethod method) { - if (method == null || method.getImplementation() == null) { - return true; + private void indexElement(Element element, ImFunction owner) { + indexedElements.add(element); + for (int i = 0; i < element.size(); i++) { + Element child = element.get(i); + indexElement(child, owner); + addDependency(child, element); } - if (functionsUsingLocalPlayer.contains(method.getImplementation())) { - return true; + + if (element instanceof ImVarAccess) { + addDependency(variableFact(((ImVarAccess) element).getVar()), element); + } else if (element instanceof ImVarArrayAccess) { + addDependency(variableFact(((ImVarArrayAccess) element).getVar()), element); + } else if (element instanceof ImMemberAccess) { + addDependency(variableFact(((ImMemberAccess) element).getVar()), element); } - for (ImMethod subMethod : method.getSubMethods()) { - if (methodUsesLocalPlayer(subMethod)) { - return true; + + if (element instanceof ImSet) { + ImSet set = (ImSet) element; + forEachAssignedVariable(set.getLeft(), + variable -> addDependency(set.getRight(), variableFact(variable))); + } else if (element instanceof ImReturn) { + ImReturn returnStmt = (ImReturn) element; + if (returnStmt.getReturnValue() instanceof ImExpr) { + addDependency(returnStmt.getReturnValue(), returnFact(owner)); } + } else if (element instanceof ImFunctionCall) { + indexFunctionCall((ImFunctionCall) element, owner); + } else if (element instanceof ImMethodCall) { + indexMethodCall((ImMethodCall) element, owner); } - return false; } - private void markMethodParametersLocalPlayerDependent(ImMethod method) { - if (method == null || method.getImplementation() == null) { - return; + private void indexFunctionCall(ImFunctionCall call, ImFunction owner) { + ImFunction called = call.getFunc(); + addDependency(returnFact(called), call); + addDependency(useFact(called), useFact(owner)); + if (isGetLocalPlayer(called)) { + functionsDirectlyUsingLocalPlayer.add(owner); + addLocalPlayerSource(called); } - localPlayerDependentVars.addAll(method.getImplementation().getParameters()); - for (ImMethod subMethod : method.getSubMethods()) { - markMethodParametersLocalPlayerDependent(subMethod); + + int count = Math.min(call.getArguments().size(), called.getParameters().size()); + for (int i = 0; i < count; i++) { + addDependency(call.getArguments().get(i), + variableFact(called.getParameters().get(i))); } } - private boolean containsDirectGetLocalPlayerCall(Element element) { - if (element instanceof ImFunctionCall - && isGetLocalPlayer(((ImFunctionCall) element).getFunc())) { - return true; + private void indexMethodCall(ImMethodCall call, ImFunction owner) { + Set implementations = + Collections.newSetFromMap(new IdentityHashMap<>()); + boolean allImplementationsKnown = collectMethodImplementations( + call.getMethod(), + implementations, + Collections.newSetFromMap(new IdentityHashMap<>())); + if (!allImplementationsKnown) { + addDependency(unknownDispatchSource, call); + addDependency(unknownDispatchSource, useFact(owner)); } - for (int i = 0; i < element.size(); i++) { - if (containsDirectGetLocalPlayerCall(element.get(i))) { - return true; + + for (ImFunction implementation : implementations) { + addDependency(returnFact(implementation), call); + addDependency(useFact(implementation), useFact(owner)); + for (ImVar parameter : implementation.getParameters()) { + addDependency(call.getReceiver(), variableFact(parameter)); + for (ImExpr argument : call.getArguments()) { + addDependency(argument, variableFact(parameter)); + } } } - return false; } - private void addAssignedVariables(ImLExpr left) { + private boolean collectMethodImplementations(ImMethod method, + Set implementations, + Set visited) { + if (method == null || !visited.add(method) || method.getImplementation() == null) { + return method != null && method.getImplementation() != null; + } + implementations.add(method.getImplementation()); + for (ImMethod subMethod : method.getSubMethods()) { + if (!collectMethodImplementations(subMethod, implementations, visited)) { + return false; + } + } + return true; + } + + private void forEachAssignedVariable(ImLExpr left, Consumer consumer) { if (left instanceof ImVarAccess) { - localPlayerDependentVars.add(((ImVarAccess) left).getVar()); + consumer.accept(((ImVarAccess) left).getVar()); } else if (left instanceof ImVarArrayAccess) { - localPlayerDependentVars.add(((ImVarArrayAccess) left).getVar()); + consumer.accept(((ImVarArrayAccess) left).getVar()); } else if (left instanceof ImMemberAccess) { - localPlayerDependentVars.add(((ImMemberAccess) left).getVar()); + consumer.accept(((ImMemberAccess) left).getVar()); } else if (left instanceof ImTupleSelection) { ImExpr tupleExpr = ((ImTupleSelection) left).getTupleExpr(); if (tupleExpr instanceof ImLExpr) { - addAssignedVariables((ImLExpr) tupleExpr); + forEachAssignedVariable((ImLExpr) tupleExpr, consumer); } } else if (left instanceof ImTupleExpr) { for (ImExpr expr : ((ImTupleExpr) left).getExprs()) { if (expr instanceof ImLExpr) { - addAssignedVariables((ImLExpr) expr); + forEachAssignedVariable((ImLExpr) expr, consumer); } } } else if (left instanceof ImStatementExpr) { ImExpr expr = ((ImStatementExpr) left).getExpr(); if (expr instanceof ImLExpr) { - addAssignedVariables((ImLExpr) expr); + forEachAssignedVariable((ImLExpr) expr, consumer); } } } + private void addLocalPlayerSource(ImFunction function) { + sourceFacts.add(returnFact(function)); + sourceFacts.add(useFact(function)); + } + + private void addDependency(Object dependency, Object dependent) { + dependents.computeIfAbsent(dependency, + ignored -> new ArrayList<>()) + .add(dependent); + } + + private void propagateFacts() { + Deque worklist = new ArrayDeque<>(); + for (Object source : sourceFacts) { + activateFact(source, worklist); + } + while (!worklist.isEmpty()) { + Object fact = worklist.removeFirst(); + for (Object dependent : dependents.getOrDefault(fact, Collections.emptyList())) { + activateFact(dependent, worklist); + } + } + } + + private void activateFact(Object fact, Deque worklist) { + if (activeFacts.add(fact)) { + publishFact(fact); + worklist.addLast(fact); + } + } + + private void publishFact(Object fact) { + if (!(fact instanceof Fact)) { + return; + } + Fact typedFact = (Fact) fact; + switch (typedFact.kind) { + case VARIABLE: + localPlayerDependentVars.add((ImVar) typedFact.subject); + break; + case RETURN: + localPlayerDependentReturns.add((ImFunction) typedFact.subject); + break; + case USE: + functionsUsingLocalPlayer.add((ImFunction) typedFact.subject); + break; + case SOURCE: + break; + } + } + + private Fact variableFact(ImVar variable) { + return variableFacts.computeIfAbsent(variable, + ignored -> new Fact(FactKind.VARIABLE, variable)); + } + + private Fact returnFact(ImFunction function) { + return returnFacts.computeIfAbsent(function, + ignored -> new Fact(FactKind.RETURN, function)); + } + + private Fact useFact(ImFunction function) { + return useFacts.computeIfAbsent(function, + ignored -> new Fact(FactKind.USE, function)); + } + + private enum FactKind { + VARIABLE, + RETURN, + USE, + SOURCE + } + + private static final class Fact { + private final FactKind kind; + private final Object subject; + + private Fact(FactKind kind, Object subject) { + this.kind = kind; + this.subject = subject; + } + } + private static boolean isGetLocalPlayer(ImFunction function) { return function != null && function.isNative() diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java index b4f2732c9..5bc892cd9 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java @@ -7,6 +7,8 @@ import de.peeeq.wurstscript.intermediatelang.optimizer.BranchMerger; import de.peeeq.wurstscript.intermediatelang.optimizer.ConstantAndCopyPropagation; import de.peeeq.wurstscript.intermediatelang.optimizer.DispatchCheckDeduplicator; +import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerAwareOptimizerPass; +import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerContextAnalyzer; import de.peeeq.wurstscript.intermediatelang.optimizer.LocalMerger; import de.peeeq.wurstscript.intermediatelang.optimizer.SideEffectAnalyzer; import de.peeeq.wurstscript.intermediatelang.optimizer.SimpleRewrites; @@ -73,8 +75,25 @@ public void localOptimizations() { int finalItr = 0; for (int i = 1; i <= 10 && optCount > 0; i++) { optCount = 0; + LocalPlayerContextAnalyzer localPlayerContextAnalyzer = null; for (OptimizerPass pass : localPasses) { - int count = timeTaker.measure(pass.getName(), () -> pass.optimize(trans)); + int count; + if (pass instanceof LocalPlayerAwareOptimizerPass) { + if (localPlayerContextAnalyzer == null) { + localPlayerContextAnalyzer = + new LocalPlayerContextAnalyzer(trans.getImProg()); + } + LocalPlayerContextAnalyzer analyzer = localPlayerContextAnalyzer; + LocalPlayerAwareOptimizerPass localPlayerAwarePass = + (LocalPlayerAwareOptimizerPass) pass; + count = timeTaker.measure( + pass.getName(), + () -> localPlayerAwarePass.optimize(trans, analyzer)); + } else { + count = timeTaker.measure(pass.getName(), () -> pass.optimize(trans)); + // A general mutating pass may invalidate dependency edges. + localPlayerContextAnalyzer = null; + } optCount += count; totalCount.put(pass.getName(), totalCount.getOrDefault(pass.getName(), 0) + count); } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/UselessFunctionCallsRemover.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/UselessFunctionCallsRemover.java index f473dba55..131424467 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/UselessFunctionCallsRemover.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/UselessFunctionCallsRemover.java @@ -1,6 +1,7 @@ package de.peeeq.wurstscript.translation.imoptimizer; import de.peeeq.wurstscript.WurstOperator; +import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerAwareOptimizerPass; import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerContextAnalyzer; import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.translation.imtranslation.ImHelper; @@ -17,14 +18,14 @@ *

* when the result is not used */ -public class UselessFunctionCallsRemover implements OptimizerPass { +public class UselessFunctionCallsRemover implements LocalPlayerAwareOptimizerPass { public int totalCallsRemoved = 0; private LocalPlayerContextAnalyzer localPlayerContextAnalyzer; - public int optimize(ImTranslator trans) { + public int optimize(ImTranslator trans, LocalPlayerContextAnalyzer analyzer) { totalCallsRemoved = 0; ImProg prog = trans.getImProg(); - localPlayerContextAnalyzer = new LocalPlayerContextAnalyzer(prog); + localPlayerContextAnalyzer = analyzer; for (ImFunction func : prog.getFunctions()) { optimizeFunc(func, trans); } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index e675e28fd..a92d3088d 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -9,6 +9,7 @@ import de.peeeq.wurstscript.ast.WurstModel; import de.peeeq.wurstscript.intermediatelang.optimizer.FunctionSplitter; import de.peeeq.wurstscript.intermediatelang.optimizer.LocalMerger; +import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerContextAnalyzer; import de.peeeq.wurstscript.intermediatelang.optimizer.SideEffectAnalyzer; import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; @@ -1915,6 +1916,87 @@ public void functionUsingGetLocalPlayerMustNotBeInlined() throws Exception { "transitive GetLocalPlayer wrappers must remain explicit calls"); } + @Test(timeOut = 10_000) + public void deeplyNestedIndependentCallsDoNotCauseExponentialLocalPlayerAnalysis() { + String nestedCall = "Player(0)"; + for (int i = 0; i < 30; i++) { + nestedCall = "passthrough(" + nestedCall + ")"; + } + + test().lines( + "type player extends handle", + "package test", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "@noinline function passthrough(player p) returns player", + " return p", + "init", + " if " + nestedCall + " == Player(0)", + " print(1)" + ); + } + + @Test(timeOut = 10_000) + public void reverseOrderedCallChainUsesLocalPlayerWorklist() { + Element trace = Ast.NoExpr(); + ImFunctions functions = JassIm.ImFunctions(); + for (int i = 0; i < 4_000; i++) { + functions.add(JassIm.ImFunction( + trace, + "chain" + i, + JassIm.ImTypeVars(), + JassIm.ImVars(), + TypesHelper.imInt(), + JassIm.ImVars(), + JassIm.ImStmts(), + Collections.emptyList() + )); + } + ImFunction getLocalPlayer = JassIm.ImFunction( + trace, + "GetLocalPlayer", + JassIm.ImTypeVars(), + JassIm.ImVars(), + TypesHelper.imInt(), + JassIm.ImVars(), + JassIm.ImStmts(), + Collections.singletonList(FunctionFlagEnum.IS_NATIVE) + ); + functions.add(getLocalPlayer); + + for (int i = 0; i < functions.size() - 1; i++) { + ImFunction caller = functions.get(i); + ImFunction callee = functions.get(i + 1); + caller.getBody().add(JassIm.ImReturn( + trace, + JassIm.ImFunctionCall( + trace, + callee, + JassIm.ImTypeArguments(), + JassIm.ImExprs(), + false, + de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL + ) + )); + } + + ImProg prog = JassIm.ImProg( + trace, + JassIm.ImVars(), + functions, + JassIm.ImMethods(), + JassIm.ImClasses(), + JassIm.ImTypeClassFuncs(), + new java.util.HashMap<>() + ); + LocalPlayerContextAnalyzer analyzer = new LocalPlayerContextAnalyzer(prog); + + assertTrue(analyzer.functionInliningIsLocalPlayerSensitive(functions.get(0)), + "GetLocalPlayer dependency must propagate through the complete call chain"); + assertTrue(analyzer.functionUsesLocalPlayer(functions.get(0)), + "GetLocalPlayer usage must propagate through the complete call chain"); + } + private static int countOccurrences(String text, String needle) { int count = 0; int from = 0; From a900ba6d10dbd922c03696d1fc32b1935ef4e400 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sun, 26 Jul 2026 19:34:42 +0200 Subject: [PATCH 3/6] review #2 --- .../optimizer/LocalPlayerContextAnalyzer.java | 76 ++++++++++++++-- .../wurstscript/tests/OptimizerTests.java | 90 +++++++++++++++++++ 2 files changed, 157 insertions(+), 9 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java index 3d8edd37a..2729f7a68 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java @@ -39,6 +39,7 @@ public final class LocalPlayerContextAnalyzer { private final Map variableFacts = new IdentityHashMap<>(); private final Map returnFacts = new IdentityHashMap<>(); private final Map useFacts = new IdentityHashMap<>(); + private final Map entryControlFacts = new IdentityHashMap<>(); private final Set sourceFacts = Collections.newSetFromMap(new IdentityHashMap<>()); private final Fact unknownDispatchSource = new Fact(FactKind.SOURCE, null); @@ -120,7 +121,7 @@ private void analyze(ImProg prog) { if (isGetLocalPlayer(function)) { addLocalPlayerSource(function); } else if (!function.isNative()) { - indexElement(function.getBody(), function); + indexElement(function.getBody(), function, entryControlFact(function)); addDependency(function.getBody(), useFact(function)); } } @@ -142,11 +143,37 @@ private boolean methodReturnsLocalPlayerDependentValue(ImMethod method) { return false; } - private void indexElement(Element element, ImFunction owner) { + private void indexElement(Element element, ImFunction owner, Object controlContext) { indexedElements.add(element); + + Object branchControl = null; + if (element instanceof ImIf) { + ImIf ifStmt = (ImIf) element; + branchControl = new Fact(FactKind.CONTROL, ifStmt); + addDependency(ifStmt.getCondition(), branchControl); + addEnclosingControlDependency(controlContext, branchControl); + } + + Object loopControl = null; + if (element instanceof ImLoop) { + ImLoop loop = (ImLoop) element; + loopControl = new Fact(FactKind.CONTROL, loop); + addEnclosingControlDependency(controlContext, loopControl); + addLoopExitDependencies(loop.getBody(), loopControl); + } + for (int i = 0; i < element.size(); i++) { Element child = element.get(i); - indexElement(child, owner); + Object childControl = controlContext; + if (element instanceof ImIf + && (child == ((ImIf) element).getThenBlock() + || child == ((ImIf) element).getElseBlock())) { + childControl = branchControl; + } else if (element instanceof ImLoop + && child == ((ImLoop) element).getBody()) { + childControl = loopControl; + } + indexElement(child, owner, childControl); addDependency(child, element); } @@ -160,24 +187,30 @@ private void indexElement(Element element, ImFunction owner) { if (element instanceof ImSet) { ImSet set = (ImSet) element; - forEachAssignedVariable(set.getLeft(), - variable -> addDependency(set.getRight(), variableFact(variable))); + forEachAssignedVariable(set.getLeft(), variable -> { + addDependency(set.getRight(), variableFact(variable)); + addEnclosingControlDependency(controlContext, variableFact(variable)); + }); } else if (element instanceof ImReturn) { ImReturn returnStmt = (ImReturn) element; if (returnStmt.getReturnValue() instanceof ImExpr) { addDependency(returnStmt.getReturnValue(), returnFact(owner)); + addEnclosingControlDependency(controlContext, returnFact(owner)); } } else if (element instanceof ImFunctionCall) { - indexFunctionCall((ImFunctionCall) element, owner); + indexFunctionCall((ImFunctionCall) element, owner, controlContext); } else if (element instanceof ImMethodCall) { - indexMethodCall((ImMethodCall) element, owner); + indexMethodCall((ImMethodCall) element, owner, controlContext); } } - private void indexFunctionCall(ImFunctionCall call, ImFunction owner) { + private void indexFunctionCall(ImFunctionCall call, ImFunction owner, Object controlContext) { ImFunction called = call.getFunc(); addDependency(returnFact(called), call); addDependency(useFact(called), useFact(owner)); + if (!called.isNative()) { + addEnclosingControlDependency(controlContext, entryControlFact(called)); + } if (isGetLocalPlayer(called)) { functionsDirectlyUsingLocalPlayer.add(owner); addLocalPlayerSource(called); @@ -190,7 +223,7 @@ private void indexFunctionCall(ImFunctionCall call, ImFunction owner) { } } - private void indexMethodCall(ImMethodCall call, ImFunction owner) { + private void indexMethodCall(ImMethodCall call, ImFunction owner, Object controlContext) { Set implementations = Collections.newSetFromMap(new IdentityHashMap<>()); boolean allImplementationsKnown = collectMethodImplementations( @@ -205,6 +238,7 @@ private void indexMethodCall(ImMethodCall call, ImFunction owner) { for (ImFunction implementation : implementations) { addDependency(returnFact(implementation), call); addDependency(useFact(implementation), useFact(owner)); + addEnclosingControlDependency(controlContext, entryControlFact(implementation)); for (ImVar parameter : implementation.getParameters()) { addDependency(call.getReceiver(), variableFact(parameter)); for (ImExpr argument : call.getArguments()) { @@ -214,6 +248,23 @@ private void indexMethodCall(ImMethodCall call, ImFunction owner) { } } + private void addEnclosingControlDependency(Object controlContext, Object dependent) { + if (controlContext != null) { + addDependency(controlContext, dependent); + } + } + + private void addLoopExitDependencies(Element element, Object loopControl) { + if (element instanceof ImExitwhen) { + addDependency(((ImExitwhen) element).getCondition(), loopControl); + } else if (element instanceof ImLoop || element instanceof ImVarargLoop) { + return; + } + for (int i = 0; i < element.size(); i++) { + addLoopExitDependencies(element.get(i), loopControl); + } + } + private boolean collectMethodImplementations(ImMethod method, Set implementations, Set visited) { @@ -301,6 +352,7 @@ private void publishFact(Object fact) { case USE: functionsUsingLocalPlayer.add((ImFunction) typedFact.subject); break; + case CONTROL: case SOURCE: break; } @@ -321,10 +373,16 @@ private Fact useFact(ImFunction function) { ignored -> new Fact(FactKind.USE, function)); } + private Fact entryControlFact(ImFunction function) { + return entryControlFacts.computeIfAbsent(function, + ignored -> new Fact(FactKind.CONTROL, function)); + } + private enum FactKind { VARIABLE, RETURN, USE, + CONTROL, SOURCE } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index a92d3088d..14ea42dc8 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -1891,6 +1891,96 @@ public void branchMergerMustTrackLocalPlayerThroughFunctionParameters() throws E "GetLocalPlayer taint must flow through call arguments and parameters"); } + @Test + public void branchMergerMustTrackLocalPlayerControlDependentAssignments() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "player selected", + "integer result = 0", + "init", + " if GetLocalPlayer() == Player(0)", + " selected = Player(0)", + " else", + " selected = Player(1)", + " if selected == Player(0)", + " result = 11", + " else", + " result = 11", + " print(result)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_branchMergerMustTrackLocalPlayerControlDependentAssignments_opt.j"), + Charsets.UTF_8); + assertTrue(countOccurrences(optimized, "test_result = 11") >= 2, + "values assigned under local-player control must remain local-player-dependent"); + } + + @Test + public void localPlayerControlMustPropagateThroughCalledFunctions() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "player selected", + "integer result = 0", + "@noinline function select(player p)", + " selected = p", + "init", + " if GetLocalPlayer() == Player(0)", + " select(Player(0))", + " else", + " select(Player(1))", + " if selected == Player(0)", + " result = 13", + " else", + " result = 13", + " print(result)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_localPlayerControlMustPropagateThroughCalledFunctions_opt.j"), + Charsets.UTF_8); + assertTrue(countOccurrences(optimized, "test_result = 13") >= 2, + "callee assignments must inherit local-player control from their call sites"); + } + + @Test + public void localPlayerControlMustPropagateIntoFunctionReturns() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "integer result = 0", + "@noinline function selectedPlayer() returns player", + " if GetLocalPlayer() == Player(0)", + " return Player(0)", + " else", + " return Player(1)", + "init", + " player selected = selectedPlayer()", + " if selected == Player(0)", + " result = 17", + " else", + " result = 17", + " print(result)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_localPlayerControlMustPropagateIntoFunctionReturns_opt.j"), + Charsets.UTF_8); + assertTrue(countOccurrences(optimized, "test_result = 17") >= 2, + "returns selected under local-player control must remain local-player-dependent"); + } + @Test public void functionUsingGetLocalPlayerMustNotBeInlined() throws Exception { test().lines( From fa626f8e3ec51f7df20469e8b03ab6a19e56a55e Mon Sep 17 00:00:00 2001 From: Frotty Date: Sun, 26 Jul 2026 19:51:36 +0200 Subject: [PATCH 4/6] #3 --- .../optimizer/LocalPlayerContextAnalyzer.java | 32 ++++++++++ .../wurstscript/tests/OptimizerTests.java | 60 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java index 2729f7a68..46136043b 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java @@ -164,6 +164,16 @@ private void indexElement(Element element, ImFunction owner, Object controlConte for (int i = 0; i < element.size(); i++) { Element child = element.get(i); + if (element instanceof ImOperatorCall + && ((ImOperatorCall) element).getOp().isLazy() + && child == ((ImOperatorCall) element).getArguments()) { + indexShortCircuitArguments( + ((ImOperatorCall) element).getArguments(), + owner, + controlContext); + addDependency(child, element); + continue; + } Object childControl = controlContext; if (element instanceof ImIf && (child == ((ImIf) element).getThenBlock() @@ -204,6 +214,28 @@ private void indexElement(Element element, ImFunction owner, Object controlConte } } + private void indexShortCircuitArguments(ImExprs arguments, + ImFunction owner, + Object controlContext) { + indexedElements.add(arguments); + Object operandControl = controlContext; + for (int i = 0; i < arguments.size(); i++) { + ImExpr argument = arguments.get(i); + indexElement(argument, owner, operandControl); + addDependency(argument, arguments); + + if (i + 1 < arguments.size()) { + Fact followingOperandControl = + new Fact(FactKind.CONTROL, argument); + addEnclosingControlDependency( + operandControl, + followingOperandControl); + addDependency(argument, followingOperandControl); + operandControl = followingOperandControl; + } + } + } + private void indexFunctionCall(ImFunctionCall call, ImFunction owner, Object controlContext) { ImFunction called = call.getFunc(); addDependency(returnFact(called), call); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 14ea42dc8..13371b0f6 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -1981,6 +1981,66 @@ public void localPlayerControlMustPropagateIntoFunctionReturns() throws Exceptio "returns selected under local-player control must remain local-player-dependent"); } + @Test + public void andRightOperandMustInheritLocalPlayerControl() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "player selected", + "integer result = 0", + "@noinline function updateSelectedState() returns boolean", + " selected = Player(0)", + " return true", + "init", + " if (GetLocalPlayer() == Player(0)) and updateSelectedState()", + " print(0)", + " if selected == Player(0)", + " result = 19", + " else", + " result = 19", + " print(result)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_andRightOperandMustInheritLocalPlayerControl_opt.j"), + Charsets.UTF_8); + assertTrue(countOccurrences(optimized, "test_result = 19") >= 2, + "the right operand of local-player-dependent AND must be locally controlled"); + } + + @Test + public void orRightOperandMustInheritLocalPlayerControl() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "player selected", + "integer result = 0", + "@noinline function updateSelectedState() returns boolean", + " selected = Player(0)", + " return false", + "init", + " if (GetLocalPlayer() == Player(0)) or updateSelectedState()", + " print(0)", + " if selected == Player(0)", + " result = 23", + " else", + " result = 23", + " print(result)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_orRightOperandMustInheritLocalPlayerControl_opt.j"), + Charsets.UTF_8); + assertTrue(countOccurrences(optimized, "test_result = 23") >= 2, + "the right operand of local-player-dependent OR must be locally controlled"); + } + @Test public void functionUsingGetLocalPlayerMustNotBeInlined() throws Exception { test().lines( From 61e52fa8040fca76d08fc805790133878eb5bf28 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sun, 26 Jul 2026 20:18:39 +0200 Subject: [PATCH 5/6] #4 --- .../optimizer/LocalPlayerContextAnalyzer.java | 89 +++++++++++++++++-- .../tests/LuaBackendAuditTests.java | 27 ++++++ .../wurstscript/tests/OptimizerTests.java | 30 +++++++ 3 files changed, 141 insertions(+), 5 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java index 46136043b..2e1912f0e 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java @@ -13,6 +13,8 @@ import java.util.Set; import java.util.function.Consumer; +import static de.peeeq.wurstscript.translation.imtranslation.FunctionFlagEnum.IS_VARARG; + /** * Conservative, flow-insensitive analysis for values and functions which may * depend on {@code GetLocalPlayer()}. @@ -40,6 +42,7 @@ public final class LocalPlayerContextAnalyzer { private final Map returnFacts = new IdentityHashMap<>(); private final Map useFacts = new IdentityHashMap<>(); private final Map entryControlFacts = new IdentityHashMap<>(); + private final Map containsReturnCache = new IdentityHashMap<>(); private final Set sourceFacts = Collections.newSetFromMap(new IdentityHashMap<>()); private final Fact unknownDispatchSource = new Fact(FactKind.SOURCE, null); @@ -162,6 +165,11 @@ private void indexElement(Element element, ImFunction owner, Object controlConte addLoopExitDependencies(loop.getBody(), loopControl); } + if (element instanceof ImStmts) { + indexStatementSequence((ImStmts) element, owner, controlContext); + return; + } + for (int i = 0; i < element.size(); i++) { Element child = element.get(i); if (element instanceof ImOperatorCall @@ -193,6 +201,13 @@ private void indexElement(Element element, ImFunction owner, Object controlConte addDependency(variableFact(((ImVarArrayAccess) element).getVar()), element); } else if (element instanceof ImMemberAccess) { addDependency(variableFact(((ImMemberAccess) element).getVar()), element); + } else if (element instanceof ImVarargLoop) { + ImVar varargParameter = varargParameter(owner); + if (varargParameter != null) { + addDependency( + variableFact(varargParameter), + variableFact(((ImVarargLoop) element).getLoopVar())); + } } if (element instanceof ImSet) { @@ -214,6 +229,45 @@ private void indexElement(Element element, ImFunction owner, Object controlConte } } + private void indexStatementSequence(ImStmts statements, + ImFunction owner, + Object controlContext) { + Object continuationControl = controlContext; + for (ImStmt statement : statements) { + indexElement(statement, owner, continuationControl); + addDependency(statement, statements); + + if (containsFunctionReturn(statement)) { + Fact followingStatementControl = + new Fact(FactKind.CONTROL, statement); + addEnclosingControlDependency( + continuationControl, + followingStatementControl); + addDependency(statement, followingStatementControl); + continuationControl = followingStatementControl; + } + } + } + + private boolean containsFunctionReturn(Element element) { + Boolean cached = containsReturnCache.get(element); + if (cached != null) { + return cached; + } + if (element instanceof ImReturn) { + containsReturnCache.put(element, true); + return true; + } + for (int i = 0; i < element.size(); i++) { + if (containsFunctionReturn(element.get(i))) { + containsReturnCache.put(element, true); + return true; + } + } + containsReturnCache.put(element, false); + return false; + } + private void indexShortCircuitArguments(ImExprs arguments, ImFunction owner, Object controlContext) { @@ -248,11 +302,29 @@ private void indexFunctionCall(ImFunctionCall call, ImFunction owner, Object con addLocalPlayerSource(called); } - int count = Math.min(call.getArguments().size(), called.getParameters().size()); - for (int i = 0; i < count; i++) { + int fixedParameterCount = called.getParameters().size(); + if (called.hasFlag(IS_VARARG) && fixedParameterCount > 0) { + fixedParameterCount--; + } + int positionalCount = Math.min(call.getArguments().size(), fixedParameterCount); + for (int i = 0; i < positionalCount; i++) { addDependency(call.getArguments().get(i), variableFact(called.getParameters().get(i))); } + ImVar varargParameter = varargParameter(called); + if (varargParameter != null) { + for (int i = fixedParameterCount; i < call.getArguments().size(); i++) { + addDependency(call.getArguments().get(i), + variableFact(varargParameter)); + } + } + } + + private ImVar varargParameter(ImFunction function) { + if (function.hasFlag(IS_VARARG) && !function.getParameters().isEmpty()) { + return function.getParameters().get(function.getParameters().size() - 1); + } + return null; } private void indexMethodCall(ImMethodCall call, ImFunction owner, Object controlContext) { @@ -286,15 +358,22 @@ private void addEnclosingControlDependency(Object controlContext, Object depende } } - private void addLoopExitDependencies(Element element, Object loopControl) { + private boolean addLoopExitDependencies(Element element, Object loopControl) { if (element instanceof ImExitwhen) { addDependency(((ImExitwhen) element).getCondition(), loopControl); + return true; } else if (element instanceof ImLoop || element instanceof ImVarargLoop) { - return; + return false; } + + boolean containsExit = false; for (int i = 0; i < element.size(); i++) { - addLoopExitDependencies(element.get(i), loopControl); + containsExit |= addLoopExitDependencies(element.get(i), loopControl); + } + if (containsExit && element instanceof ImIf) { + addDependency(((ImIf) element).getCondition(), loopControl); } + return containsExit; } private boolean collectMethodImplementations(ImMethod method, diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index c07f83e76..1f269fa18 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -78,6 +78,33 @@ public void localPlayerEffectfulBooleanOperandSurvivesOptimization() { compiled.indexOf("localProbe()", definitionOrCall + 1) >= 0); } + @Test + public void localPlayerTaintFlowsThroughVarargLoopValues() { + String compiled = compileOptimizedLua( + "localPlayerTaintFlowsThroughVarargLoopValues", + "type player extends handle", + "package Test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "player selected", + "integer result = 0", + "@noinline function selectLast(vararg player players)", + " for p in players", + " selected = p", + "init", + " selectLast(Player(1), GetLocalPlayer())", + " if selected == Player(0)", + " result = 31", + " else", + " result = 31", + " print(result)" + ); + + assertTrue("vararg loop values must retain local-player dependence", + countOccurrences(compiled, "result = 31") >= 2); + } + /** * A bare {@code return} inside a vararg loop used to truncate the literal * {@code end} that closed the loop, producing unparseable Lua diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 13371b0f6..df14c4880 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -1981,6 +1981,36 @@ public void localPlayerControlMustPropagateIntoFunctionReturns() throws Exceptio "returns selected under local-player control must remain local-player-dependent"); } + @Test + public void statementsAfterLocalEarlyReturnMustRemainLocallyControlled() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "player selected", + "integer result = 0", + "@noinline function updateUnlessLocalPlayerZero()", + " if GetLocalPlayer() == Player(0)", + " return", + " selected = Player(1)", + "init", + " updateUnlessLocalPlayerZero()", + " if selected == Player(1)", + " result = 29", + " else", + " result = 29", + " print(result)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_statementsAfterLocalEarlyReturnMustRemainLocallyControlled_opt.j"), + Charsets.UTF_8); + assertTrue(countOccurrences(optimized, "test_result = 29") >= 2, + "statements reached after a local early return must remain locally controlled"); + } + @Test public void andRightOperandMustInheritLocalPlayerControl() throws Exception { test().lines( From 3f6143aeda79fc932dcd33e9a1c038ae177b725c Mon Sep 17 00:00:00 2001 From: Frotty Date: Sun, 26 Jul 2026 20:34:06 +0200 Subject: [PATCH 6/6] #5 --- .../optimizer/LocalPlayerContextAnalyzer.java | 1 + .../wurstscript/tests/OptimizerTests.java | 62 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java index 2e1912f0e..ddf2d9947 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java @@ -213,6 +213,7 @@ private void indexElement(Element element, ImFunction owner, Object controlConte if (element instanceof ImSet) { ImSet set = (ImSet) element; forEachAssignedVariable(set.getLeft(), variable -> { + addDependency(set.getLeft(), variableFact(variable)); addDependency(set.getRight(), variableFact(variable)); addEnclosingControlDependency(controlContext, variableFact(variable)); }); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index df14c4880..33ae93fa1 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -1920,6 +1920,68 @@ public void branchMergerMustTrackLocalPlayerControlDependentAssignments() throws "values assigned under local-player control must remain local-player-dependent"); } + @Test + public void branchMergerMustTrackLocalPlayerDependentArrayIndexWrites() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native GetPlayerId(player p) returns integer", + "native print(integer i)", + "integer array values", + "integer result = 0", + "init", + " values[GetPlayerId(GetLocalPlayer())] = 1", + " if values[0] == 1", + " result = 31", + " else", + " result = 31", + " print(result)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_branchMergerMustTrackLocalPlayerDependentArrayIndexWrites_opt.j"), + Charsets.UTF_8); + assertTrue(countOccurrences(optimized, "test_result = 31") >= 2, + "an array written through a local-player-dependent index must remain local-player-dependent"); + } + + @Test + public void branchMergerMustTrackLocalPlayerDependentMemberReceiverWrites() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native print(integer i)", + "class Box", + " integer value", + "Box first", + "Box second", + "integer result = 0", + "init", + " first = new Box", + " second = new Box", + " Box selected", + " if GetLocalPlayer() == Player(0)", + " selected = first", + " else", + " selected = second", + " selected.value = 1", + " if first.value == 1", + " result = 37", + " else", + " result = 37", + " print(result)" + ); + + String optimized = Files.toString( + new File("test-output/OptimizerTests_branchMergerMustTrackLocalPlayerDependentMemberReceiverWrites_opt.j"), + Charsets.UTF_8); + assertTrue(countOccurrences(optimized, "test_result = 37") >= 2, + "a member written through a local-player-dependent receiver must remain local-player-dependent"); + } + @Test public void localPlayerControlMustPropagateThroughCalledFunctions() throws Exception { test().lines(