Backport some of the code refactoring to previous chapters (#50)

* #44 Revise docs and comments

* #44 Add starting scope to xScopes

* #44 back some of the code refactoring port to previous chapters

* #44 add_def should not be public

* #44 add_def should not be public. Rename method names so that they are camelcase

15 files changed, 289 insertions, 239 deletions

diff --git a/chapter06/src/main/java/com/seaofnodes/simple/GraphVisualizer.java b/chapter06/src/main/java/com/seaofnodes/simple/GraphVisualizer.java
index c1c1c4c9..704d4802 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/GraphVisualizer.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/GraphVisualizer.java
@@ -40,14 +40,14 @@ public class GraphVisualizer {
         nodes(sb, all);
         
         // Now the scopes, in a cluster no edges
-        for (ScopeNode sn: parser._allScopes)
+        for (ScopeNode sn: parser._xScopes)
             scopes(sb, sn);
 
         // Walk the Node edges
         nodeEdges(sb, all);
 
         // Walk the Scope edges
-        for (ScopeNode sn: parser._allScopes)
+        for (ScopeNode sn: parser._xScopes)
             scopeEdges(sb, sn);
         
         sb.append("}\n");
@@ -207,29 +207,24 @@ public class GraphVisualizer {
      * Finds all nodes in the graph.
      */
     private Collection<Node> findAll(Parser parser) {
-        final StartNode start = Parser.START;
         final HashMap<Integer, Node> all = new HashMap<>();
-        for( Node n : start._outputs )
+        for( Node n : Parser.START._outputs )
+            walk(all, n);
+        for( Node n : parser._scope._inputs )
             walk(all, n);
-        // Scan symbol tables
-        for( HashMap<String,Integer> scope : parser._scope._scopes )
-            for (Integer i : scope.values())
-                walk(all, parser._scope.in(i));
         return all.values();
     }
 
     /**
      * Walk a subgraph and populate distinct nodes in the all list.
      */
-    private void walk(HashMap<Integer, Node> all, Node n) {
+    private void walk(HashMap<Integer,Node> all, Node n) {
         if(n == null ) return;
         if (all.get(n._nid) != null) return; // Been there, done that
         all.put(n._nid, n);
         for (Node c : n._inputs)
-            if (c != null)
-                walk(all, c);
-        for (Node c : n._outputs)
-            if (c != null)
-                walk(all, c);
+            walk(all, c);
+        for( Node c : n._outputs )
+            walk(all, c);
     }
 }
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/Parser.java b/chapter06/src/main/java/com/seaofnodes/simple/Parser.java
index 1bfe5b0d..81cb9e57 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/Parser.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/Parser.java
@@ -39,7 +39,7 @@ public class Parser {
      * <p>
      * We keep a list of all ScopeNodes so that we can show them in graphs.
      * @see #parseIf()
-     * @see #_allScopes
+     * @see #_xScopes
      */
     public ScopeNode _scope;
 
@@ -60,14 +60,13 @@ public class Parser {
      * We clone ScopeNodes when control flows branch; it is useful to have
      * a list of all active ScopeNodes for purposes of visualization of the SoN graph
      */
-    public final Stack<ScopeNode> _allScopes = new Stack<>();
+    public final Stack<ScopeNode> _xScopes = new Stack<>();
 
     public Parser(String source, TypeInteger arg) {
-        _lexer = new Lexer(source);
         Node.reset();
+        _lexer = new Lexer(source);
         _scope = new ScopeNode();
         START = new StartNode(new Type[]{ Type.CONTROL, arg });
-        START.peephole();
         STOP = new StopNode();
     }
 
@@ -80,29 +79,30 @@ public class Parser {
   
     String src() { return new String( _lexer._input ); }
 
+    // Debugging utility to find a Node by index
+    public static Node find(int nid) { return START.find(nid); }
+    
     private Node ctrl() { return _scope.ctrl(); }
 
     private Node ctrl(Node n) { return _scope.ctrl(n); }
 
     public StopNode parse() { return parse(false); }
     public StopNode parse(boolean show) {
-        _allScopes.push(_scope);
+	    _xScopes.push(_scope);
+        // Enter a new scope for the initial control and arguments
         _scope.push();
-        try {
-            _scope.ctrl(new ProjNode(START, 0, ScopeNode.CTRL).peephole());
-            _scope.define("arg", new ProjNode(START, 1, "arg").peephole());
-            parseBlock();
-            if (!_lexer.isEOF()) throw error("Syntax error, unexpected " + _lexer.getAnyNextToken());
-            return STOP;
-        }
-        finally {
-            _scope.pop();
-            _allScopes.pop();
-            STOP.peephole();
-            if( show ) showGraph();
-        }
+        _scope.define(ScopeNode.CTRL, new ProjNode(START, 0, ScopeNode.CTRL).peephole());
+        _scope.define(ScopeNode.ARG0, new ProjNode(START, 1, ScopeNode.ARG0).peephole());
+        parseBlock();
+        _scope.pop();
+		_xScopes.pop();
+        if (!_lexer.isEOF()) throw error("Syntax error, unexpected " + _lexer.getAnyNextToken());
+        STOP.peephole();
+        if( show ) showGraph();
+        return STOP;
     }
 
+    
     /**
      * Parses a block
      *
@@ -115,14 +115,11 @@ public class Parser {
     private Node parseBlock() {
         // Enter a new scope
         _scope.push();
-        Node n = null;
-        while (!peek('}') && !_lexer.isEOF()) {
-            Node n0 = parseStatement();
-            if (n0 != null) n = n0; // Allow null returns from eg showGraph
-        };
+        while (!peek('}') && !_lexer.isEOF())
+            parseStatement();
         // Exit scope
         _scope.pop();
-        return n;
+        return null;
     }
 
     /**
@@ -164,7 +161,7 @@ public class Parser {
         // But first clone the scope and set it as current
         int ndefs = _scope.nIns();
         ScopeNode fScope = _scope.dup(); // Duplicate current scope
-        _allScopes.push(fScope); // For graph visualization we need all scopes
+        _xScopes.push(fScope); // For graph visualization we need all scopes
 
         // Parse the true side
         ctrl(ifT);              // set ctrl token to ifTrue projection
@@ -174,14 +171,17 @@ public class Parser {
         // Parse the false side
         _scope = fScope;        // Restore scope, then parse else block if any
         ctrl(ifF);              // Ctrl token is now set to ifFalse projection
-        if (matchx("else")) parseStatement();
+        if (matchx("else")) {
+            parseStatement();
+            fScope = _scope;
+        }
 
         if( tScope.nIns() != ndefs || fScope.nIns() != ndefs )
             throw error("Cannot define a new name on one arm of an if");
         
         // Merge results
         _scope = tScope;
-        _allScopes.pop();       // Discard pushed from graph display
+        _xScopes.pop();       // Discard pushed from graph display
 
         return ctrl(tScope.mergeScopes(fScope));
     }
@@ -267,12 +267,12 @@ public class Parser {
      */
     private Node parseComparison() {
         var lhs = parseAddition();
-        if (match("==")) return new BoolNode.EQNode(lhs, parseComparison()).peephole();
-        if (match("!=")) return new NotNode(new BoolNode.EQNode(lhs, parseComparison()).peephole()).peephole();
-        if (match("<" )) return new BoolNode.LTNode(lhs, parseComparison()).peephole();
-        if (match("<=")) return new BoolNode.LENode(lhs, parseComparison()).peephole();
-        if (match(">" )) return new BoolNode.LTNode(parseComparison(), lhs).peephole();
-        if (match(">=")) return new BoolNode.LENode(parseComparison(), lhs).peephole();
+        if (match("==")) return new BoolNode.EQ(lhs, parseComparison()).peephole();
+        if (match("!=")) return new NotNode(new BoolNode.EQ(lhs, parseComparison()).peephole()).peephole();
+        if (match("<" )) return new BoolNode.LT(lhs, parseComparison()).peephole();
+        if (match("<=")) return new BoolNode.LE(lhs, parseComparison()).peephole();
+        if (match(">" )) return new BoolNode.LT(parseComparison(), lhs).peephole();
+        if (match(">=")) return new BoolNode.LE(parseComparison(), lhs).peephole();
         return lhs;
     }
 
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/Utils.java b/chapter06/src/main/java/com/seaofnodes/simple/Utils.java
index c79d96f0..4d663c25 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/Utils.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/Utils.java
@@ -1,6 +1,39 @@
 package com.seaofnodes.simple;
 
+import java.util.ArrayList;
+
 public class Utils {
     public static RuntimeException TODO() { return TODO("Not yet implemented"); }
     public static RuntimeException TODO(String msg) { return new RuntimeException(msg); }
+
+    /**
+     *  Fast, constant-time, element removal.  Does not preserve order
+     *
+     *  @param array ArrayList to modify
+     *  @param i element to be removed
+     *  @return element removed
+     */
+    public static <E> E del(ArrayList<E> array, int i) {
+        if ( i >= 0 && i < array.size() ) {
+            E tmp = array.get(i);
+            E last = array.removeLast();
+            if (i < array.size()) array.set(i, last);
+            return tmp;
+        }
+        return null;
+    }
+
+    /**
+     * Search a list for an element by reference
+     *
+     * @param ary List to search in
+     * @param x Object to be searched
+     * @return >= 0 on success, -1 on failure
+     */
+    public static <E> int find( ArrayList<E> ary, E x ) {
+        for( int i=0; i<ary.size(); i++ )
+            if( ary.get(i)==x )
+                return i;
+        return -1;
+    }
 }
\ No newline at end of file
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/node/AddNode.java b/chapter06/src/main/java/com/seaofnodes/simple/node/AddNode.java
index b29fb92c..5ef9c94a 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/node/AddNode.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/node/AddNode.java
@@ -128,7 +128,7 @@ public class AddNode extends Node {
     }
 
     static PhiNode pcon(Node op) {
-        return op instanceof PhiNode phi && phi.all_cons() ? phi : null;
+        return op instanceof PhiNode phi && phi.allCons() ? phi : null;
     }
         
     // Compare two off-spline nodes and decide what order they should be in.
@@ -140,8 +140,8 @@ public class AddNode extends Node {
         if( lo._type.isConstant() ) return false;
         if( hi._type.isConstant() ) return true ;
             
-        if( lo instanceof PhiNode && lo.all_cons() ) return false;
-        if( hi instanceof PhiNode && hi.all_cons() ) return true ;
+        if( lo instanceof PhiNode && lo.allCons() ) return false;
+        if( hi instanceof PhiNode && hi.allCons() ) return true ;
 
         if( lo instanceof PhiNode && !(hi instanceof PhiNode) ) return true;
         if( hi instanceof PhiNode && !(lo instanceof PhiNode) ) return false;
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/node/BoolNode.java b/chapter06/src/main/java/com/seaofnodes/simple/node/BoolNode.java
index bbcc742d..4eeef873 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/node/BoolNode.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/node/BoolNode.java
@@ -52,7 +52,7 @@ abstract public class BoolNode extends Node {
         return null;
     }
 
-    public static class EQNode extends BoolNode { public EQNode(Node lhs, Node rhs) { super(lhs,rhs); } String op() { return "=="; } boolean doOp(long lhs, long rhs) { return lhs == rhs; } Node copy(Node lhs, Node rhs) { return new EQNode(lhs,rhs); } }
-    public static class LTNode extends BoolNode { public LTNode(Node lhs, Node rhs) { super(lhs,rhs); } String op() { return "<" ; } boolean doOp(long lhs, long rhs) { return lhs <  rhs; } Node copy(Node lhs, Node rhs) { return new LTNode(lhs,rhs); } }
-    public static class LENode extends BoolNode { public LENode(Node lhs, Node rhs) { super(lhs,rhs); } String op() { return "<="; } boolean doOp(long lhs, long rhs) { return lhs <= rhs; } Node copy(Node lhs, Node rhs) { return new LENode(lhs,rhs); } }
+    public static class EQ extends BoolNode { public EQ(Node lhs, Node rhs) { super(lhs,rhs); } String op() { return "=="; } boolean doOp(long lhs, long rhs) { return lhs == rhs; } Node copy(Node lhs, Node rhs) { return new EQ(lhs,rhs); } }
+    public static class LT extends BoolNode { public LT(Node lhs, Node rhs) { super(lhs,rhs); } String op() { return "<" ; } boolean doOp(long lhs, long rhs) { return lhs <  rhs; } Node copy(Node lhs, Node rhs) { return new LT(lhs,rhs); } }
+    public static class LE extends BoolNode { public LE(Node lhs, Node rhs) { super(lhs,rhs); } String op() { return "<="; } boolean doOp(long lhs, long rhs) { return lhs <= rhs; } Node copy(Node lhs, Node rhs) { return new LE(lhs,rhs); } }
 }
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/node/ConstantNode.java b/chapter06/src/main/java/com/seaofnodes/simple/node/ConstantNode.java
index bc92b540..ba6f55d7 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/node/ConstantNode.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/node/ConstantNode.java
@@ -23,7 +23,7 @@ public class ConstantNode extends Node {
     }
     
     @Override
-    public String label() { return "#"+_con; }
+    public String label() { return ""+_con; }
   
     @Override
     public String uniqueName() { return "Con_" + _nid; }
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/node/Node.java b/chapter06/src/main/java/com/seaofnodes/simple/node/Node.java
index 36e4896d..f5e58284 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/node/Node.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/node/Node.java
@@ -52,7 +52,7 @@ public abstract class Node {
      * parsing where we do not have the whole program, and also peepholes
      * change the CFG incrementally.
      * <p>
-     * See {@link https://en.wikipedia.org/wiki/Dominator_(graph_theory)}
+     * See {@link <a href="https://en.wikipedia.org/wiki/Dominator_(graph_theory)">...</a>}
      */
     int _idepth;
     
@@ -81,7 +81,15 @@ public abstract class Node {
 
     // Graphical label, e.g. "+" or "Region" or "=="
     public String glabel() { return label(); }
+
+
+    // ------------------------------------------------------------------------
+    
+    // Debugger Printing.
     
+    // {@code toString} is what you get in the debugger.  It has to print 1
+    // line (because this is what a debugger typically displays by default) and
+    // has to be robust with broken graph/nodes.
     @Override
     public final String toString() {
         // TODO: This needs a lot of work
@@ -115,6 +123,8 @@ public abstract class Node {
 
     public int nIns() { return _inputs.size(); }
 
+    public Node out(int i) { return _outputs.get(i); }
+
     public int nOuts() { return _outputs.size(); }
 
     public boolean isUnused() { return nOuts() == 0; }
@@ -122,67 +132,6 @@ public abstract class Node {
     public boolean isCFG() { return false; }
   
     /**
-     * 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 if
-     * possible.  We compute a {@link Type} and then check and replace:
-     * <ul>
-     * <li>if the Type {@link Type#isConstant}, we replace with a {@link ConstantNode}</li>
-     * <li>in a future chapter we will look for a
-     * <a href="https://en.wikipedia.org/wiki/Common_subexpression_elimination">Common Subexpression</a>
-     * to eliminate.</li>
-     * <li>we ask the Node for a better replacement.  The "better replacement"
-     * is things like {@code (1+2)} becomes {@code 3} and {@code (1+(x+2))} becomes
-     * {@code (x+(1+2))}.  By canonicalizing expressions we fold common addressing
-     * math constants, remove algebraic identities and generally simplify the
-     * code. </li>
-     * </ul>
-     */
-    public final Node peephole( ) {
-        // Compute initial or improved Type
-        Type type = _type = compute();
-        
-        if (_disablePeephole)
-            return this;        // Peephole optimizations turned off
-
-        // Replace constant computations from non-constants with a constant node
-        if (!(this instanceof ConstantNode) && type.isConstant())
-            return deadCodeElim(new ConstantNode(type).peephole());
-
-        // Future chapter: Global Value Numbering goes here
-        
-        // Ask each node for a better replacement
-        Node n = idealize();
-        if( n != null )         // Something changed
-            // Recursively optimize
-            return deadCodeElim(n.peephole());
-        
-        return this;            // No progress
-    }
-
-    // m is the new Node, self is the old.
-    // Return 'm', which may have zero uses but is alive nonetheless.
-    // If self has zero uses (and is not 'm'), {@link #kill} self.
-    private Node deadCodeElim(Node m) {
-        // If self is going dead and not being returned here (Nodes returned
-        // from peephole commonly have no uses (yet)), then kill self.
-        if( m != this && isUnused() ) {
-            // Killing self - and since self recursively kills self's inputs we
-            // might end up killing 'm', which we are returning as a live Node.
-            // So we add a bogus extra null output edge to stop kill().
-            m.addUse(null); // Add bogus null use to keep m alive
-            kill();            // Kill self because replacing with 'm'
-            m.delUse(null);    // Remove bogus null.
-        }
-        return m;
-    }
-
-    
-    /**
      * Change a <em>def</em> into a Node.  Keeps the edges correct, by removing
      * the corresponding <em>use->def</em> edge.  This may make the original
      * <em>def</em> go dead.  This function is co-recursive with {@link #kill}.
@@ -196,9 +145,9 @@ public abstract class Node {
      *     
      * @param idx which def to set
      * @param new_def the new definition
-     * @return this for flow coding
+     * @return new_def for flow coding
      */
-    Node set_def(int idx, Node new_def ) {
+    Node setDef(int idx, Node new_def ) {
         Node old_def = in(idx);
         if( old_def == new_def ) return this; // No change
         // If new def is not null, add the corresponding def->use edge
@@ -212,7 +161,7 @@ public abstract class Node {
         // Set the new_def over the old (killed) edge
         _inputs.set(idx,new_def);
         // Return self for easy flow-coding
-        return this;
+        return new_def;
     }
   
     // Remove the numbered input, compressing the inputs in-place.  This
@@ -223,10 +172,7 @@ public abstract class Node {
         if( old_def != null &&  // If the old def exists, remove a def->use edge
             old_def.delUse(this) ) // If we removed the last use, the old def is now dead
             old_def.kill();     // Kill old def
-        // Set the new_def over the old (killed) edge
-        int last = nIns()-1;
-        _inputs.set(idx,in(last));
-        _inputs.remove(last);
+        Utils.del(_inputs, idx);
     }
 
     /**
@@ -236,7 +182,7 @@ public abstract class Node {
      * @param new_def the new definition, appended to the end of existing definitions
      * @return new_def for flow coding
      */
-    Node add_def(Node new_def) {
+    Node addDef(Node new_def) {
         // Add use->def edge
         _inputs.add(new_def);
         // If new def is not null, add the corresponding def->use edge
@@ -247,29 +193,20 @@ public abstract class Node {
 
     // Breaks the edge invariants, used temporarily
     protected <N extends Node> N addUse(Node n) { _outputs.add(n); return (N)this; }
-    // Shortcuts to stop DCE mid-parse
-    public <N extends Node> N keep() { return addUse(null); }
-    public Node unkeep() { delUse(null); return this; }
 
     // Remove node 'use' from 'def's (i.e. our) output list, by compressing the list in-place.
     // Return true if the output list is empty afterward.
     // Error is 'use' does not exist; ok for 'use' to be null.
     protected boolean delUse( Node use ) {
-        ArrayList<Node> outs = _outputs;
-        int lidx = outs.size()-1; // Last index            
-        // This 1-line hack compresses an element out of an ArrayList
-        // without having to copy the contents.  The last element is
-        // stuffed over the deleted element, and then the size is reduced.            
-        outs.set(outs.indexOf(use),outs.get(lidx));
-        outs.remove(lidx);  // Reduce ArrayList size without copying anything
-        return lidx==0;
+        Utils.del(_outputs, Utils.find(_outputs, use));
+        return _outputs.size() == 0;
     }
 
     // Shortcut for "popping" n nodes.  A "pop" is basically a
     // set_def(last,null) followed by lowering the nIns() count.
-    void pop_n(int n) {
+    void popN(int n) {
         for( int i=0; i<n; i++ ) {
-            Node old_def = _inputs.remove(_inputs.size()-1);
+            Node old_def = _inputs.removeLast();
             if( old_def != null &&     // If it exists and
                 old_def.delUse(this) ) // If we removed the last use, the old def is now dead
                 old_def.kill();        // Kill old def
@@ -279,17 +216,85 @@ public abstract class Node {
     /**
      * Kill a Node with no <em>uses</em>, by setting all of its <em>defs</em>
      * to null.  This may recursively kill more Nodes and is basically dead
-     * code elimination.  This function is co-recursive with {@link #pop_n}.
+     * code elimination.  This function is co-recursive with {@link #popN}.
      */
     public void kill( ) {
         assert isUnused();      // Has no uses, so it is dead
-        pop_n(nIns());          // Set all inputs to null, recursively killing unused Nodes
+        popN(nIns());          // Set all inputs to null, recursively killing unused Nodes
         _type=null;             // Flag as dead
         assert isDead();        // Really dead now
     }
 
     // Mostly used for asserts and printing.
     boolean isDead() { return isUnused() && nIns()==0 && _type==null; }
+
+    // Shortcuts to stop DCE mid-parse
+    // Add bogus null use to keep node alive
+    public <N extends Node> N keep() { return addUse(null); }
+    // Remove bogus null.
+    public <N extends Node> N unkeep() { delUse(null); return (N)this; }
+    // ------------------------------------------------------------------------
+    // 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 if
+     * possible.  We compute a {@link Type} and then check and replace:
+     * <ul>
+     * <li>if the Type {@link Type#isConstant}, we replace with a {@link ConstantNode}</li>
+     * <li>in a future chapter we will look for a
+     * <a href="https://en.wikipedia.org/wiki/Common_subexpression_elimination">Common Subexpression</a>
+     * to eliminate.</li>
+     * <li>we ask the Node for a better replacement.  The "better replacement"
+     * is things like {@code (1+2)} becomes {@code 3} and {@code (1+(x+2))} becomes
+     * {@code (x+(1+2))}.  By canonicalizing expressions we fold common addressing
+     * math constants, remove algebraic identities and generally simplify the
+     * code. </li>
+     * </ul>
+     */
+    public final Node peephole( ) {
+        // Compute initial or improved Type
+        Type type = _type = compute();
+        
+        if (_disablePeephole)
+            return this;        // Peephole optimizations turned off
+
+        // Replace constant computations from non-constants with a constant node
+        if (!(this instanceof ConstantNode) && type.isConstant())
+            return deadCodeElim(new ConstantNode(type).peephole());
+
+        // Future chapter: Global Value Numbering goes here
+        
+        // Ask each node for a better replacement
+        Node n = idealize();
+        if( n != null )         // Something changed
+            // Recursively optimize
+            return deadCodeElim(n.peephole());
+        
+        return this;            // No progress
+    }
+
+    // m is the new Node, self is the old.
+    // Return 'm', which may have zero uses but is alive nonetheless.
+    // If self has zero uses (and is not 'm'), {@link #kill} self.
+    private Node deadCodeElim(Node m) {
+        // If self is going dead and not being returned here (Nodes returned
+        // from peephole commonly have no uses (yet)), then kill self.
+        if( m != this && isUnused() ) {
+            // Killing self - and since self recursively kills self's inputs we
+            // might end up killing 'm', which we are returning as a live Node.
+            // So we add a bogus extra null output edge to stop kill().
+            m.keep();      // Keep m alive
+            kill();        // Kill self because replacing with 'm'
+            m.unkeep();    // Okay to peephole m
+        }
+        return m;
+    }
   
     /**
      * This function needs to be
@@ -357,7 +362,7 @@ public abstract class Node {
     public abstract Node idealize();
 
 
-    // -----------------------
+    // ------------------------------------------------------------------------
     // Peephole utilities
     
     // Swap inputs without letting either input go dead during the swap.
@@ -370,7 +375,7 @@ public abstract class Node {
     
     // does this node contain all constants?
     // Ignores in(0), as is usually control.
-    boolean all_cons() {
+    boolean allCons() {
         for( int i=1; i<nIns(); i++ )
             if( !(in(i)._type.isConstant()) )
                 return false;
@@ -394,5 +399,25 @@ public abstract class Node {
      * Used to allow repeating tests in the same JVM.  This just resets the
      * Node unique id generator, and is done as part of making a new Parser.
      */
-    public static void reset() { UNIQUE_ID = 1; }
+    public static void reset() { UNIQUE_ID = 1; _disablePeephole=false; }
+
+    /**
+     * Debugging utility to find a Node by index
+     */
+    public Node find(int nid) { return _find(new BitSet(),nid); }
+    private Node _find(BitSet visit, int nid) {
+        if( _nid==nid ) return this;
+        if( visit.get(_nid) ) return null;
+        visit.set(_nid);
+        for( Node def : _inputs )
+            if( def!=null ) {
+                Node rez = def._find(visit,nid);
+                if( rez != null ) return rez;
+            }
+        for( Node use : _outputs ) {
+            Node rez = use._find(visit,nid);
+            if( rez != null ) return rez;
+        }
+        return null;
+    }
 }
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/node/PhiNode.java b/chapter06/src/main/java/com/seaofnodes/simple/node/PhiNode.java
index a5750922..d435ea01 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/node/PhiNode.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/node/PhiNode.java
@@ -34,13 +34,8 @@ public class PhiNode extends Node {
 
     @Override
     public Node idealize() {
-        // Remove a "junk" Phi: Phi(x,x) is just x
-        if( same_inputs() )
-            return in(1);
-
-        // If only 1 of our input values have live control
-        // then return that as phi is dead
-        Node live = singleLiveInput();
+        // If we have only a single unique input, become it.
+        Node live = singleUniqueInput();
         if (live != null)
             return live;
 
@@ -72,25 +67,15 @@ public class PhiNode extends Node {
         return true;
     }
 
-    private boolean same_inputs() {
-        for( int i=2; i<nIns(); i++ )
-            if( in(1) != in(i) )
-                return false;
-        return true;
-    }
-
     /**
-     * If only 1 of the inputs is live then return it
+     * If only single unique input, return it
      */
-    private Node singleLiveInput() {
+    private Node singleUniqueInput() {
         Node live = null;
-        for( int i=1; i<nIns(); i++ )
-            // The control type is in the region
-            if( region().in(i)._type != Type.XCONTROL )
-                if (live == null)
-                    live = in(i);
-                else
-                    return null;
+        for( int i=1; i<nIns(); i++ ) 
+            if( region().in(i)._type != Type.XCONTROL && in(i) != this )
+                if( live == null || live == in(i) ) live = in(i);
+                else return null;
         return live;
     }
 
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/node/ProjNode.java b/chapter06/src/main/java/com/seaofnodes/simple/node/ProjNode.java
index 424a1e9b..ef957507 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/node/ProjNode.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/node/ProjNode.java
@@ -30,10 +30,6 @@ public class ProjNode extends Node {
     @Override
     public Type compute() {
         Type t = ctrl()._type;
-        return getType(t);
-    }
-
-    private Type getType(Type t) {
         return t instanceof TypeTuple tt ? tt._types[_idx] : Type.BOTTOM;
     }
 
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/node/RegionNode.java b/chapter06/src/main/java/com/seaofnodes/simple/node/RegionNode.java
index d3fd7745..104735d5 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/node/RegionNode.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/node/RegionNode.java
@@ -11,7 +11,7 @@ public class RegionNode extends Node {
 
     @Override
     StringBuilder _print1(StringBuilder sb) {
-        return sb.append("Region").append(_nid);
+        return sb.append(label()).append(_nid);
     }
 
     @Override public boolean isCFG() { return true; }
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter06/src/main/java/com/seaofnodes/simple/node/ScopeNode.java
index 96564f3b..76fe98b9 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/node/ScopeNode.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/node/ScopeNode.java
@@ -5,7 +5,7 @@ import com.seaofnodes.simple.type.Type;
 import java.util.*;
 
 /**
- * The Scope node is a purely parser helper - it tracks names to nodes with a
+ * The Scope node is purely a parser helper - it tracks names to nodes with a
  * stack of scopes.
  */
 public class ScopeNode extends Node {
@@ -15,16 +15,20 @@ public class ScopeNode extends Node {
      * node in the graph
      */
     public static final String CTRL = "$ctrl";
+    public static final String ARG0 = "arg";
 
     /**
-     * Stack of lexical scopes, each scope is a symbol table
-     * that binds variable names to Nodes.
-     * The top of this stack represents current scope.
+     * Names for every input edge
      */
-    public final Stack<HashMap<String, Integer>> _scopes = new Stack<>();
+    public final Stack<HashMap<String, Integer>> _scopes;
 
-    // Reverse lookup for nicer graph printing
-    private final ArrayList<String> _rlabels = new ArrayList<>();
+
+    // A new ScopeNode
+    public ScopeNode() {
+        _scopes = new Stack<>();
+        _type = Type.BOTTOM;
+    }
+    
     
     @Override public String label() { return "Scope"; }
 
@@ -46,42 +50,74 @@ public class ScopeNode extends Node {
         }
         return sb;
     }
-  
-    @Override
-    public Type compute() { return Type.BOTTOM; }
 
-    @Override public Node idealize() { return null; }
-
-    // Add an empty lexical scope
-    public HashMap<String, Integer> push() {
-        return _scopes.push(new HashMap<>());
+    /**
+     * Recover the names for all variable bindings.
+     * The result is an array of names that is aligned with the
+     * inputs to the Node.
+     *
+     * This is an expensive operation.
+     */
+    public String[] reverseNames() {
+        String[] names = new String[nIns()];
+        for( HashMap<String,Integer> syms : _scopes )
+            for( String name : syms.keySet() )
+                names[syms.get(name)] = name;
+        return names;
     }
+    
+    @Override public Type compute() { return Type.BOTTOM; }
 
-    // Remove the current lexical scope, killing all unused nodes.
-    public void pop() {
-        HashMap<String,Integer> scope = _scopes.pop();
-        pop_n(scope.size());
-    }
+    @Override public Node idealize() { return null; }
 
-    // Create a new name in the current scope
+    public void push() { _scopes.push(new HashMap<>());  }
+    public void pop() { popN(_scopes.pop().size());  }
+
+    /**
+     * Create a new name in the current scope
+     */
     public Node define( String name, Node n ) {
-        HashMap<String,Integer> scope = _scopes.lastElement();
-        if( scope.put(name,nIns()) != null )
+        HashMap<String,Integer> syms = _scopes.lastElement();
+        if( syms.put(name,nIns()) != null )
             return null;        // Double define
-        _rlabels.add(name);
-        return add_def(n);
+        return addDef(n);
     }
-
-    // Lookup a name in all scopes
-    public Node lookup(String name) {
-        for (int i = _scopes.size() - 1; i >= 0; i--) {
-            var idx = _scopes.get(i).get(name);
-            if (idx != null) return in(idx);
-        }
-        return null;
+    /**
+     * Lookup a name in all scopes starting from most deeply nested.
+     *
+     * @param name Name to be looked up
+     * @see #update(String, Node, int)
+     */
+    public Node lookup( String name ) { return update(name,null,_scopes.size()-1);  }
+    /**
+     * If the name is present in any scope, then redefine else null
+     *
+     * @param name Name being redefined
+     * @param n    The node to bind to the name
+     */
+    public Node update( String name, Node n ) { return update(name,n,_scopes.size()-1); }
+    /**
+     * Both recursive lookup and update.
+     *
+     * A shared implementation allows us to create lazy phis both during
+     * lookups and updates; the lazy phi creation is part of chapter 8.
+     *
+     * @param name  Name whose binding is being updated or looked up
+     * @param n     If null, do a lookup, else update binding
+     * @param nestingLevel The starting nesting level
+     * @return node being looked up, or the one that was updated
+     */
+    private Node update( String name, Node n, int nestingLevel ) {
+        if( nestingLevel<0 ) return null;  // Missed in all scopes, not found
+        var syms = _scopes.get(nestingLevel); // Get the symbol table for nesting level
+        var idx = syms.get(name);
+        if( idx == null ) return update(name,n,nestingLevel-1); // Not found in this scope, recursively look in parent scope
+        Node old = in(idx);
+        // If n is null we are looking up rather than updating, hence return existing value
+        return n==null ? old : setDef(idx,n);
     }
 
-    public Node ctrl() { assert lookup(CTRL)==in(0); return in(0); }
+    public Node ctrl() { return in(0); }
 
     /**
      * The ctrl of a ScopeNode is always bound to the currently active
@@ -93,29 +129,7 @@ public class ScopeNode extends Node {
      *
      * @return Node that was bound
      */
-    public Node ctrl(Node n) {
-        if (nIns() == 0) {
-            // The first time a scope is created we do not have the name binding
-            // for '$ctrl' but after that we expect to find the name already bound
-            assert lookup(CTRL) == null;
-            define(CTRL, n);
-        }
-        else assert lookup(CTRL) == in(0);
-        set_def(0,n);
-        return n;
-    }
-
-  
-    // If the name is present in any scope, then redefine
-    public Node update(String name, Node n) {
-        for (int i = _scopes.size() - 1; i >= 0; i--) {
-            HashMap<String, Integer> scope = _scopes.get(i);
-            Integer idx = scope.get(name);
-            if (idx != null)           // Found prior def
-                return set_def(idx,n); // Update def in scope
-        }
-        return null;
-    }
+    public Node ctrl(Node n) { return setDef(0,n); }
 
     /**
      * Duplicate a ScopeNode; including all levels, up to Nodes.  So this is
@@ -126,19 +140,19 @@ public class ScopeNode extends Node {
      * The new Scope is a full-fledged Node with proper use<->def edges.
      */
     public ScopeNode dup() {
+        ScopeNode dup = new ScopeNode();
         // Our goals are:
         // 1) duplicate the name bindings of the ScopeNode across all stack levels
         // 2) Make the new ScopeNode a user of all the nodes bound
         // 3) Ensure that the order of defs is the same to allow easy merging
-        ScopeNode dup = new ScopeNode();
-        for( HashMap<String, Integer> tab : _scopes )
-            dup._scopes.push(new HashMap<>(tab));
-        for( int i=0; i<nIns(); i++ )
-            dup.add_def(in(i));
-        dup._rlabels.addAll(_rlabels);
+        for( HashMap<String,Integer> syms : _scopes )
+            dup._scopes.push(new HashMap<>(syms));
+        dup.addDef(ctrl());      // Control input is just copied
+        for( int i=1; i<nIns(); i++ )
+            dup.addDef(in(i));
         return dup;
     }
-
+    
     /**
      * Merges the names whose node bindings differ, by creating Phi node for such names
      * The names could occur at all stack levels, but a given name can only differ in the
@@ -149,10 +163,11 @@ public class ScopeNode extends Node {
      */
     public Node mergeScopes(ScopeNode that) {
         RegionNode r = (RegionNode) ctrl(new RegionNode(null,ctrl(), that.ctrl()).keep());
+        String[] ns = reverseNames();
         // Note that we skip i==0, which is bound to '$ctrl'
         for (int i = 1; i < nIns(); i++)
-            if (in(i) != that.in(i)) // No need for redundant Phis
-                set_def(i, new PhiNode(_rlabels.get(i), r, in(i), that.in(i)).peephole());
+            if( in(i) != that.in(i) ) // No need for redundant Phis
+                setDef(i, new PhiNode(ns[i], r, in(i), that.in(i)).peephole());
         that.kill();            // Kill merged scope
         return r.unkeep().peephole();
     }
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/node/StartNode.java b/chapter06/src/main/java/com/seaofnodes/simple/node/StartNode.java
index 8cff25bd..fd0f6c18 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/node/StartNode.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/node/StartNode.java
@@ -17,6 +17,7 @@ public class StartNode extends MultiNode {
     public StartNode(Type[] args) {
         super();
         _args = new TypeTuple(args);
+        _type = _args;
     }
 
     @Override
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/node/StopNode.java b/chapter06/src/main/java/com/seaofnodes/simple/node/StopNode.java
index f17c7ec7..c8056fbe 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/node/StopNode.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/node/StopNode.java
@@ -45,7 +45,7 @@ public class StopNode extends Node {
     }
 
     public Node addReturn(Node node) {
-        return add_def(node);
+        return addDef(node);
     }
 
 }
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/type/Type.java b/chapter06/src/main/java/com/seaofnodes/simple/type/Type.java
index 78908375..f88403a5 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/type/Type.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/type/Type.java
@@ -33,7 +33,7 @@ public class Type {
     public final byte _type;
 
     public boolean is_simple() { return _type < TSIMPLE; }
-    private static final String[] STRS = new String[]{"BOTTOM","TOP","CONTROL","~CONTROL"};
+    private static final String[] STRS = new String[]{"Bot","Top","Ctrl","~Ctrl"};
     protected Type(byte type) { _type = type; }
 
     public static final Type BOTTOM   = new Type( TBOT   ); // ALL
diff --git a/chapter06/src/main/java/com/seaofnodes/simple/type/TypeTuple.java b/chapter06/src/main/java/com/seaofnodes/simple/type/TypeTuple.java
index 2e6152a8..f8c3b3c8 100644
--- a/chapter06/src/main/java/com/seaofnodes/simple/type/TypeTuple.java
+++ b/chapter06/src/main/java/com/seaofnodes/simple/type/TypeTuple.java
@@ -16,7 +16,7 @@ public class TypeTuple extends Type {
 
     @Override
     public StringBuilder _print(StringBuilder sb) {
-        sb.append("[ ");
+        sb.append("[");
         for( Type t : _types )
             t._print(sb).append(",");
         sb.setLength(sb.length()-1);