From c2169787d92da57aa0e11cd026e398b3d5fcda12 Mon Sep 17 00:00:00 2001 From: Cliff Click Date: Tue, 7 Jan 2025 11:15:10 -0800 Subject: Fix a series of small bugs Various kinds of missing checks, over-aggressive asserts, and a few odds and ends --- .../main/java/com/seaofnodes/simple/Parser.java | 11 ++++-- .../com/seaofnodes/simple/node/CallEndNode.java | 30 +++++++++------ .../java/com/seaofnodes/simple/node/CallNode.java | 14 +++++-- .../java/com/seaofnodes/simple/node/CastNode.java | 2 - .../java/com/seaofnodes/simple/node/LoadNode.java | 3 +- .../com/seaofnodes/simple/type/TypeFunPtr.java | 6 +-- .../java/com/seaofnodes/simple/Chapter17Test.java | 15 ++++++++ .../java/com/seaofnodes/simple/Chapter18Test.java | 43 ++++++++++++++++++---- .../src/test/java/com/seaofnodes/simple/Eval2.java | 3 +- 9 files changed, 92 insertions(+), 35 deletions(-) (limited to 'chapter18/src') diff --git a/chapter18/src/main/java/com/seaofnodes/simple/Parser.java b/chapter18/src/main/java/com/seaofnodes/simple/Parser.java index f1cbf446..2d98287d 100644 --- a/chapter18/src/main/java/com/seaofnodes/simple/Parser.java +++ b/chapter18/src/main/java/com/seaofnodes/simple/Parser.java @@ -217,7 +217,6 @@ public class Parser { // All args, "as-if" called externally for( int i=0; i= _scope._lexSize.last()) ) throw error("Cannot reassign final '"+name+"'"); // Lift expression, based on type @@ -1281,8 +1282,10 @@ public class Parser { if( matchx("++") || matchx("--") ) { if( f._final && !f._fname.equals("[]") ) throw error("Cannot reassign final '"+f._fname+"'"); - Node inc = peep(new AddNode(load,con( _lexer.peek(-1)=='+' ? 1 : -1))); - Node val = zsMask(inc,tf); + int delta = _lexer.peek(-1)=='+' ? 1 : -1; // Pre vs post + Node val = f._type instanceof TypeFloat + ? peep(new AddFNode(load,con(TypeFloat.constant(delta)))) + : zsMask(peep(new AddNode(load,con(delta))),f._type); Node st = new StoreNode(loc(), name, f._alias, tf, memAlias(f._alias), expr.unkeep(), off, val, false); // Arrays include control, as a proxy for a safety range check. // Structs don't need this; they only need a NPE check which is diff --git a/chapter18/src/main/java/com/seaofnodes/simple/node/CallEndNode.java b/chapter18/src/main/java/com/seaofnodes/simple/node/CallEndNode.java index 578d7f87..c9ea2f1f 100644 --- a/chapter18/src/main/java/com/seaofnodes/simple/node/CallEndNode.java +++ b/chapter18/src/main/java/com/seaofnodes/simple/node/CallEndNode.java @@ -48,21 +48,27 @@ public class CallEndNode extends CFGNode implements MultiNode { if( !_folding && nIns()==2 ) { CallNode call = call(); Node fptr = call.fptr(); - if( fptr.nOuts() == 1 ) { // Only user is this call + if( fptr.nOuts() == 1 && // Only user is this call + // Function is being called, and its not-null + fptr._type instanceof TypeFunPtr tfp && tfp.notNull()) { ReturnNode ret = (ReturnNode)in(1); FunNode fun = ret.fun(); - assert fun.nIns()==3; // Expecting Start, and the Call - assert fun.in(1) instanceof StartNode && fun.in(2)==call; + // Expecting Start, and the Call + if( fun.nIns()==3 ) { + assert fun.in(1) instanceof StartNode && fun.in(2)==call; - // Trivial inline: rewrite - _folding = true; - // Rewrite Fun so the normal RegionNode ideal collapses - fun._folding = true; - fun.setDef(1,Parser.XCTRL); // No default/unknown StartNode caller - fun.setDef(2,call.ctrl()); // Bypass the Call; - fun.ret().setDef(3,null); // Return is folding also - IterPeeps.addAll(fun._outputs); - return this; + // Trivial inline: rewrite + _folding = true; + // Rewrite Fun so the normal RegionNode ideal collapses + fun._folding = true; + fun.setDef(1,Parser.XCTRL); // No default/unknown StartNode caller + fun.setDef(2,call.ctrl()); // Bypass the Call; + fun.ret().setDef(3,null); // Return is folding also + IterPeeps.addAll(fun._outputs); + return this; + } else { + fun.addDep(this); + } } else { // Function ptr has multiple users (so maybe multiple call sites) fptr.addDep(this); } diff --git a/chapter18/src/main/java/com/seaofnodes/simple/node/CallNode.java b/chapter18/src/main/java/com/seaofnodes/simple/node/CallNode.java index 6963ff2b..246398ee 100644 --- a/chapter18/src/main/java/com/seaofnodes/simple/node/CallNode.java +++ b/chapter18/src/main/java/com/seaofnodes/simple/node/CallNode.java @@ -1,6 +1,7 @@ package com.seaofnodes.simple.node; import com.seaofnodes.simple.CodeGen; +import com.seaofnodes.simple.IterPeeps; import com.seaofnodes.simple.Parser; import com.seaofnodes.simple.Utils; import com.seaofnodes.simple.type.Type; @@ -37,9 +38,11 @@ public class CallNode extends CFGNode { Node ctrl() { return in(0); } Node mem () { return in(1); } - // Args mapped 1-to-1 on inputs + // Args mapped 1-to-1 on inputs, so conceptually start at 2 Node arg(int idx) { return in(idx); } - // args from input 2 to last + // Same arg accounting as TFPs, although the numbering starts at 2 + int nargs() { return nIns()-3; } // Minus control, memory, fptr + // args from input 2 to last; last is function input public Node fptr() { return _inputs.last(); } // Find the Call End from the Call @@ -79,7 +82,7 @@ public class CallNode extends CFGNode { // Link: call calls target function. Linking makes the target FunNode // point to this Call, and all his Parms point to the call arguments; // also the CallEnd points to the Return. - if( fptr()._type instanceof TypeFunPtr tfp ) { + if( fptr()._type instanceof TypeFunPtr tfp && tfp.nargs() == nargs() ) { // If fidxs is negative, then infinite unknown functions long fidxs = tfp.fidxs(); if( fidxs > 0 ) { @@ -115,7 +118,7 @@ public class CallNode extends CFGNode { if( use instanceof ParmNode parm ) parm.addDef(parm._idx==0 ? cend() : arg(parm._idx)); // Call end points to function return - cend().addDef(fun.ret()); + IterPeeps.add(cend()).addDef(fun.ret()); assert linked(fun); } @@ -125,6 +128,9 @@ public class CallNode extends CFGNode { throw Utils.TODO(); if( !tfp.notNull() ) return Parser.error( "Might be null calling "+tfp, _loc); + if( nargs() != tfp.nargs() ) + return Parser.error( "Expecting "+tfp.nargs()+" arguments, but found "+nargs(), _loc); + // Check for args for( int i=0; i FIDXS = new HashMap<>(); diff --git a/chapter18/src/test/java/com/seaofnodes/simple/Chapter17Test.java b/chapter18/src/test/java/com/seaofnodes/simple/Chapter17Test.java index 5247f122..98c30721 100644 --- a/chapter18/src/test/java/com/seaofnodes/simple/Chapter17Test.java +++ b/chapter18/src/test/java/com/seaofnodes/simple/Chapter17Test.java @@ -279,6 +279,21 @@ return new A { catch( Exception e ) { assertEquals("'A' is not fully initialized, field 'b' needs to be set in a constructor",e.getMessage()); } } + @Test + public void testVar16() { + CodeGen code = new CodeGen( +""" +struct S{}; +val x = 1; +val s = new S{x = 2;}; +return x; +""" +); + try { code.parse().opto().typeCheck(); fail(); } + catch( Exception e ) { assertEquals("Cannot reassign final 'x'",e.getMessage()); } + } + + // --------------------------------------------------------------- @Test public void testTrinary0() { diff --git a/chapter18/src/test/java/com/seaofnodes/simple/Chapter18Test.java b/chapter18/src/test/java/com/seaofnodes/simple/Chapter18Test.java index 545bb546..8f8d15a1 100644 --- a/chapter18/src/test/java/com/seaofnodes/simple/Chapter18Test.java +++ b/chapter18/src/test/java/com/seaofnodes/simple/Chapter18Test.java @@ -11,13 +11,14 @@ public class Chapter18Test { public void testJig() { CodeGen code = new CodeGen( """ -struct S { int i; }; -val newS = { int x -> return new S { i=x; }; }; -return newS(1).i; +if( arg ? f : g ) return 1; +val f = {->1}; +val g = {->2}; +return 2; """); - code.parse(false).opto().typeCheck().GCM(); - assertEquals("return 0;", code._stop.toString()); - assertEquals("0", Eval2.eval(code, 0)); + code.parse().opto().typeCheck().GCM(); + assertEquals("return 0.0f;", code._stop.toString()); + assertEquals("0.0", Eval2.eval(code, 0)); } @Test @@ -143,8 +144,8 @@ struct S { int i; }; val newS = { int x -> return new S { i=x; }; }; return newS(1).i; """); - code.parse(false).opto().typeCheck().GCM(); - assertEquals("Stop[ return .i; return S; ]", code._stop.toString()); + code.parse().opto().typeCheck().GCM(); + assertEquals("return 1;", code._stop.toString()); assertEquals("1", Eval2.eval(code, 0)); } @@ -166,6 +167,32 @@ return 1; catch( Exception e ) { assertEquals("No active loop for a break or continue",e.getMessage()); } } + // Calling and inlining a null function + @Test + public void testErr2() { + CodeGen code = new CodeGen( +""" +{int -> int}? i2i = { int i -> return i; }; +for(;;) { + if (i2i(2) == arg) break; + i2i = null; +} +"""); + try { code.parse().opto().typeCheck(); fail(); } + catch( Exception e ) { assertEquals("Might be null calling { int -> int #1}?",e.getMessage()); } + } + + + @Test + public void testErr3() { + CodeGen code = new CodeGen( +""" +val f = { int i, int j -> return i+j; }; +return f(); +"""); + try { code.parse().opto().typeCheck(); fail(); } + catch( Exception e ) { assertEquals("Expecting 2 arguments, but found 0",e.getMessage()); } + } // Mutual recursion. Fails without SCCP to lift the recursive return types. @Ignore @Test diff --git a/chapter18/src/test/java/com/seaofnodes/simple/Eval2.java b/chapter18/src/test/java/com/seaofnodes/simple/Eval2.java index 66b5c28b..e68c8fd3 100644 --- a/chapter18/src/test/java/com/seaofnodes/simple/Eval2.java +++ b/chapter18/src/test/java/com/seaofnodes/simple/Eval2.java @@ -263,6 +263,7 @@ public abstract class Eval2 { case LoadNode ld -> load(ld); case MinusFNode mnf -> - d(mnf.in(1)); case MinusNode sub -> - x(sub.in(1)); + case MulFNode mlf -> d(mlf.in(1)) * d(mlf.in(2)); case MulNode mul -> x(mul.in(1)) * x(mul.in(2)); case NewNode alloc-> alloc(alloc); case NotNode not -> x(not.in(1)) == 0 ? 1L : 0L; @@ -297,7 +298,7 @@ public abstract class Eval2 { return switch ( t ) { case TypeInteger i -> i.isConstant() ? (Long )i.value() : "INT"; case TypeFloat f -> f.isConstant() ? (Double)f.value() : "FLT"; - case TypeMemPtr tmp -> throw Utils.TODO(); + case TypeMemPtr tmp -> tmp; case TypeFunPtr tfp -> tfp; case TypeMem mem -> "MEM"; default -> null; -- cgit v1.2.3