From 9cf21825fcf13c5f2817d07aa2f18c9cad54cbf7 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Tue, 16 Jun 2020 19:24:45 +0300 Subject: [PATCH] Fix wrong evaluation when optimizer sees array initialization that looks like array initializer expression, but has access to array variable on its elements, i.e. on a case like this: Object[] array = new Object[1]; array[0] = array; That can't be optimized to Object[] array = new Object[] { array }; --- .../java/org/teavm/ast/RecursiveVisitor.java | 118 ++++++++++++++---- .../ast/optimization/OptimizingVisitor.java | 7 ++ .../optimization/VariableAccessFinder.java | 45 +++++++ 3 files changed, 145 insertions(+), 25 deletions(-) create mode 100644 core/src/main/java/org/teavm/ast/optimization/VariableAccessFinder.java diff --git a/core/src/main/java/org/teavm/ast/RecursiveVisitor.java b/core/src/main/java/org/teavm/ast/RecursiveVisitor.java index f1779cab8..0554f6c8d 100644 --- a/core/src/main/java/org/teavm/ast/RecursiveVisitor.java +++ b/core/src/main/java/org/teavm/ast/RecursiveVisitor.java @@ -24,18 +24,30 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { protected void afterVisit(Expr expr) { } + protected boolean canceled; + + protected final void cancel() { + canceled = true; + } + @Override public void visit(BinaryExpr expr) { beforeVisit(expr); - expr.getFirstOperand().acceptVisitor(this); - expr.getSecondOperand().acceptVisitor(this); + if (!canceled) { + expr.getFirstOperand().acceptVisitor(this); + if (!canceled) { + expr.getSecondOperand().acceptVisitor(this); + } + } afterVisit(expr); } @Override public void visit(UnaryExpr expr) { beforeVisit(expr); - expr.getOperand().acceptVisitor(this); + if (!canceled) { + expr.getOperand().acceptVisitor(this); + } afterVisit(expr); } @@ -43,6 +55,9 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { public void visit(AssignmentStatement statement) { if (statement.getLeftValue() != null) { statement.getLeftValue().acceptVisitor(this); + if (canceled) { + return; + } } statement.getRightValue().acceptVisitor(this); } @@ -50,15 +65,24 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { @Override public void visit(ConditionalExpr expr) { beforeVisit(expr); - expr.getCondition().acceptVisitor(this); - expr.getConsequent().acceptVisitor(this); - expr.getAlternative().acceptVisitor(this); + if (!canceled) { + expr.getCondition().acceptVisitor(this); + if (!canceled) { + expr.getConsequent().acceptVisitor(this); + if (!canceled) { + expr.getAlternative().acceptVisitor(this); + } + } + } afterVisit(expr); } public void visit(List statements) { for (Statement part : statements) { part.acceptVisitor(this); + if (canceled) { + break; + } } } @@ -77,7 +101,9 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { public void visit(ConditionalStatement statement) { statement.getCondition().acceptVisitor(this); visit(statement.getConsequent()); - visit(statement.getAlternative()); + if (!canceled) { + visit(statement.getAlternative()); + } } @Override @@ -90,23 +116,35 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { public void visit(SubscriptExpr expr) { beforeVisit(expr); expr.getArray().acceptVisitor(this); - expr.getIndex().acceptVisitor(this); + if (!canceled) { + expr.getIndex().acceptVisitor(this); + } afterVisit(expr); } @Override public void visit(SwitchStatement statement) { statement.getValue().acceptVisitor(this); + if (canceled) { + return; + } for (SwitchClause clause : statement.getClauses()) { visit(clause.getBody()); + if (canceled) { + return; + } + } + if (!canceled) { + visit(statement.getDefaultClause()); } - visit(statement.getDefaultClause()); } @Override public void visit(UnwrapArrayExpr expr) { beforeVisit(expr); - expr.getArray().acceptVisitor(this); + if (!canceled) { + expr.getArray().acceptVisitor(this); + } afterVisit(expr); } @@ -114,6 +152,9 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { public void visit(WhileStatement statement) { if (statement.getCondition() != null) { statement.getCondition().acceptVisitor(this); + if (canceled) { + return; + } } visit(statement.getBody()); } @@ -121,8 +162,13 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { @Override public void visit(InvocationExpr expr) { beforeVisit(expr); - for (Expr argument : expr.getArguments()) { - argument.acceptVisitor(this); + if (!canceled) { + for (Expr argument : expr.getArguments()) { + argument.acceptVisitor(this); + if (canceled) { + break; + } + } } afterVisit(expr); } @@ -135,8 +181,10 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { @Override public void visit(QualificationExpr expr) { beforeVisit(expr); - if (expr.getQualified() != null) { - expr.getQualified().acceptVisitor(this); + if (!canceled) { + if (expr.getQualified() != null) { + expr.getQualified().acceptVisitor(this); + } } afterVisit(expr); } @@ -158,15 +206,22 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { @Override public void visit(NewArrayExpr expr) { beforeVisit(expr); - expr.getLength().acceptVisitor(this); + if (!canceled) { + expr.getLength().acceptVisitor(this); + } afterVisit(expr); } @Override public void visit(NewMultiArrayExpr expr) { beforeVisit(expr); - for (Expr dimension : expr.getDimensions()) { - dimension.acceptVisitor(this); + if (!canceled) { + for (Expr dimension : expr.getDimensions()) { + dimension.acceptVisitor(this); + if (canceled) { + break; + } + } } afterVisit(expr); } @@ -174,8 +229,13 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { @Override public void visit(ArrayFromDataExpr expr) { beforeVisit(expr); - for (Expr element : expr.getData()) { - element.acceptVisitor(this); + if (!canceled) { + for (Expr element : expr.getData()) { + element.acceptVisitor(this); + if (canceled) { + break; + } + } } afterVisit(expr); } @@ -190,7 +250,9 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { @Override public void visit(InstanceOfExpr expr) { beforeVisit(expr); - expr.getExpr().acceptVisitor(this); + if (!canceled) { + expr.getExpr().acceptVisitor(this); + } afterVisit(expr); } @@ -202,7 +264,9 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { @Override public void visit(CastExpr expr) { beforeVisit(expr); - expr.getValue().acceptVisitor(this); + if (!canceled) { + expr.getValue().acceptVisitor(this); + } afterVisit(expr); } @@ -213,7 +277,9 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { @Override public void visit(PrimitiveCastExpr expr) { beforeVisit(expr); - expr.getValue().acceptVisitor(this); + if (!canceled) { + expr.getValue().acceptVisitor(this); + } afterVisit(expr); } @@ -240,9 +306,11 @@ public class RecursiveVisitor implements ExprVisitor, StatementVisitor { @Override public void visit(BoundCheckExpr expr) { beforeVisit(expr); - expr.getIndex().acceptVisitor(this); - if (expr.getArray() != null) { - expr.getArray().acceptVisitor(this); + if (!canceled) { + expr.getIndex().acceptVisitor(this); + if (!canceled && expr.getArray() != null) { + expr.getArray().acceptVisitor(this); + } } afterVisit(expr); } diff --git a/core/src/main/java/org/teavm/ast/optimization/OptimizingVisitor.java b/core/src/main/java/org/teavm/ast/optimization/OptimizingVisitor.java index ba2abc7ce..5007ef45f 100644 --- a/core/src/main/java/org/teavm/ast/optimization/OptimizingVisitor.java +++ b/core/src/main/java/org/teavm/ast/optimization/OptimizingVisitor.java @@ -762,6 +762,13 @@ class OptimizingVisitor implements StatementVisitor, ExprVisitor { return false; } + VariableAccessFinder isVariableAccessed = new VariableAccessFinder(v -> v == optimization.arrayVariable + || v == optimization.unwrappedArrayVariable); + assign.getRightValue().acceptVisitor(isVariableAccessed); + if (isVariableAccessed.isFound()) { + return false; + } + optimization.elements.add(assign.getRightValue()); if (++optimization.arrayElementIndex == optimization.arraySize) { applyArrayOptimization(optimization); diff --git a/core/src/main/java/org/teavm/ast/optimization/VariableAccessFinder.java b/core/src/main/java/org/teavm/ast/optimization/VariableAccessFinder.java new file mode 100644 index 000000000..3b564a120 --- /dev/null +++ b/core/src/main/java/org/teavm/ast/optimization/VariableAccessFinder.java @@ -0,0 +1,45 @@ +/* + * Copyright 2020 Alexey Andreev. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.teavm.ast.optimization; + +import java.util.function.IntPredicate; +import org.teavm.ast.RecursiveVisitor; +import org.teavm.ast.VariableExpr; + +public class VariableAccessFinder extends RecursiveVisitor { + private IntPredicate predicate; + private boolean found; + + public VariableAccessFinder(IntPredicate predicate) { + this.predicate = predicate; + } + + public boolean isFound() { + return found; + } + + public void reset() { + found = false; + } + + @Override + public void visit(VariableExpr expr) { + if (predicate.test(expr.getIndex())) { + found = true; + cancel(); + } + } +}