From ad390247950ed3316247c5bf88f303f6afaf4400 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Sat, 3 Dec 2016 13:46:02 +0300 Subject: [PATCH] Prevent PhiUpdater from placing e-phis with source variable equal to receiver --- .../model/optimization/LoopInversionImpl.java | 15 ++-- .../java/org/teavm/model/util/PhiUpdater.java | 79 +++++++++++-------- tests/src/test/java/org/teavm/vm/VMTest.java | 23 ++++++ tools/idea/jps-common/teavm-jps-common.iml | 4 +- .../java/org/teavm/junit/TeaVMTestRunner.java | 16 +++- 5 files changed, 91 insertions(+), 46 deletions(-) diff --git a/core/src/main/java/org/teavm/model/optimization/LoopInversionImpl.java b/core/src/main/java/org/teavm/model/optimization/LoopInversionImpl.java index cbb69bcfd..09ca0d81b 100644 --- a/core/src/main/java/org/teavm/model/optimization/LoopInversionImpl.java +++ b/core/src/main/java/org/teavm/model/optimization/LoopInversionImpl.java @@ -152,14 +152,10 @@ class LoopInversionImpl { } private LoopWithExits getLoopWithExits(Map cache, Loop loop) { - LoopWithExits result = cache.get(loop); - if (result == null) { - result = new LoopWithExits(loop.getHead(), loop.getParent() != null - ? getLoopWithExits(cache, loop.getParent()) - : null); - cache.put(loop, result); - } - return result; + return cache.computeIfAbsent(loop, k -> + new LoopWithExits(loop.getHead(), loop.getParent() != null + ? getLoopWithExits(cache, loop.getParent()) + : null)); } private void sortLoops(LoopWithExits loop, Set visited, List target) { @@ -179,7 +175,6 @@ class LoopInversionImpl { final IntSet nodesAndCopies = new IntOpenHashSet(); final IntSet exits = new IntOpenHashSet(); int bodyStart; - int copyStart; int headCopy; final IntIntMap copiedNodes = new IntIntOpenHashMap(); boolean shouldSkip; @@ -352,7 +347,7 @@ class LoopInversionImpl { TryCatchJoint jointCopy = new TryCatchJoint(); jointCopy.setReceiver(joint.getReceiver()); jointCopy.getSourceVariables().addAll(joint.getSourceVariables()); - tryCatchCopy.getJoints().add(joint); + tryCatchCopy.getJoints().add(jointCopy); } } } 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 bddddc0d9..b86026352 100644 --- a/core/src/main/java/org/teavm/model/util/PhiUpdater.java +++ b/core/src/main/java/org/teavm/model/util/PhiUpdater.java @@ -244,7 +244,7 @@ public class PhiUpdater { TryCatchBlock tryCatch = currentBlock.getTryCatchBlocks().get(i); catchSuccessors.add(tryCatch.getHandler().getIndex()); for (TryCatchJoint joint : synthesizedJoints.get(index).get(i)) { - joint.setReceiver(define(joint.getReceiver())); + joint.setReceiver(defineForExceptionPhi(joint.getReceiver())); } } variableMap = regularVariableMap; @@ -308,36 +308,55 @@ public class PhiUpdater { while (head > 0) { BasicBlock block = worklist[--head]; int[] frontiers = domFrontiers[block.getIndex()]; - if (frontiers == null) { - continue; + + if (frontiers != null) { + for (int frontier : frontiers) { + BasicBlock frontierBlock = program.basicBlockAt(frontier); + if (frontierBlock.getExceptionVariable() == var) { + continue; + } + + boolean exists = frontierBlock.getPhis().stream() + .flatMap(phi -> phi.getIncomings().stream()) + .anyMatch(incoming -> incoming.getSource() == block && incoming.getValue() == var); + if (exists) { + continue; + } + + Phi phi = phiMap[frontier][var.getIndex()]; + if (phi == null) { + phi = new Phi(); + phi.setReceiver(var); + phiIndexMap[frontier][synthesizedPhis.get(frontier).size()] = var.getIndex(); + synthesizedPhis.get(frontier).add(phi); + phiMap[frontier][var.getIndex()] = phi; + worklist[head++] = frontierBlock; + } + } } - for (int frontier : frontiers) { - BasicBlock frontierBlock = program.basicBlockAt(frontier); - if (frontierBlock.getExceptionVariable() == var) { - continue; - } - - boolean exists = frontierBlock.getPhis().stream() - .flatMap(phi -> phi.getIncomings().stream()) - .anyMatch(incoming -> incoming.getSource() == block && incoming.getValue() == var); - if (exists) { - continue; - } - - Phi phi = phiMap[frontier][var.getIndex()]; - if (phi == null) { - phi = new Phi(); - phi.setReceiver(var); - phiIndexMap[frontier][synthesizedPhis.get(frontier).size()] = var.getIndex(); - synthesizedPhis.get(frontier).add(phi); - phiMap[frontier][var.getIndex()] = phi; - worklist[head++] = frontierBlock; + List tryCatchBlocks = block.getTryCatchBlocks(); + for (int i = 0; i < tryCatchBlocks.size(); i++) { + TryCatchBlock tryCatch = tryCatchBlocks.get(i); + TryCatchJoint joint = jointMap.computeIfAbsent(tryCatch, k -> new HashMap<>()).get(var); + if (joint == null) { + joint = new TryCatchJoint(); + joint.setReceiver(var); + synthesizedJoints.get(block.getIndex()).get(i).add(joint); + jointMap.get(tryCatch).put(var, joint); + worklist[head++] = tryCatch.getHandler(); } } } } + private Variable defineForExceptionPhi(Variable var) { + Variable original = var; + var = introduce(var); + mapVariable(original.getIndex(), var); + return var; + } + private Variable define(Variable var) { Variable old = variableMap[var.getIndex()]; Variable original = var; @@ -354,15 +373,13 @@ public class PhiUpdater { continue; } - Map joints = jointMap.computeIfAbsent(tryCatch, k -> new HashMap<>()); + Map joints = jointMap.get(tryCatch); + if (joints == null) { + continue; + } + TryCatchJoint joint = joints.get(original); if (joint == null) { - joint = new TryCatchJoint(); - joint.setReceiver(original); - joints.put(original, joint); - synthesizedJoints.get(currentBlock.getIndex()).get(i).add(joint); - } - if (joint.getReceiver() == var) { continue; } diff --git a/tests/src/test/java/org/teavm/vm/VMTest.java b/tests/src/test/java/org/teavm/vm/VMTest.java index 9765e0887..c8e2e2e54 100644 --- a/tests/src/test/java/org/teavm/vm/VMTest.java +++ b/tests/src/test/java/org/teavm/vm/VMTest.java @@ -18,6 +18,8 @@ package org.teavm.vm; import static org.junit.Assert.*; import org.junit.Test; import org.junit.runner.RunWith; +import org.teavm.jso.JSBody; +import org.teavm.junit.SkipJVM; import org.teavm.junit.TeaVMTestRunner; @RunWith(TeaVMTestRunner.class) @@ -132,6 +134,27 @@ public class VMTest { assertEquals("ok", AsyncClinitClass.state); } + @Test + @SkipJVM + public void loopAndExceptionPhi() { + int[] a = createArray(); + int s = 0; + for (int i = 0; i < 10; ++i) { + int x = 0; + try { + x += 2; + x += 3; + } catch (RuntimeException e) { + fail("Unexpected exception caught: " + x); + } + s += a[0] + a[1]; + } + assertEquals(30, s); + } + + @JSBody(params = {}, script = "return [1, 2]") + private static native int[] createArray(); + static int initCount = 0; private static class AsyncClinitClass { diff --git a/tools/idea/jps-common/teavm-jps-common.iml b/tools/idea/jps-common/teavm-jps-common.iml index 2c4d04d31..f8f512fdd 100644 --- a/tools/idea/jps-common/teavm-jps-common.iml +++ b/tools/idea/jps-common/teavm-jps-common.iml @@ -15,7 +15,9 @@ - + + + diff --git a/tools/junit/src/main/java/org/teavm/junit/TeaVMTestRunner.java b/tools/junit/src/main/java/org/teavm/junit/TeaVMTestRunner.java index f8ac3c13a..338989c30 100644 --- a/tools/junit/src/main/java/org/teavm/junit/TeaVMTestRunner.java +++ b/tools/junit/src/main/java/org/teavm/junit/TeaVMTestRunner.java @@ -228,11 +228,19 @@ public class TeaVMTestRunner extends Runner implements Filterable { }); for (TeaVMTestConfiguration configuration : configurations) { - TestRun run = compileByTeaVM(child, notifier, expectedExceptions, configuration, onSuccess.get(0)); - if (run != null) { - runs.add(run); - } else { + try { + TestRun run = compileByTeaVM(child, notifier, expectedExceptions, configuration, onSuccess.get(0)); + if (run != null) { + runs.add(run); + } else { + notifier.fireTestFinished(description); + latch.countDown(); + return; + } + } catch (Throwable e) { + notifier.fireTestFailure(new Failure(description, e)); notifier.fireTestFinished(description); + latch.countDown(); return; } }