More code coverage cleanup

4 files changed, 62 insertions, 99 deletions

diff --git a/chapter25/src/main/java/com/seaofnodes/simple/IterPeeps.java b/chapter25/src/main/java/com/seaofnodes/simple/IterPeeps.java
index 83616053..760a03ab 100644
--- a/chapter25/src/main/java/com/seaofnodes/simple/IterPeeps.java
+++ b/chapter25/src/main/java/com/seaofnodes/simple/IterPeeps.java
@@ -97,7 +97,7 @@ public class IterPeeps {
 
     // Run all the code-reduction and type-lifting peeps as possible
     private void iteratePeeps( CodeGen code ) {
-        assert !CodeGen.expensiveAssert(1) || progressOnList(code, _work, true);
+        assert !CodeGen.expensiveAssert(1) || progressOnList(code, _work);
         int cnt=0;
 
         Node n;
@@ -108,7 +108,7 @@ public class IterPeeps {
             if( n instanceof CallEndNode cend )
                 _workInline.push(cend);
             if( x != null ) {
-                if( x.isDead() ) continue;
+                assert !x.isDead(); // Peepholes return alive answers
                 // peepholeOpt can return brand-new nodes, needing an initial type set
                 if( x._type==null ) x.setType(x.compute());
                 // Changes require neighbors onto the worklist
@@ -125,7 +125,7 @@ public class IterPeeps {
                     // If the result is not self, revisit all inputs (because
                     // there's a new user), and replace in the graph.
                     if( x != n ) {
-                        for( Node z : n. _inputs ) _work.push(z);
+                        //for( Node z : n. _inputs ) _work.push(z);
                         for( Node z : x._outputs ) _work.push(z);
                         n.subsume(x);
                     }
@@ -134,10 +134,12 @@ public class IterPeeps {
                 n.moveDepsToWorklist();
                 JSViewer.show(); // Show again
                 // Very expensive assert.
-                assert !CodeGen.expensiveAssert(cnt) || progressOnList(code, _work, true);
+                assert !CodeGen.expensiveAssert(cnt) || progressOnList(code, _work);
             }
-            if( n.isUnused() && !(n instanceof StopNode) )
+            if( n.isUnused() ) {
+                assert !(n instanceof StopNode); // StopNodes can die if all code in the compunit dies
                 n.kill();       // Just plain dead
+            }
         }
 
     }
@@ -155,26 +157,21 @@ public class IterPeeps {
     // {@link #Node.addDep} which is a side effect in an assert.  The {@link
     // #midAssert} is used to stop this side effect.
     // Pessimistic solver assert
-    public static boolean progressOnList(CodeGen code, WorkList<Node> list, boolean dir ) {
+    public static boolean progressOnList(CodeGen code, WorkList<Node> list ) {
         code._midAssert = true;
         Node changed = code._stop.walk( n -> {
             Node m = n;
             Type nval = n.compute();
+            // Ignore most in-progress things
+            if( n.iskeep() ) return null;
 
             // Types must be forwards, even if on the worklist.
-            boolean checkType = !n.iskeep() ||
-                n._nid <= 8 ||
-                (n instanceof ProjNode && n.in(0) instanceof StartNode);
-
-            boolean monotonic = dir
-                ? nval.isa(n._type) // Pesi: new value lifts over old
-                : n._type.isa(nval); // Opto: new value falls over old
-            assert !checkType || monotonic : "Non-monotonic peep: "+n+"#"+n._nid+" old="+n._type+" new="+nval+" inputs="+inputTypes(n)+" peep="+m;
+            assert nval.isa(n._type) : "Non-monotonic peep: "+n+"#"+n._nid+" old="+n._type+" new="+nval+" inputs="+inputTypes(n)+" peep="+m;
             if( list.on(n) )
-                return null;
+                return null;    // On worklist is ok!
             if( n instanceof CallEndNode cend ) {
                 if( code._iter._workInline.on(cend) )
-                    return null;
+                    return null; // On inline worklist is ok!
                 assert cend.maybeInline() <= 0 : "Inline fired and not on worklist, CallEndNode#"+cend._nid;
             }
 
@@ -251,8 +248,6 @@ public class IterPeeps {
          * True if Node is on the WorkList
          */
         public boolean on( E x ) { return _on.get(x._nid); }
-        boolean isEmpty() { return _len==0; }
-        Node[] asAry() { return Arrays.copyOf(_es,_len); }
 
         /**
          * Removes a random Node from the WorkList; null if WorkList is empty
@@ -266,11 +261,13 @@ public class IterPeeps {
             return x;
         }
 
-        public void clear() {
-            _len = 0;
-            _on.clear();
-            _R.setSeed(_seed);
-            _totalWork = 0;
-        }
+        //boolean isEmpty() { return _len==0; }
+        //Node[] asAry() { return Arrays.copyOf(_es,_len); }
+        //public void clear() {
+        //    _len = 0;
+        //    _on.clear();
+        //    _R.setSeed(_seed);
+        //    _totalWork = 0;
+        //}
     }
 }
diff --git a/chapter25/src/main/java/com/seaofnodes/simple/codegen/GlobalCodeMotion.java b/chapter25/src/main/java/com/seaofnodes/simple/codegen/GlobalCodeMotion.java
index f13e8ae3..71c08d18 100644
--- a/chapter25/src/main/java/com/seaofnodes/simple/codegen/GlobalCodeMotion.java
+++ b/chapter25/src/main/java/com/seaofnodes/simple/codegen/GlobalCodeMotion.java
@@ -34,9 +34,8 @@ public abstract class GlobalCodeMotion {
     private static void _rpo_cfg(CFGNode def, Node use, BitSet visit, Ary<CFGNode> rpo) {
         if( !(use instanceof CFGNode cfg) || visit.get(cfg._nid) )
             return;             // Been there, done that
-        if( def instanceof ReturnNode && use instanceof CallEndNode )
-            return;
-        assert !( def instanceof CallNode && use instanceof FunNode ); // All calls unwired now
+        assert !( def instanceof ReturnNode && use instanceof CallEndNode ); // All calls unwired now
+        assert !( def instanceof   CallNode && use instanceof     FunNode ); // All calls unwired now
         visit.set(cfg._nid);
         for( Node useuse : cfg._outputs )
             _rpo_cfg(cfg,useuse,visit,rpo);
@@ -53,7 +52,7 @@ public abstract class GlobalCodeMotion {
             Node con = start.out(i);
             if( isStartPinnedGlobalValue(con) ) {
                 breakUpGlobalConstantSingle(con);
-                if( con.nIns()==0 || con.in(0) != start )
+                if( con.in(0) != start )
                     i--;    // Removed a global constant, re-run same index
             }
         }
@@ -65,8 +64,7 @@ public abstract class GlobalCodeMotion {
         // will recursively clone its Start-pinned inputs on demand.
         for( Node use : con.outs() )
             if( use != null && useFun(use)==null ) {
-                if( !isStartPinnedGlobalValue(use) )
-                    return;
+                assert isStartPinnedGlobalValue(use);
                 breakUpGlobalConstantSingle(use);
             }
         // While constant has users in different functions
@@ -77,7 +75,7 @@ public abstract class GlobalCodeMotion {
             for( Node use : con.outs() ) {
                 if( use == null ) continue;
                 FunNode fun2 = useFun(use);
-                if( fun2==null || !CodeGen.CODE.owns(fun2) ) continue;
+                assert fun2 != null && CodeGen.CODE.owns(fun2);
                 if( fun==null || fun==fun2 ) fun=fun2;
                 else { done=false; break; }
             }
@@ -144,29 +142,26 @@ public abstract class GlobalCodeMotion {
     }
 
     private static FunNode useFun(Node use) {
-        return useFun(use,new BitSet());
-    }
-
-    private static FunNode useFun(Node use, BitSet visit) {
-        if( visit.get(use._nid) ) return null;
-        visit.set(use._nid);
         if( use instanceof ReturnNode ret )
             return ret.fun();
         if( use instanceof ParmNode parm )
             return parm.fun();
         CFGNode cfg = use.cfg0();
-        FunNode fun = cfg==null ? null : cfg.fun();
+        if( cfg!=null ) {
+            FunNode fun = cfg.fun();
+            if( fun != null ) return fun;
+        }
         // Use itself is not in a function... which makes it a 2-part
         // constant such as happens on some chips where large constants have
         // to be built up in parts.  PtrToInt followed by the array-body Add is
         // another zero-code/global-value chain, so walk through any number of
         // single-use pieces until reaching the function-owned use.
-        if( fun==null )
-            for( Node out : use.outs() ) {
-                FunNode f = useFun(out,(BitSet)visit.clone());
-                if( f != null && fun != null && f != fun ) return null;
-                if( f != null ) fun = f;
-            }
+        FunNode fun = null;
+        for( Node out : use.outs() ) {
+            FunNode f = useFun(out);
+            if( f != null && fun != null && f != fun ) return null;
+            if( f != null ) fun = f;
+        }
         return fun;
     }
 
@@ -254,10 +249,10 @@ public abstract class GlobalCodeMotion {
                             // Load-use directly defines memory
                             (memuse._type instanceof TypeMem ||
                              // Load-use directly defines memory
-                             memuse instanceof CallNode ||
-                             // Load-use indirectly defines memory
-                             (memuse._type instanceof TypeTuple tt && tt._types[ld._alias] instanceof TypeMem)) )
+                             memuse instanceof CallNode) ) {
+                            assert !( memuse._type instanceof TypeTuple tt && tt._types[ld._alias] instanceof TypeMem );
                             continue outer;
+                        }
 
                 // All uses done, schedule
                 _doSchedLate(n,ns,late);
@@ -296,33 +291,6 @@ public abstract class GlobalCodeMotion {
         if( n instanceof MemOpNode load && load._isLoad )
             lca = find_anti_dep(lca,load,early,late);
 
-
-        // Nodes setting a single register and getting killed will stay close
-        // to the uses, since they will be forced to spill anyway.  The kill
-        // check is very weak, and some may be hoisted only to spill in the RA.
-        if( n instanceof MachNode mach ) {
-            RegMask out = mach.outregmap();
-            if( out!=null && out.size1() ) {
-                int reg = mach.outregmap().firstReg();
-                // Look for kills
-                outer:
-                for( CFGNode lca2=lca; lca2 != early; lca2 = lca2.idom() ) {
-                    if( lca2 instanceof MachNode mach2 ) {
-                        for( int i=1; i<lca2.nIns(); i++ ) {
-                            RegMask mask = mach2.regmap(i);
-                            if( mask!=null && mask.test(reg) ) {
-                                early = lca2 instanceof IfNode ? lca2.idom() : lca2;
-                                break outer;
-                            }
-                        }
-                        RegMask kill = mach2.killmap();
-                        if( kill != null )
-                            throw Utils.TODO();
-                    }
-                }
-            }
-        }
-
         // Walk up from the LCA to the early, looking for best place.  This is
         // the lowest execution frequency, approximated by least loop depth and
         // deepest control flow.
@@ -339,7 +307,7 @@ public abstract class GlobalCodeMotion {
     // Block of use.  Normally from late[] schedule, except for Phis, which go
     // to the matching Region input.
     private static CFGNode use_block(Node n, Node use, CFGNode[] late) {
-        if( use instanceof ParmNode parm && n.cfg0().fun()==parm.fun() )
+        if( use instanceof ParmNode parm )
             return late[use._nid];
         if( !(use instanceof PhiNode phi) )
             return late[use._nid];
@@ -364,7 +332,7 @@ public abstract class GlobalCodeMotion {
     private static CFGNode find_anti_dep(CFGNode lca, MemOpNode load, CFGNode early, CFGNode[] late) {
         // We could skip final-field loads here.
         // Walk LCA->early, flagging Load's block location choices
-        for( CFGNode cfg=lca; early!=null && cfg!=early.idom(); cfg = cfg.idom() )
+        for( CFGNode cfg=lca; cfg!=early.idom(); cfg = cfg.idom() )
             cfg._anti = load._nid;
         // Walk load->mem uses, looking for Stores causing an anti-dep
         for( Node mem : load.mem()._outputs ) {
@@ -392,11 +360,9 @@ public abstract class GlobalCodeMotion {
                     if( phi.in(i)==load.mem() )
                         lca = anti_dep(load,phi.region().cfg(i),load.mem().cfg0(),lca,null);
                 break;
-            case NewNode st: break;
             case ReturnNode ret: break; // Load must already be ahead of Return
             case MemMergeNode ret: break; // Mem uses now on ScopeMin
-            case NeverNode never: break;
-            default: throw Utils.TODO();
+            default: throw Utils.TODO("Should not reach here");
             }
         }
         return lca;
diff --git a/chapter25/src/main/java/com/seaofnodes/simple/codegen/Opto.java b/chapter25/src/main/java/com/seaofnodes/simple/codegen/Opto.java
index e8062c90..690ae3ad 100644
--- a/chapter25/src/main/java/com/seaofnodes/simple/codegen/Opto.java
+++ b/chapter25/src/main/java/com/seaofnodes/simple/codegen/Opto.java
@@ -129,7 +129,9 @@ abstract public class Opto {
         int[] fidxs = tfp.fidxs();
         // An unresolved forward call can still carry the infinite, inferred
         // function set into Opto.  There is no finite call graph to link yet;
-        // type checking will report the unresolved reference afterwards.
+        // type checking will report the unresolved reference afterwards.  This
+        // can yet be valid if the call is eventually dead.
+        //   `if( false ) frefCall(args);`
         if( XInt.isHigh(fidxs) ) return;
         for( int fidx = XInt.next(fidxs,0); fidx >=0; fidx = XInt.next(fidxs,fidx) ) {
             // unlinkStart deliberately leaves uncalled functions in a
@@ -171,7 +173,8 @@ abstract public class Opto {
     private static void linkStart( CodeGen code, FunNode fun ) {
         assert !fun.isDead();
         StartCUNode start = fun._compunit._start;
-        if( fun.nIns() < 2 || fun.in(1) != start ) {
+        if( fun.nIns() < 2 ) {
+            assert fun.nIns() < 2 || fun.in(1) != start;
             assert !start.isDead();
             keepInCompUnit(code,fun);
             // Function is reachable by any *remote* caller who gets the pointer!
@@ -233,15 +236,14 @@ abstract public class Opto {
 
             // Link a Call which becomes alive.  Like above, adds new Call
             // Graph edges to the graph.
-            if( n instanceof CallNode call && oval.isHigh() && !nval.isHigh() && call.fptr()._type instanceof TypeFunPtr )
+            if( n instanceof CallNode call && nval == Type.CONTROL && call.fptr()._type instanceof TypeFunPtr )
                 linkCG(code,call.tfp(),call);
 
             // If an otherwise-dead function pointer escapes, any future linked
             // caller might find and call it.  Force the function to be alive
             // and called by Start.
-            if( n instanceof StartNode && n._type instanceof TypeTuple tt )
-                linkStart(code,tt);
-
+            if( n instanceof StartNode )
+                linkStart(code,(TypeTuple)n._type);
 
             // Since n._type changed, visit all output neighbors
             code._iter.addAll(n._outputs);
@@ -302,6 +304,7 @@ abstract public class Opto {
                 int mode = 0;
                 for( int i=1; i<n.nIns(); i++ )
                     mode |= numericEvidence(n.in(i),visited);
+                assert mode != 0;
                 if( mode != 0 )  {
                     // If both evidence, FP wins
                     modeNode.setMode((byte)((mode&2)==2 ? 2 : 1)).init();
@@ -315,7 +318,8 @@ abstract public class Opto {
     // Gather numeric evidence through Phi and interprocedural return cycles.
     // The visited set makes self- and mutually-recursive calls finite.
     private static int numericEvidence(Node n, BitSet visited) {
-        if( n==null || visited.get(n._nid) ) return 0;
+        if( n == null ) return 0;
+        if( visited.get(n._nid) ) return 0;
         visited.set(n._nid);
 
         Type t = n._type;
@@ -325,19 +329,15 @@ abstract public class Opto {
             return 0; // Something unrelated, like a ptr
 
         int evidence = 0;
+        assert !(n instanceof ConvertNode); // No test case
+        assert !(n instanceof    ModeNode);
         switch( n ) {
-        case ModeNode modeNode -> evidence = modeNode.mode();
         case PhiNode phi -> {}
         case ReturnNode ret -> {}
-        case ProjNode proj when proj._idx==2 && proj.in(0) instanceof CallEndNode cend -> {
+        case ProjNode proj when proj.in(0) instanceof CallEndNode cend -> {
             for( int i=1; i<cend.nIns(); i++ )
                 evidence |= numericEvidence(cend.in(i),visited);
         }
-        case ConvertNode convert -> {
-            Type dst = convert.dst();
-            evidence = dst instanceof TypeFloat   ? 2 :
-                       dst instanceof TypeInteger ? 1 : 0;
-        }
         default -> { return 0; }
         }
         for( int i=1; i<n.nIns(); i++ )
@@ -354,10 +354,9 @@ abstract public class Opto {
             Node use = code._start.out(i);
             if( use instanceof ConFldOffNode off ) {
                 Node con = off.asOffset();
-                if( con != null ) { // Can be null for trying to reference missing field
-                    off.subsume( con );
-                    i--;
-                }
+                assert con != null; // Can be null for trying to reference missing field
+                off.subsume( con );
+                i--;
             }
         }
     }
diff --git a/chapter25/src/main/java/com/seaofnodes/simple/node/LoadNode.java b/chapter25/src/main/java/com/seaofnodes/simple/node/LoadNode.java
index 10adeb3c..1c9cf963 100644
--- a/chapter25/src/main/java/com/seaofnodes/simple/node/LoadNode.java
+++ b/chapter25/src/main/java/com/seaofnodes/simple/node/LoadNode.java
@@ -121,7 +121,8 @@ public class LoadNode extends MemOpNode {
         // Simple Load-after-Store on same address.
         if( mem instanceof StoreNode st &&
             ptr == st.nnptr() &&
-            off() == st.off() ) { // Must check same object
+            off() == st.off() &&  // Must check same object
+            st.val()._type.isa(_type) ) {
             assert _name.equals(st._name); // Equiv class aliasing is perfect
             return extend(st.val());
         }
@@ -272,7 +273,7 @@ public class LoadNode extends MemOpNode {
         //// To avoid cyclic pushing a Load up then down, getting here means
         //// the load *must* replace with the high/constant.
         //return true;
-        if( px instanceof StoreNode st1 && ptr()==addDep(st1.nnptr() )&& off()==st1.off() )
+        if( px instanceof StoreNode st1 && ptr()==addDep(st1.nnptr() ) && off()==st1.off() && st1.val()._type.isa(_type) )
             // To avoid cyclic pushing a Load up then down, getting here means
             // the load *must* match against the Store
             return true;