diff --git a/core/src/main/java/org/teavm/model/util/ControlFlowUtils.java b/core/src/main/java/org/teavm/model/util/ControlFlowUtils.java new file mode 100644 index 000000000..bd5dc0d62 --- /dev/null +++ b/core/src/main/java/org/teavm/model/util/ControlFlowUtils.java @@ -0,0 +1,56 @@ +/* + * Copyright 2016 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.model.util; + +import org.teavm.common.IntegerArray; +import org.teavm.common.Loop; +import org.teavm.common.LoopGraph; + +public final class ControlFlowUtils { + private ControlFlowUtils() { + } + + public static int[][] findLoopExits(LoopGraph cfg) { + IntegerArray[] loops = new IntegerArray[cfg.size()]; + for (Loop loop : cfg.knownLoops()) { + loops[loop.getHead()] = new IntegerArray(4); + } + + for (int node = 0; node < cfg.size(); ++node) { + Loop loop = cfg.loopAt(node); + while (loop != null) { + for (int successor : cfg.outgoingEdges(node)) { + Loop successorLoop = cfg.loopAt(successor); + if (successorLoop == null || !successorLoop.isChildOf(loop)) { + loops[loop.getHead()].add(node); + break; + } + } + loop = loop.getParent(); + } + } + + int[][] result = new int[cfg.size()][]; + for (int i = 0; i < loops.length; ++i) { + IntegerArray builder = loops[i]; + if (builder != null) { + result[i] = builder.getAll(); + } + } + + return result; + } +} diff --git a/core/src/main/java/org/teavm/model/util/PhiUpdater.java b/core/src/main/java/org/teavm/model/util/PhiUpdater.java index d69ee1796..5564fe954 100644 --- a/core/src/main/java/org/teavm/model/util/PhiUpdater.java +++ b/core/src/main/java/org/teavm/model/util/PhiUpdater.java @@ -15,10 +15,6 @@ */ package org.teavm.model.util; -import java.nio.ByteBuffer; -import java.nio.CharBuffer; -import java.nio.charset.CharsetDecoder; -import java.nio.charset.CoderResult; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -29,12 +25,9 @@ import org.teavm.common.DominatorTree; import org.teavm.common.Graph; import org.teavm.common.GraphUtils; import org.teavm.model.BasicBlock; -import org.teavm.model.ClassHolder; import org.teavm.model.Incoming; import org.teavm.model.Instruction; import org.teavm.model.InvokeDynamicInstruction; -import org.teavm.model.MethodDescriptor; -import org.teavm.model.MethodHolder; import org.teavm.model.Phi; import org.teavm.model.Program; import org.teavm.model.TryCatchBlock; @@ -77,7 +70,6 @@ import org.teavm.model.instructions.RaiseInstruction; import org.teavm.model.instructions.StringConstantInstruction; import org.teavm.model.instructions.SwitchInstruction; import org.teavm.model.instructions.UnwrapArrayInstruction; -import org.teavm.parsing.ClasspathClassHolderSource; public class PhiUpdater { private Program program; @@ -651,35 +643,4 @@ public class PhiUpdater { insn.setObjectRef(use(insn.getObjectRef())); } }; - - public static void main(String[] args) { - ClasspathClassHolderSource classSource = new ClasspathClassHolderSource(); - ClassHolder cls = classSource.get(CharsetDecoder.class.getName()); - MethodHolder method = cls.getMethod(new MethodDescriptor("decode", ByteBuffer.class, CharBuffer.class, - boolean.class, CoderResult.class)); - Program program = method.getProgram(); - - System.out.println(new ListingBuilder().buildListing(program, "")); - - Variable[] arguments = new Variable[method.parameterCount() + 1]; - for (int i = 0; i <= method.parameterCount(); ++i) { - arguments[i] = program.variableAt(i); - } - new PhiUpdater().updatePhis(program, arguments); - - System.out.println("After:"); - System.out.println(new ListingBuilder().buildListing(program, "")); - } - - private static void test() { - int a = 0; - try { - int x = Math.random() > 0.5 ? 50 : -50; - System.out.println("Foo " + x); - } catch (RuntimeException e) { - System.out.println("Foo " + a); - a = 23; - } - System.out.println("bar " + a); - } } diff --git a/core/src/main/java/org/teavm/model/util/ProgramUtils.java b/core/src/main/java/org/teavm/model/util/ProgramUtils.java index 5c43d5547..fa2e49d3f 100644 --- a/core/src/main/java/org/teavm/model/util/ProgramUtils.java +++ b/core/src/main/java/org/teavm/model/util/ProgramUtils.java @@ -171,4 +171,31 @@ public final class ProgramUtils { return outputs; } + + public static BasicBlock[] getVariableDefinitionPlaces(Program program) { + BasicBlock[] places = new BasicBlock[program.variableCount()]; + DefinitionExtractor defExtractor = new DefinitionExtractor(); + for (int i = 0; i < program.basicBlockCount(); ++i) { + BasicBlock block = program.basicBlockAt(i); + for (Phi phi : block.getPhis()) { + places[phi.getReceiver().getIndex()] = block; + } + for (TryCatchJoint joint : block.getTryCatchJoints()) { + places[joint.getReceiver().getIndex()] = block; + } + for (Instruction insn : block.getInstructions()) { + insn.acceptVisitor(defExtractor); + for (Variable var : defExtractor.getDefinedVariables()) { + places[var.getIndex()] = block; + } + } + for (TryCatchBlock tryCatch : block.getTryCatchBlocks()) { + Variable var = tryCatch.getExceptionVariable(); + if (var != null) { + places[var.getIndex()] = tryCatch.getHandler(); + } + } + } + return places; + } } diff --git a/core/src/main/java/org/teavm/optimization/LoopInvariantMotion.java b/core/src/main/java/org/teavm/optimization/LoopInvariantMotion.java index 4f868651f..2f620eab3 100644 --- a/core/src/main/java/org/teavm/optimization/LoopInvariantMotion.java +++ b/core/src/main/java/org/teavm/optimization/LoopInvariantMotion.java @@ -23,10 +23,6 @@ import org.teavm.model.*; import org.teavm.model.instructions.*; import org.teavm.model.util.*; -/** - * - * @author Alexey Andreev - */ public class LoopInvariantMotion implements MethodOptimization { private int[] preheaders; private LoopGraph graph; @@ -58,8 +54,14 @@ public class LoopInvariantMotion implements MethodOptimization { UsageExtractor useExtractor = new UsageExtractor(); InstructionAnalyzer analyzer = new InstructionAnalyzer(); CopyConstantVisitor constantCopier = new CopyConstantVisitor(); + int[][] loopExits = ControlFlowUtils.findLoopExits(graph); + while (!stack.isEmpty()) { int v = stack.pop(); + Loop defLoop = graph.loopAt(v); + int[] exits = loopExits[v]; + boolean dominatesExits = exits != null && Arrays.stream(exits) + .allMatch(exit -> dom.dominates(v, exit)); BasicBlock block = program.basicBlockAt(v); insnLoop: for (int i = 0; i < block.getInstructions().size(); ++i) { Instruction insn = block.getInstructions().get(i); @@ -70,6 +72,7 @@ public class LoopInvariantMotion implements MethodOptimization { } analyzer.canMove = false; analyzer.constant = false; + analyzer.sideEffect = false; insn.acceptVisitor(analyzer); if (analyzer.constant) { constantInstructions[defs[0].getIndex()] = insn; @@ -77,10 +80,12 @@ public class LoopInvariantMotion implements MethodOptimization { if (!analyzer.canMove) { continue; } - Loop defLoop = graph.loopAt(v); if (defLoop == null) { continue; } + if (analyzer.sideEffect && !dominatesExits) { + continue; + } insn.acceptVisitor(useExtractor); Loop commonUseLoop = null; for (Variable use : useExtractor.getUsedVariables()) { @@ -142,7 +147,7 @@ public class LoopInvariantMotion implements MethodOptimization { int preheader = preheaders[header]; if (preheader < 0) { int[] entries = getLoopEntries(header); - if (entries.length == 1) { + if (entries.length == 1 && graph.outgoingEdgesCount(entries[0]) == 1) { preheader = entries[0]; } else { preheader = insertPreheader(header); @@ -206,7 +211,8 @@ public class LoopInvariantMotion implements MethodOptimization { private static class InstructionAnalyzer implements InstructionVisitor { boolean canMove; - public boolean constant; + boolean constant; + boolean sideEffect; @Override public void visit(EmptyInstruction insn) { @@ -250,6 +256,10 @@ public class LoopInvariantMotion implements MethodOptimization { @Override public void visit(BinaryInstruction insn) { canMove = true; + if (insn.getOperation() == BinaryOperation.DIVIDE) { + sideEffect = insn.getOperandType() == NumericOperandType.INT + || insn.getOperandType() == NumericOperandType.LONG; + } } @Override @@ -323,8 +333,8 @@ public class LoopInvariantMotion implements MethodOptimization { @Override public void visit(ArrayLengthInstruction insn) { - // TODO: Sometimes we can cast NPE when array is null and its length is read only in certain cases - //canMove = true; + canMove = true; + sideEffect = true; } @Override @@ -333,8 +343,8 @@ public class LoopInvariantMotion implements MethodOptimization { @Override public void visit(UnwrapArrayInstruction insn) { - // TODO: Sometimes we can cast NPE when array is null and is is unwrapped only in certain cases - //canMove = true; + canMove = true; + sideEffect = true; } @Override @@ -365,6 +375,7 @@ public class LoopInvariantMotion implements MethodOptimization { @Override public void visit(NullCheckInstruction insn) { canMove = true; + sideEffect = true; } @Override diff --git a/core/src/main/java/org/teavm/optimization/LoopInversionImpl.java b/core/src/main/java/org/teavm/optimization/LoopInversionImpl.java index 20de56b2d..571682b38 100644 --- a/core/src/main/java/org/teavm/optimization/LoopInversionImpl.java +++ b/core/src/main/java/org/teavm/optimization/LoopInversionImpl.java @@ -40,10 +40,18 @@ import org.teavm.model.Program; import org.teavm.model.TryCatchBlock; import org.teavm.model.TryCatchJoint; import org.teavm.model.Variable; +import org.teavm.model.instructions.ArrayLengthInstruction; +import org.teavm.model.instructions.BinaryInstruction; +import org.teavm.model.instructions.BinaryOperation; +import org.teavm.model.instructions.NullCheckInstruction; +import org.teavm.model.instructions.NumericOperandType; +import org.teavm.model.instructions.UnwrapArrayInstruction; import org.teavm.model.util.BasicBlockMapper; +import org.teavm.model.util.DefinitionExtractor; import org.teavm.model.util.InstructionCopyReader; import org.teavm.model.util.PhiUpdater; import org.teavm.model.util.ProgramUtils; +import org.teavm.model.util.UsageExtractor; /** * Transforms loop in form: @@ -82,10 +90,12 @@ class LoopInversionImpl { private DominatorTree dom; private boolean postponed; private boolean changed; + private BasicBlock[] definitionPlaces; LoopInversionImpl(Program program, int parameterCount) { this.program = program; this.parameterCount = parameterCount; + definitionPlaces = ProgramUtils.getVariableDefinitionPlaces(program); } void apply() { @@ -194,7 +204,12 @@ class LoopInversionImpl { return false; } - collectNodesToCopy(); + IntSet nodesToCopy = nodesToCopy(); + if (!isInversionProfitable(nodesToCopy)) { + return false; + } + copyBasicBlocks(nodesToCopy); + copyCondition(); moveBackEdges(); removeInternalPhiInputsFromCondition(); @@ -204,6 +219,49 @@ class LoopInversionImpl { return true; } + private boolean isInversionProfitable(IntSet nodesToCopy) { + UsageExtractor useExtractor = new UsageExtractor(); + DefinitionExtractor defExtractor = new DefinitionExtractor(); + for (int node : nodes.toArray()) { + if (nodesToCopy.contains(node)) { + continue; + } + BasicBlock block = program.basicBlockAt(node); + Set currentInvariants = new HashSet<>(); + for (Instruction insn : block.getInstructions()) { + insn.acceptVisitor(useExtractor); + boolean invariant = Arrays.stream(useExtractor.getUsedVariables()).allMatch(var -> { + if (currentInvariants.contains(var)) { + return true; + } + BasicBlock definedAt = var.getIndex() < definitionPlaces.length + ? definitionPlaces[var.getIndex()] + : null; + return definedAt == null || dom.dominates(definedAt.getIndex(), block.getIndex()); + }); + if (invariant) { + if (becomesInvariant(insn)) { + return true; + } + insn.acceptVisitor(defExtractor); + currentInvariants.addAll(Arrays.asList(defExtractor.getDefinedVariables())); + } + } + } + return false; + } + + private boolean becomesInvariant(Instruction insn) { + if (insn instanceof BinaryInstruction) { + BinaryInstruction binary = (BinaryInstruction) insn; + return binary.getOperation() == BinaryOperation.DIVIDE + && (binary.getOperandType() == NumericOperandType.INT + || binary.getOperandType() == NumericOperandType.LONG); + } + return insn instanceof ArrayLengthInstruction || insn instanceof UnwrapArrayInstruction + || insn instanceof NullCheckInstruction; + } + private boolean findCondition() { IntSet tailNodes = new IntOpenHashSet(program.basicBlockCount()); for (int tailCandidate : cfg.incomingEdges(head)) { @@ -226,12 +284,12 @@ class LoopInversionImpl { return candidate != bodyStart; } - private void collectNodesToCopy() { + private void copyBasicBlocks(IntSet nodesToCopy) { int[] nodes = this.nodes.toArray(); Arrays.sort(nodes); for (int node : nodes) { nodesAndCopies.add(node); - if (node == head || (node != bodyStart && !dom.dominates(bodyStart, node))) { + if (nodesToCopy.contains(node)) { int copy = program.createBasicBlock().getIndex(); if (head == node) { headCopy = copy; @@ -242,6 +300,18 @@ class LoopInversionImpl { } } + private IntSet nodesToCopy() { + IntSet result = new IntOpenHashSet(); + int[] nodes = this.nodes.toArray(); + Arrays.sort(nodes); + for (int node : nodes) { + if (node == head || (node != bodyStart && !dom.dominates(bodyStart, node))) { + result.add(node); + } + } + return result; + } + private void copyCondition() { BasicBlockMapper blockMapper = new BasicBlockMapper(block -> copiedNodes.getOrDefault(block, block)); diff --git a/tests/src/test/java/org/teavm/classlib/java/lang/VMTest.java b/tests/src/test/java/org/teavm/classlib/java/lang/VMTest.java index 98abb6aad..14476351c 100644 --- a/tests/src/test/java/org/teavm/classlib/java/lang/VMTest.java +++ b/tests/src/test/java/org/teavm/classlib/java/lang/VMTest.java @@ -107,8 +107,8 @@ public class VMTest { // See https://github.com/konsoletyper/teavm/issues/167 @Test public void passesStaticFieldToSuperClassConstructor() { - SubClass obj = new SubClass(); - assertNotNull(obj.getValue()); + SubClass obj = new SubClass(); + assertNotNull(obj.getValue()); } // See https://github.com/konsoletyper/teavm/issues/196 diff --git a/tests/src/test/java/org/teavm/optimizations/LoopInversionTest.java b/tests/src/test/java/org/teavm/optimizations/LoopInversionTest.java deleted file mode 100644 index dada88ac6..000000000 --- a/tests/src/test/java/org/teavm/optimizations/LoopInversionTest.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright 2016 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.optimizations; - -import static org.junit.Assert.assertEquals; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.teavm.junit.TeaVMTestRunner; - -@RunWith(TeaVMTestRunner.class) -public class LoopInversionTest { - @Test - public void respectsLoopOutput() { - int a = 0; - int b = 1; - for (int i = 0; i < 10; ++i) { - int c = a + b; - a = b; - b = c; - } - assertEquals(55, a); - } -}