Fix issues related to sigma nodes in phi updater

classlib/src/main/java/org/teavm/classlib/impl/console/JSStderrPrintStream.java

@@ -22,6 +22,9 @@ import org.teavm.jso.JSBody;
 public class JSStderrPrintStream extends JsConsolePrintStream {
     @Override
     public void print(String s) {
+        if (s == null) {
+            s = "null";
+        }
         if (PlatformDetector.isWebAssemblyGC()) {
             for (int i = 0; i < s.length(); ++i) {
                 WasmGCSupport.putCharStderr(s.charAt(i));

core/src/main/java/org/teavm/model/util/PhiUpdater.java

@@ -101,6 +101,7 @@ public class PhiUpdater {
     private Sigma[][] sigmas;
     private Predicate<Instruction> sigmaPredicate = instruction -> false;
     private int[][][] frontierVariableCache;
+    private Variable[][] pendingVariableMap;
 
     public int getSourceVariable(int var) {
         if (var >= variableToSourceMap.size()) {
@@ -139,8 +140,8 @@ public class PhiUpdater {
         phisByReceiver.clear();
         cfg = ProgramUtils.buildControlFlowGraph(program);
         domTree = GraphUtils.buildDominatorTree(cfg);
-        domFrontiers = new int[cfg.size()][];
         domGraph = GraphUtils.buildDominatorGraph(domTree, program.basicBlockCount());
+        pendingVariableMap = new Variable[program.basicBlockCount()][];
 
         variableMap = new Variable[program.variableCount()];
         usedDefinitions = new boolean[program.variableCount()];
@@ -207,11 +208,7 @@ public class PhiUpdater {
                 Sigma sigma = new Sigma(block, variables[j]);
                 sigmasInBlock[j] = sigma;
                 for (BasicBlock target : targets) {
-                    Variable outgoingVar = program.createVariable();
+                    Outgoing outgoing = new Outgoing(variables[j], target);
-                    variableToSourceMap.add(sigma.getValue().getIndex());
-                    outgoingVar.setDebugName(sigma.getValue().getDebugName());
-                    outgoingVar.setLabel(sigma.getValue().getLabel());
-                    Outgoing outgoing = new Outgoing(outgoingVar, target);
                     sigma.getOutgoings().add(outgoing);
                 }
             }
@@ -231,42 +228,28 @@ public class PhiUpdater {
             int i = stack.removeLast();
             currentBlock = program.basicBlockAt(i);
 
-            for (int predecessor : cfg.incomingEdges(i)) {
-                if (sigmas[predecessor] == null) {
-                    continue;
-                }
-                if (domTree.immediateDominatorOf(i) != predecessor) {
-                    continue;
-                }
-                for (Sigma sigma : sigmas[predecessor]) {
-                    for (Outgoing outgoing : sigma.getOutgoings()) {
-                        if (outgoing.getTarget() == currentBlock) {
-                            markAssignment(sigma.getValue());
-                            break;
-                        }
-                    }
-                }
-            }
-
             if (currentBlock.getExceptionVariable() != null) {
-                markAssignment(currentBlock.getExceptionVariable());
+                markAssignment(currentBlock.getExceptionVariable(), currentBlock);
             }
 
             for (Phi phi : currentBlock.getPhis()) {
-                markAssignment(phi.getReceiver());
+                markAssignment(phi.getReceiver(), currentBlock);
             }
 
             for (Instruction insn : currentBlock) {
                 currentBlock = program.basicBlockAt(i);
                 insn.acceptVisitor(definitionExtractor);
                 for (Variable var : definitionExtractor.getDefinedVariables()) {
-                    markAssignment(var);
+                    markAssignment(var, currentBlock);
                 }
             }
 
-            if (sigmas[i] != null) {
+            var sigmasAtBlock = sigmas[i];
-                for (Sigma sigma : sigmas[i]) {
+            if (sigmasAtBlock != null) {
-                    markAssignment(sigma.getValue());
+                for (var sigma : sigmasAtBlock) {
+                    for (var outgoing : sigma.getOutgoings()) {
+                        markAssignment(outgoing.getValue(), outgoing.getTarget());
+                    }
                 }
             }
 
@@ -292,7 +275,6 @@ public class PhiUpdater {
             var sigmas = getSigmasAt(block.getIndex());
             if (sigmas != null) {
                 for (var sigma : sigmas) {
-                    increaseDefinitionCount(sigma.getValue());
                     for (var i = 0; i < sigma.getOutgoings().size(); ++i) {
                         increaseDefinitionCount(sigma.getValue());
                     }
@@ -307,31 +289,22 @@ public class PhiUpdater {
         }
     }
 
-    private static class Task {
-        Variable[] variables;
-        BasicBlock block;
-    }
-
     private void renameVariables() {
-        Deque<Task> stack = new ArrayDeque<>();
+        Deque<BasicBlock> stack = new ArrayDeque<>();
         for (int i = 0; i < program.basicBlockCount(); ++i) {
             if (domGraph.incomingEdgesCount(i) == 0) {
-                Task task = new Task();
+                stack.push(program.basicBlockAt(i));
-                task.block = program.basicBlockAt(i);
+                pendingVariableMap[i] = variableMap.clone();
-                task.variables = variableMap.clone();
-                stack.push(task);
             }
         }
 
-        List<List<Incoming>> phiOutputs = ProgramUtils.getPhiOutputs(program);
-
         while (!stack.isEmpty()) {
             Collections.fill(definedVersions, null);
-            Task task = stack.pop();
 
-            currentBlock = task.block;
+            currentBlock = stack.pop();
             int index = currentBlock.getIndex();
-            variableMap = task.variables.clone();
+            variableMap = pendingVariableMap[index];
+            pendingVariableMap[index] = null;
 
             if (currentBlock.getExceptionVariable() != null) {
                 currentBlock.setExceptionVariable(define(currentBlock.getExceptionVariable()));
@@ -355,30 +328,20 @@ public class PhiUpdater {
 
             int[] successors = domGraph.outgoingEdges(index);
 
-            for (Incoming output : phiOutputs.get(index)) {
+            Sigma[] nextSigmas = sigmas[index];
-                Variable var = output.getValue();
+            if (nextSigmas != null) {
-                Variable sigmaVar = applySigmaRename(output.getPhi().getBasicBlock(), var);
+                for (Sigma sigma : sigmas[index]) {
-                var = sigmaVar != var ? sigmaVar : use(var);
+                    sigma.setValue(use(sigma.getValue()));
-                output.setValue(var);
+                }
             }
 
-            Sigma[] nextSigmas = sigmas[index];
+            var oldVariableMap = variableMap;
             for (int j = successors.length - 1; j >= 0; --j) {
                 int successor = successors[j];
-                Task next = new Task();
+                var variables = oldVariableMap.clone();
-                next.variables = variableMap.clone();
+                pendingVariableMap[successor] = variables;
-                next.block = program.basicBlockAt(successor);
+                stack.push(program.basicBlockAt(successor));
-                if (nextSigmas != null) {
+                variableMap = variables;
-                    for (Sigma sigma : nextSigmas) {
-                        for (Outgoing outgoing : sigma.getOutgoings()) {
-                            if (outgoing.getTarget().getIndex() == successor) {
-                                next.variables[sigma.getValue().getIndex()] = outgoing.getValue();
-                                break;
-                            }
-                        }
-                    }
-                }
-                stack.push(next);
             }
 
             IntSet exceptionHandlingSuccessors = new IntHashSet();
@@ -387,14 +350,20 @@ public class PhiUpdater {
             }
 
             for (int successor : cfg.outgoingEdges(index)) {
+                variableMap = domTree.immediateDominatorOf(successor) == currentBlock.getIndex()
+                        ? pendingVariableMap[successor]
+                        : oldVariableMap.clone();
+                if (nextSigmas != null) {
+                    for (var sigma : nextSigmas) {
+                        for (var outgoing : sigma.getOutgoings()) {
+                            if (outgoing.getTarget().getIndex() == successor) {
+                                outgoing.setValue(define(outgoing.getValue()));
+                            }
+                        }
+                    }
+                }
                 renameOutgoingPhis(successor, exceptionHandlingSuccessors.contains(successor));
             }
-
-            if (sigmas[index] != null) {
-                for (Sigma sigma : sigmas[index]) {
-                    sigma.setValue(use(sigma.getValue()));
-                }
-            }
         }
     }
 
@@ -445,7 +414,6 @@ public class PhiUpdater {
 
         for (int j = 0; j < phis.size(); ++j) {
             Phi phi = phis.get(j);
-            Variable originalVar = program.variableAt(phiIndexes[j]);
             Variable var = variableMap[phiIndexes[j]];
             if (var != null) {
                 List<Variable> versions = definedVersions.get(phiIndexes[j]);
@@ -458,42 +426,38 @@ public class PhiUpdater {
                     }
                 }
 
-                Variable sigmaVar = applySigmaRename(program.basicBlockAt(successor), originalVar);
                 Incoming incoming = new Incoming();
                 incoming.setSource(currentBlock);
-                incoming.setValue(sigmaVar != originalVar ? sigmaVar : var);
+                incoming.setValue(var);
                 phi.getIncomings().add(incoming);
                 phi.getReceiver().setDebugName(var.getDebugName());
             }
         }
+
+        for (var phi : program.basicBlockAt(successor).getPhis()) {
+            for (var incoming : phi.getIncomings()) {
+                if (incoming.getSource() == currentBlock) {
+                    var value = variableMap[incoming.getValue().getIndex()];
+                    if (value != null) {
+                        incoming.setValue(value);
+                        usedPhis.set(value.getIndex());
+                    } else {
+                        usedPhis.set(incoming.getValue().getIndex());
+                    }
+                }
+            }
+        }
     }
 
-    private Variable applySigmaRename(BasicBlock target, Variable var) {
+    private void markAssignment(Variable var, BasicBlock targetBlock) {
-        Sigma[] blockSigmas = sigmas[currentBlock.getIndex()];
-        if (blockSigmas == null) {
-            return var;
-        }
-        for (Sigma sigma : blockSigmas) {
-            if (sigma.getValue() != var) {
-                continue;
-            }
-            for (Outgoing outgoing : sigma.getOutgoings()) {
-                if (outgoing.getTarget() == target) {
-                    return outgoing.getValue();
-                }
-            }
-        }
-        return var;
-    }
-
-    private void markAssignment(Variable var) {
         if (variableDefinitionCount[var.getIndex()] < 2) {
             variableDefined[var.getIndex()] = true;
             return;
         }
 
         Deque<BasicBlock> worklist = new ArrayDeque<>();
-        worklist.push(currentBlock);
+        worklist.push(targetBlock);
+        var isCurrentlyFrontier = currentBlock != targetBlock && cfg.incomingEdgesCount(targetBlock.getIndex()) > 1;
 
         if (variableDefined[var.getIndex()]) {
             for (TryCatchBlock tryCatch : currentBlock.getTryCatchBlocks()) {
@@ -505,7 +469,8 @@ public class PhiUpdater {
 
         while (!worklist.isEmpty()) {
             BasicBlock block = worklist.pop();
-            int[] frontiers = domFrontiers[block.getIndex()];
+            var frontiers = isCurrentlyFrontier ? new int[] { block.getIndex() } : domFrontiers[block.getIndex()];
+            isCurrentlyFrontier = false;
 
             if (frontiers != null) {
                 for (int frontier : frontiers) {

core/src/test/java/org/teavm/model/analysis/test/NullnessAnalysisTest.java

@@ -29,7 +29,6 @@ import java.io.InputStreamReader;
 import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TestName;
@@ -86,9 +85,6 @@ public class NullnessAnalysisTest {
     }
 
     @Test
-    @Ignore
-    // Fix this issue and un-ignore
-    // Also, un-ignore TestYear.test_isLeap
     public void nonDominatedBranch2() {
         test();
     }

core/src/test/resources/model/analysis/nullness/nonDominatedBranch.extended.txt

@@ -7,11 +7,11 @@ $ifNonNull
     @b := invokeStatic `Foo.g()LBar;`
     if @b !== null then goto $joint else goto $ifNull
 $ifNull
-    return @a_2
+    return @a_1
 $joint
-    @c := phi @a_1 from $start, @b_1 from $ifNonNull
+    @c := phi @a_2 from $start, @b_2 from $ifNonNull
     return @c
 
 // NULLABLE c
-// NULL a_1
+// NULL a_2
-// NOT_NULL a_2
+// NOT_NULL a_1

core/src/test/resources/model/analysis/nullness/nonDominatedBranch2.extended.txt

@@ -1,10 +1,12 @@
 var @this as this // 0
-$0
+
+$start
     @a := invokeStatic `Foo.f()LBar;`
-    if @a === null then goto $2 else goto $1
+    if @a === null then goto $joint else goto $ifNonNull
-$1
+$ifNonNull
-    goto $2
+    goto $joint
-$2
+$joint
-    @c := phi @a_1 from $0, @a_2 from $1
+    @c := phi @a_2 from $start, @a_1 from $ifNonNull
-    @d := @a
+    @a_3 := phi @a_2 from $start, @a_1 from $ifNonNull
+    @d := @a_3
     return @c

tests/src/test/java/org/teavm/classlib/java/time/temporal/TestYear.java

@@ -76,7 +76,6 @@ import java.time.temporal.TemporalQueries;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import org.junit.Ignore;
 import org.junit.runner.RunWith;
 import org.teavm.classlib.java.time.AbstractDateTimeTest;
 import org.teavm.junit.SkipPlatform;
@@ -332,8 +331,6 @@ public class TestYear extends AbstractDateTimeTest {
     // isLeap()
     //-----------------------------------------------------------------------
     @Test
-    @Ignore
-    // Fails due to bug in nullness analysis
     public void test_isLeap() {
         assertEquals(Year.of(1999).isLeap(), false);
         assertEquals(Year.of(2000).isLeap(), true);