From 6813c5ca6c3d87dc78995bd44668593c3b9e0883 Mon Sep 17 00:00:00 2001 From: Cliff Click Date: Thu, 10 Sep 2026 19:33:38 -0700 Subject: More code coverage cleanup --- .../main/java/com/seaofnodes/simple/IterPeeps.java | 45 ++++++------- .../simple/codegen/GlobalCodeMotion.java | 76 ++++++---------------- .../java/com/seaofnodes/simple/codegen/Opto.java | 35 +++++----- .../java/com/seaofnodes/simple/node/LoadNode.java | 5 +- 4 files changed, 62 insertions(+), 99 deletions(-) (limited to 'chapter25/src/main/java/com') 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 list, boolean dir ) { + public static boolean progressOnList(CodeGen code, WorkList 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 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; iearly, 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 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 { - Type dst = convert.dst(); - evidence = dst instanceof TypeFloat ? 2 : - dst instanceof TypeInteger ? 1 : 0; - } default -> { return 0; } } for( int i=1; i