Remove -disablePeephole

15 files changed, 19 insertions, 140 deletions

diff --git a/chapter18/src/main/java/com/seaofnodes/simple/CodeGen.java b/chapter18/src/main/java/com/seaofnodes/simple/CodeGen.java
index 65d820f5..a9dfe476 100644
--- a/chapter18/src/main/java/com/seaofnodes/simple/CodeGen.java
+++ b/chapter18/src/main/java/com/seaofnodes/simple/CodeGen.java
@@ -45,14 +45,12 @@ public class CodeGen {
     CodeGen( String src, TypeInteger arg ) { _phase = null; _src = src; _arg=arg; CODE=this; }
     public CodeGen( String src ) { this(src,TypeInteger.BOT); }
 
-    public CodeGen parse() { return parse(false); }
-    public CodeGen parse(boolean disable) {
+    public CodeGen parse() {
         assert _phase == null;
         _phase = Phase.Parse;
         _linker.clear();
 
         P = new Parser(_src,_arg);
-        Node._disablePeephole = disable;
         _stop = P.parse();
         _start = Parser.START;   // Capture global start
         if( _obs != null ) _obs.phase("Parse");
@@ -75,8 +73,7 @@ public class CodeGen {
 
     public CodeGen typeCheck() {
         // Demand phase Opto for cleaning up dead control flow at least,
-        // required for the following GCM.  Note that peeps can be disabled,
-        // but still the dead CFG will get cleaned.
+        // required for the following GCM.
         assert _phase == Phase.Opto;
         _phase = Phase.TypeCheck;
 
diff --git a/chapter18/src/main/java/com/seaofnodes/simple/node/IfNode.java b/chapter18/src/main/java/com/seaofnodes/simple/node/IfNode.java
index eafcdf5b..c49ee662 100644
--- a/chapter18/src/main/java/com/seaofnodes/simple/node/IfNode.java
+++ b/chapter18/src/main/java/com/seaofnodes/simple/node/IfNode.java
@@ -34,7 +34,6 @@ public class IfNode extends CFGNode implements MultiNode {
         // If the If node is not reachable then neither is any following Proj
         if (ctrl()._type != Type.CONTROL && ctrl()._type != Type.BOTTOM )
             return TypeTuple.IF_NEITHER;
-        if( _disablePeephole ) return TypeTuple.IF_BOTH;
         Node pred = pred();
         Type t = pred._type;
         // High types mean NEITHER side is reachable.
diff --git a/chapter18/src/main/java/com/seaofnodes/simple/node/Node.java b/chapter18/src/main/java/com/seaofnodes/simple/node/Node.java
index 637f4b14..0016c711 100644
--- a/chapter18/src/main/java/com/seaofnodes/simple/node/Node.java
+++ b/chapter18/src/main/java/com/seaofnodes/simple/node/Node.java
@@ -285,12 +285,6 @@ public abstract class Node implements Cloneable {
     // Graph-based optimizations
 
     /**
-     * We allow disabling peephole opt so that we can observe the
-     * full graph, vs the optimized graph.
-     */
-    public static boolean _disablePeephole = false;
-
-    /**
      * Try to peephole at this node and return a better replacement Node.
      * Always returns some not-null Node (often this).
      */
@@ -326,17 +320,8 @@ public abstract class Node implements Cloneable {
         // Compute initial or improved Type
         Type old = setType(compute());
 
-        // Peepholes can be turned off - except for cleaning up dead CFG paths.
-        // These need to clean up so the following code motion algorithms don't
-        // get confused by dead or infinite paths.
-        if( _disablePeephole && !(this instanceof RegionNode) && !(this instanceof PhiNode) )
-            return old==_type ? null : this;   // Peephole optimizations turned off
-
-        // Replace constant computations from non-constants with a constant
-        // node.  If peeps are disabled, still allow high Phis to collapse;
-        // they typically come from dead Regions, and we want the Region to
-        // collapse, which requires the Phis to die first.
-        if( _type.isHighOrConst() && !isConst() && (!_disablePeephole || _type.isHigh()) )
+        // Replace constant computations from non-constants with a constant node.
+        if( _type.isHighOrConst() && !isConst() )
             return ConstantNode.make(_type).peepholeOpt();
 
         // Global Value Numbering
@@ -633,7 +618,6 @@ public abstract class Node implements Cloneable {
      */
     public static void reset() {
         UNIQUE_ID = 1;
-        _disablePeephole=false;
         GVN.clear();
         ITER_CNT = ITER_NOP_CNT = 0;
     }
diff --git a/chapter18/src/main/java/com/seaofnodes/simple/node/PhiNode.java b/chapter18/src/main/java/com/seaofnodes/simple/node/PhiNode.java
index b3e36d41..1f3062e8 100644
--- a/chapter18/src/main/java/com/seaofnodes/simple/node/PhiNode.java
+++ b/chapter18/src/main/java/com/seaofnodes/simple/node/PhiNode.java
@@ -70,9 +70,6 @@ public class PhiNode extends Node {
         Node live = singleUniqueInput();
         if (live != null)
             return live;
-        // No more fancy peeps
-        if( _disablePeephole )
-            return null;
 
         // No bother if region is going to fold dead paths soon
         for( int i=1; i<nIns(); i++ )
diff --git a/chapter18/src/main/java/com/seaofnodes/simple/node/RegionNode.java b/chapter18/src/main/java/com/seaofnodes/simple/node/RegionNode.java
index 9a076dea..b3aa27be 100644
--- a/chapter18/src/main/java/com/seaofnodes/simple/node/RegionNode.java
+++ b/chapter18/src/main/java/com/seaofnodes/simple/node/RegionNode.java
@@ -68,8 +68,7 @@ public class RegionNode extends CFGNode {
             return in(1);       // Collapse if no Phis; 1-input Phis will collapse on their own
 
         // If a CFG diamond with no merging, delete: "if( pred ) {} else {};"
-        if( !_disablePeephole && // Optional
-            !hasPhi() &&         // No Phi users, just a control user
+        if( !hasPhi() &&         // No Phi users, just a control user
             in(1) instanceof CProjNode p1 &&
             in(2) instanceof CProjNode p2 &&
             p1.in(0).addDep(this)==p2.in(0).addDep(this) &&
diff --git a/chapter18/src/test/java/com/seaofnodes/simple/Chapter02Test.java b/chapter18/src/test/java/com/seaofnodes/simple/Chapter02Test.java
index 324ebf41..8b7a2f66 100644
--- a/chapter18/src/test/java/com/seaofnodes/simple/Chapter02Test.java
+++ b/chapter18/src/test/java/com/seaofnodes/simple/Chapter02Test.java
@@ -11,8 +11,8 @@ public class Chapter02Test {
     @Test
     public void testParseGrammar() {
         CodeGen code = new CodeGen("return 1+2*3+-5;");
-        code.parse(true); // Disable peepholes
-        assertEquals("return ((1+(2*3))+(-5));", code.print());
+        code.parse();
+        assertEquals("return 2;", code.print());
     }
 
     @Test
diff --git a/chapter18/src/test/java/com/seaofnodes/simple/Chapter03Test.java b/chapter18/src/test/java/com/seaofnodes/simple/Chapter03Test.java
index 825235cc..409e23ed 100644
--- a/chapter18/src/test/java/com/seaofnodes/simple/Chapter03Test.java
+++ b/chapter18/src/test/java/com/seaofnodes/simple/Chapter03Test.java
@@ -27,13 +27,6 @@ public class Chapter03Test {
     }
 
     @Test
-    public void testVarScopeNoPeephole() {
-        CodeGen code = new CodeGen("int a=1; int b=2; int !c=0; { int b=3; c=a+b;  } return c; ");
-        code.parse(true);
-        assertEquals("return (1+3);", code.print());
-    }
-
-    @Test
     public void testVarDist() {
         CodeGen code = new CodeGen("int x0=1; int y0=2; int x1=3; int y1=4; return (x0-x1)*(x0-x1) + (y0-y1)*(y0-y1); ");
         code.parse();
diff --git a/chapter18/src/test/java/com/seaofnodes/simple/Chapter07Test.java b/chapter18/src/test/java/com/seaofnodes/simple/Chapter07Test.java
index 7cef0d9c..6d6a3ce7 100644
--- a/chapter18/src/test/java/com/seaofnodes/simple/Chapter07Test.java
+++ b/chapter18/src/test/java/com/seaofnodes/simple/Chapter07Test.java
@@ -16,11 +16,9 @@ while(arg < 10) {
 }
 return arg;
 """);
-        Node._disablePeephole = true;
         code.parse();
         assertEquals("return Phi(Loop,arg,(Phi_arg+1));", code.print());
         assertTrue(code.ctrl() instanceof CProjNode);
-        Node._disablePeephole = false;
     }
 
     @Test
@@ -71,11 +69,9 @@ while(a < 10) {
 }
 return b;
 """);
-        Node._disablePeephole = true;
         code.parse();
         assertEquals("return Phi(Loop,2,Phi(Region,Phi_b,4));", code.print());
         assertTrue(code.ctrl() instanceof CProjNode);
-        Node._disablePeephole = false;
     }
 
     @Test
@@ -109,11 +105,9 @@ while(a < 10) {
 }
 return a;
 """);
-        Node._disablePeephole = true;
         code.parse();
         assertEquals("return Phi(Loop,1,(Phi_a+3));", code.print());
         assertTrue(code.ctrl() instanceof CProjNode);
-        Node._disablePeephole = false;
     }
 
     @Test
@@ -140,11 +134,9 @@ int a = 1;
 while(arg) a = 2;
 return a;
 """);
-        Node._disablePeephole = true;
         code.parse();
         assertEquals("return Phi(Loop,1,2);", code.print());
         assertTrue(code.ctrl() instanceof CProjNode);
-        Node._disablePeephole = false;
     }
 
     @Test
@@ -171,11 +163,9 @@ while(a < 10) {
 }
 return a;
 """);
-        Node._disablePeephole = true;
         code.parse();
         assertEquals("return Phi(Loop,1,(Phi_a+3));", code.print());
         assertTrue(code.ctrl() instanceof CProjNode);
-        Node._disablePeephole = false;
     }
 
     @Test
@@ -206,11 +196,9 @@ while(a < 10) {
 }
 return a;
 """);
-        Node._disablePeephole = true;
         code.parse();
         assertEquals("return Phi(Loop,1,(Phi_a+3));", code.print());
         assertTrue(code.ctrl() instanceof CProjNode);
-        Node._disablePeephole = false;
     }
 
     @Test
diff --git a/chapter18/src/test/java/com/seaofnodes/simple/Chapter11Test.java b/chapter18/src/test/java/com/seaofnodes/simple/Chapter11Test.java
index fc9f117c..2d7d46f7 100644
--- a/chapter18/src/test/java/com/seaofnodes/simple/Chapter11Test.java
+++ b/chapter18/src/test/java/com/seaofnodes/simple/Chapter11Test.java
@@ -7,20 +7,6 @@ import org.junit.Test;
 import static org.junit.Assert.assertEquals;
 
 public class Chapter11Test {
-    @Test public void testPrintingPreservesGraph() throws Exception {
-        var code = new CodeGen("while(arg < 10) arg = arg + 1; return arg;").parse().opto().typeCheck().GCM().localSched();
-        var stop = code._stop;
-        var bfs = new IRPrinter.BFS(stop,99);
-        for( var n : bfs._bfs )
-            if( n instanceof com.seaofnodes.simple.node.CFGNode cfg ) cfg._idepth = 0;
-        new com.seaofnodes.simple.node.ConstantNode(com.seaofnodes.simple.type.TypeInteger.constant(123456));
-        com.seaofnodes.simple.node.PrintTestSupport.unchanged(stop, () -> {
-            String first = stop.p(99);
-            assertEquals(first,stop.p(99));
-            stop.toString();
-        });
-    }
-
 
     // A placeholder test used to rapidly rotate through fuzzer produced issues
     @Test
diff --git a/chapter18/src/test/java/com/seaofnodes/simple/Chapter18Test.java b/chapter18/src/test/java/com/seaofnodes/simple/Chapter18Test.java
index 0ea32a20..9582ecce 100644
--- a/chapter18/src/test/java/com/seaofnodes/simple/Chapter18Test.java
+++ b/chapter18/src/test/java/com/seaofnodes/simple/Chapter18Test.java
@@ -85,24 +85,6 @@ public class Chapter18Test {
         org.junit.Assert.assertSame(declared,field.get(v));
     }
 
-    @Test public void testPrintingLazyMemory() throws Exception {
-        var code = new CodeGen("return 0;").parse();
-        var base = new ConstantNode(com.seaofnodes.simple.type.TypeMem.BOT);
-        var outer = new ScopeNode();
-        var inner = new ScopeNode();
-        var loop = new LoopNode(null,code._start);
-        loop._type = com.seaofnodes.simple.type.Type.CONTROL;
-        var outerMem = new MemMergeNode(true);
-        outerMem.addDef(null); outerMem.addDef(base); outerMem.addDef(base);
-        outer.addDef(loop); outer.addDef(outerMem);
-        var innerMem = new MemMergeNode(true);
-        innerMem.addDef(null); innerMem.addDef(outer); innerMem.addDef(outer);
-        inner.addDef(loop); inner.addDef(innerMem);
-        var mem = new MemMergeNode(true);
-        mem.addDef(null); mem.addDef(base); mem.addDef(inner);
-        PrintTestSupport.unchanged(mem, () -> org.junit.Assert.assertTrue(mem.toString().contains("Lazy_")));
-    }
-
 
     @Test
     public void testJig() {
diff --git a/chapter18/src/test/java/com/seaofnodes/simple/FuzzerWrap.java b/chapter18/src/test/java/com/seaofnodes/simple/FuzzerWrap.java
index 1427c99d..143828e6 100644
--- a/chapter18/src/test/java/com/seaofnodes/simple/FuzzerWrap.java
+++ b/chapter18/src/test/java/com/seaofnodes/simple/FuzzerWrap.java
@@ -18,7 +18,6 @@ public class FuzzerWrap {
     };
 
     private static final long[] OPEN_FAILING_SEEDS = {
-        973358943756616234L, // Parser failure on nullable field access after dead code
     };
 
     @Test         public void fuzzPeepsRegression  () { fuzzPeepsSeeds(REGRESSION_SEEDS); }
diff --git a/chapter18/src/test/java/com/seaofnodes/simple/SchedulerTest.java b/chapter18/src/test/java/com/seaofnodes/simple/SchedulerTest.java
index e869e133..d50c302a 100644
--- a/chapter18/src/test/java/com/seaofnodes/simple/SchedulerTest.java
+++ b/chapter18/src/test/java/com/seaofnodes/simple/SchedulerTest.java
@@ -30,7 +30,6 @@ int i=v.f;
 if (arg) v.f=1;
 return i;
 """);
-        StopNode._disablePeephole = true;
         code.parse();//.opto();
         var eval = new Evaluator(code._stop);
         assertEquals(2L, eval.evaluate(1, 10));
diff --git a/chapter18/src/test/java/com/seaofnodes/simple/fuzzer/Fuzzer.java b/chapter18/src/test/java/com/seaofnodes/simple/fuzzer/Fuzzer.java
index 9f2ffa4d..a5634e82 100644
--- a/chapter18/src/test/java/com/seaofnodes/simple/fuzzer/Fuzzer.java
+++ b/chapter18/src/test/java/com/seaofnodes/simple/fuzzer/Fuzzer.java
@@ -16,7 +16,7 @@ import java.util.function.Consumer;
  * randomly choose a variation the parser would parse and generate it.
  * These scripts are the parsed by the parser and all exceptions are caught and filtered
  * to only show one occurrence of one problem. This includes script which generate different
- * results when the compiled graph with and without peeps is executed.
+ * results when graphs optimized with different worklist seeds are executed.
  * To aid debugging scripts that cause errors are then reduced by applying rules and checking
  * that the same issue persists.
  */
@@ -73,7 +73,7 @@ public class Fuzzer {
     }
 
     /**
-     * Run checks for script. Compile the script with peeps enabled and disabled.
+     * Run checks for script. Compile the script with two different optimizer worklist seeds.
      * Check that exceptions raised in the parser by both methods are the same and only happens if the script may be invalid.
      * If the script was successfully parsed check that both version behave the same.
      * @param script The script to test
@@ -82,10 +82,10 @@ public class Fuzzer {
     private static void runCheck(String script, boolean valid) {
         CodeGen code1;
         try {
-            code1 = FuzzerUtils.parse(script, false);
+            code1 = FuzzerUtils.parse(script, 123);
         } catch (RuntimeException e1) {
             try {
-                FuzzerUtils.parse(script, true);
+                FuzzerUtils.parse(script, 456);
             } catch (RuntimeException e2) {
                 if (FuzzerUtils.isExceptionFromSameCause(e1, e2)) {
                     if (!valid || e1.getClass() == Parser.ParseException.class) return;
@@ -95,7 +95,7 @@ public class Fuzzer {
             }
             throw e1;
         }
-        CodeGen code2 = FuzzerUtils.parse(script, true);
+        CodeGen code2 = FuzzerUtils.parse(script, 456);
         checkGraphs(code1, code2, 0);
         checkGraphs(code1, code2, 1);
         checkGraphs(code1, code2, 10);
diff --git a/chapter18/src/test/java/com/seaofnodes/simple/fuzzer/FuzzerUtils.java b/chapter18/src/test/java/com/seaofnodes/simple/fuzzer/FuzzerUtils.java
index 62a149a8..4509c6fc 100644
--- a/chapter18/src/test/java/com/seaofnodes/simple/fuzzer/FuzzerUtils.java
+++ b/chapter18/src/test/java/com/seaofnodes/simple/fuzzer/FuzzerUtils.java
@@ -51,6 +51,7 @@ class FuzzerUtils {
      * Used to clear as exceptions might happen in the walk and left this bitset not cleared
      */
     private static final BitSet NodeWalkVisit;
+    private static final java.util.Random WorkRandom;
 
     /**
      * Write access to Iterate.MID_ASSERT
@@ -66,6 +67,7 @@ class FuzzerUtils {
     static {
         try {
             NodeWalkVisit = getFieldValue(Node.class, "WVISIT");
+            WorkRandom = getFieldValue(getFieldValue(IterPeeps.class, "WORK"), "_R");
             set_MID_ASSERT = MethodHandles.lookup().unreflectSetter(getField( IterPeeps.class, "MID_ASSERT"));
         } catch (Exception e) {
             throw new ExceptionInInitializerError(e);
@@ -112,9 +114,9 @@ class FuzzerUtils {
     }
 
     /**
-     * Parse script with peepholes enabled or disabled
+     * Parse and optimize using the given worklist seed
      */
-    public static CodeGen parse(String script, boolean runPeeps) {
+    public static CodeGen parse(String script, long workListSeed) {
         var err = System.err;
         var out = System.out;
         try {
@@ -126,7 +128,9 @@ class FuzzerUtils {
                 throw rethrow(e);
             }
             var code = new CodeGen(script);
-            return code.parse(!runPeeps).opto().typeCheck().GCM().localSched();
+            code.parse();
+            WorkRandom.setSeed(workListSeed);
+            return code.opto().typeCheck().GCM().localSched();
         } finally {
             NodeWalkVisit.clear();
             System.setErr(err);
diff --git a/chapter18/src/test/java/com/seaofnodes/simple/node/PrintTestSupport.java b/chapter18/src/test/java/com/seaofnodes/simple/node/PrintTestSupport.java
deleted file mode 100644
index ceea1de5..00000000
--- a/chapter18/src/test/java/com/seaofnodes/simple/node/PrintTestSupport.java
+++ /dev/null
@@ -1,48 +0,0 @@
-package com.seaofnodes.simple.node;
-import java.util.*;
-import static org.junit.Assert.*;
-
-// Inspect raw fields: the regression must not itself use lazy accessors.
-public class PrintTestSupport {
-    public static void unchanged(Node root, Runnable print) throws Exception {
-        var hash = Node.class.getDeclaredField("_hash");
-        hash.setAccessible(true);
-        var nodes = new ArrayList<Node>();
-        var seen = new IdentityHashMap<Node,Boolean>();
-        nodes.add(root);
-        seen.put(root,true);
-        for( int i=0; i<nodes.size(); i++ ) {
-            Node n = nodes.get(i);
-            for( Node def : n._inputs )
-                if( def!=null && seen.put(def,true)==null ) nodes.add(def);
-            for( Node use : n._outputs )
-                if( use!=null && seen.put(use,true)==null ) nodes.add(use);
-        }
-        var ins = new IdentityHashMap<Node,Node[]>();
-        var outs = new IdentityHashMap<Node,Node[]>();
-        var hashes = new IdentityHashMap<Node,Integer>();
-        var versions = new IdentityHashMap<Node,Integer>();
-        var depths = new IdentityHashMap<Node,Integer>();
-        for( Node n : nodes ) {
-            ins.put(n,n._inputs.toArray(new Node[0]));
-            outs.put(n,n._outputs.toArray(new Node[0]));
-            hashes.put(n,hash.getInt(n));
-            if( n instanceof CFGNode cfg ) {
-                depths.put(n,(int)cfg._idepth);
-                versions.put(n,(int)cfg._idepthVersion);
-            }
-        }
-        print.run();
-        for( Node n : nodes ) {
-            assertEquals("hash of node "+n._nid,(int)hashes.get(n),hash.getInt(n));
-            if( n instanceof CFGNode cfg ) {
-                assertEquals("depth of node "+n._nid,(int)depths.get(n),cfg._idepth);
-                assertEquals("depth version of node "+n._nid,(int)versions.get(n),cfg._idepthVersion);
-            }
-            assertEquals(ins.get(n).length,n.nIns());
-            assertEquals(outs.get(n).length,n.nOuts());
-            for( int i=0; i<n.nIns(); i++ ) assertSame(ins.get(n)[i],n.in(i));
-            for( int i=0; i<n.nOuts(); i++ ) assertSame(outs.get(n)[i],n._outputs.get(i));
-        }
-    }
-}