From e2b042bd9721995486f4f0cf96847c02d27908e3 Mon Sep 17 00:00:00 2001 From: Cliff Click Date: Sat, 24 Oct 2026 15:26:23 -0700 Subject: Fix chained comparisons and restore recursion coverage --- .../main/java/com/seaofnodes/simple/Parser.java | 108 ++++++++++++--------- .../seaofnodes/simple/ChainedComparisonTest.java | 37 +++++++ .../java/com/seaofnodes/simple/Chapter09Test.java | 2 +- .../java/com/seaofnodes/simple/Chapter18Test.java | 5 +- .../java/com/seaofnodes/simple/Chapter23Test.java | 24 ++--- .../java/com/seaofnodes/simple/Chapter24Test.java | 12 +-- .../java/com/seaofnodes/simple/MergeSortTest.java | 10 +- 7 files changed, 125 insertions(+), 73 deletions(-) create mode 100644 chapter25/src/test/java/com/seaofnodes/simple/ChainedComparisonTest.java (limited to 'chapter25') diff --git a/chapter25/src/main/java/com/seaofnodes/simple/Parser.java b/chapter25/src/main/java/com/seaofnodes/simple/Parser.java index 0b106ffc..56233250 100644 --- a/chapter25/src/main/java/com/seaofnodes/simple/Parser.java +++ b/chapter25/src/main/java/com/seaofnodes/simple/Parser.java @@ -1082,57 +1082,73 @@ public class Parser { */ private Node parseComparison() { Node lhs = parseShift(); - int dir = 0; + int dir = parseCompDir(); + // No compare + if( dir==0 ) return lhs; + lhs.keep(); - while( true ) { - int dir0; - if( false ) ; - else if( match("<=" ) ) dir0 = -1; - else if( match(">=" ) ) dir0 = -2; - else if( match("<" ) ) dir0 = 1; - else if( match(">" ) ) dir0 = 2; - else break; - lhs.keep(); - // Check and record direction - if( dir==0 ) { - dir = Math.abs(dir0); - Node rhs = parseShift(); - lhs = makeCompBool(dir0,lhs.unkeep(),rhs); // Convert to a bool - } else if( dir == Math.abs(dir0) ) { - // e0 < lhs < ??? - Node ifNode = new IfNode(ctrl(), lhs).peephole(); - Node ifT = new CProjNode(ifNode. keep(), 0, "True" ).peephole(); - Node ifF = new CProjNode(ifNode.unkeep(), 1, "False").peephole(); - // False side does nothing - ScopeNode fScope = _scope.dup(); - fScope.ctrl(ifF); - // True side executes next part of range - ctrl(ifT); - _scope.addGuards(ifT,lhs,false); - Node rhs = parseShift(); - _scope.removeGuards(ifT); // TODO: I think should remain true as long as possible - Node next = makeCompBool(dir0, lhs, rhs).keep(); - // Merge result - RegionNode r = ctrl(_scope.mergeScopes(fScope, loc())); - assert next._type.meet(lhs._type).isa( TypeInteger.BOOL ); - lhs = peep(new PhiNode("",r,next.unkeep(),lhs.unkeep())); - r.peephole(); - } else { - throw error("Mixing relational directions in a chained relational test"); - } + // Compare + Node rhs = parseShift().keep(); + Node cmp = makeCompBool(dir,lhs.unkeep(),rhs).keep(); // Convert to a bool + + // Stacked compares? + int dir0 = parseCompDir(); + if( dir0 == 0 ) { + rhs.unkill(); + return cmp.unkeep(); } - return lhs; - } - private Node makeCompBool( int dir0, Node lhs, Node rhs ) { - // Convert to a bool - if( Math.abs(dir0)==2 ) // Swap direction - { Node tmp = lhs; lhs = rhs; rhs = tmp; } - lhs = peep(dir0 < 0 ? new BoolNode.LE(lhs,rhs) : new BoolNode.LT(lhs,rhs)); - return peep(lhs); + // Keep the middle operand separately from the boolean result. + // The next test compares adjacent operands, not that boolean. + Node ifNode = new IfNode(ctrl(), cmp.unkeep()).peephole(); + Node ifT = new CProjNode(ifNode. keep(), 0, "True" ).peephole().keep(); + Node ifF = new CProjNode(ifNode.unkeep(), 1, "False").peephole(); + // False side does nothing but capture memory & side-effects + ScopeNode fail = _scope.dup(); + fail.ctrl(ifF); + + // Loop over stacked compares. Each loop moves the old RHS to become + // the new LHS, so e.g. (a < b < c < d) compares (LHS:a < RHS:b) then + // compares (LHS:b < RHS:c) then (LHS:c < RHS:d), etc. + while( dir0 != 0 ) { + if( Math.abs(dir) != Math.abs(dir0) ) + throw error("Mixing relational directions in a chained relational test"); + // True side parses next arm of test + ctrl(ifT.unkeep()); + lhs = rhs; // Reuse the previous RHS exactly once + rhs = parseShift().keep(); + cmp = makeCompBool(dir0,lhs.unkeep(),rhs); // Convert to a bool + ifNode = new IfNode(ctrl(), cmp).peephole(); + ifT = new CProjNode(ifNode. keep(), 0, "True" ).peephole().keep(); + ifF = new CProjNode(ifNode.unkeep(), 1, "False").peephole(); + // Merge result into the fail case + ctrl(ifF); + fail.mergeScopes(_scope.dup(), loc()).peephole(); + // Check next stacked compare + dir0 = parseCompDir(); + } + rhs.unkill(); + ctrl(ifT.unkeep()); + RegionNode r = fail.mergeScopes(_scope, loc()).init(); + _scope = fail; + return new PhiNode("", r,con(0),con(1)).peephole(); + } + + private int parseCompDir() { + if( match("<=") ) return -1; // Negative for "equals-or-less" + if( match(">=") ) return -2; // 1 for <, 2 for > + if( match("<" ) ) return 1; // Positive for just "less" + if( match(">" ) ) return 2; + return 0; + } + + private Node makeCompBool( int dir, Node lhs, Node rhs ) { + if( Math.abs(dir)==2 ) + { Node tmp=lhs; lhs=rhs; rhs=tmp; } + Node cmp = dir < 0 ? new BoolNode.LE(lhs,rhs) : new BoolNode.LT(lhs,rhs); + return peep(cmp); } - /** * Parse an additive expression * diff --git a/chapter25/src/test/java/com/seaofnodes/simple/ChainedComparisonTest.java b/chapter25/src/test/java/com/seaofnodes/simple/ChainedComparisonTest.java new file mode 100644 index 00000000..99f18e62 --- /dev/null +++ b/chapter25/src/test/java/com/seaofnodes/simple/ChainedComparisonTest.java @@ -0,0 +1,37 @@ +package com.seaofnodes.simple; + +import com.seaofnodes.simple.codegen.CodeGen; +import com.seaofnodes.simple.type.TypeInteger; +import org.junit.Test; +import static org.junit.Assert.*; + +public class ChainedComparisonTest { + @Test public void testAdjacentOperandsAndOperators() { + String[] expressions = {"10 < arg < 20", "0 < arg <= 3", "3 >= arg > 0", + "0 <= arg < 3", "0 < arg <= 3 < 4"}; + String[][] expected = {{"0","0","1","0"}, {"0","1","0","0"}, + {"0","1","0","0"}, {"1","0","0","0"}, + {"0","1","0","0"}}; + int[] args = {0,3,15,30}; + for(int i=0;i 1;").parse(); + fail("mixed directions"); + } catch(RuntimeException e) { + assertEquals("Mixing relational directions in a chained relational test",e.getMessage()); + } + } +} diff --git a/chapter25/src/test/java/com/seaofnodes/simple/Chapter09Test.java b/chapter25/src/test/java/com/seaofnodes/simple/Chapter09Test.java index f24d96eb..50410618 100644 --- a/chapter25/src/test/java/com/seaofnodes/simple/Chapter09Test.java +++ b/chapter25/src/test/java/com/seaofnodes/simple/Chapter09Test.java @@ -255,7 +255,7 @@ return 0!=0; int v0=0; while(0==1) while(v0) v0=1+v0; -return 0; +return 0; """); code.driver(CodeGen.Phase.FinalizeIR); assertEquals("return 0;", code.print()); diff --git a/chapter25/src/test/java/com/seaofnodes/simple/Chapter18Test.java b/chapter25/src/test/java/com/seaofnodes/simple/Chapter18Test.java index 07f521ed..4927f7e2 100644 --- a/chapter25/src/test/java/com/seaofnodes/simple/Chapter18Test.java +++ b/chapter25/src/test/java/com/seaofnodes/simple/Chapter18Test.java @@ -406,8 +406,8 @@ return g(); } - // Mutual recursion. Fails without SCCP to lift the recursive return types. - @Ignore @Test + // Mutual recursion: check results, independent of how much gets inlined. + @Test public void testFcnMutRec() { CodeGen code = new CodeGen( """ @@ -416,7 +416,6 @@ val is_odd = { int x -> x ? is_even(x-1) : false; }; return is_even(arg); """); code.driver(CodeGen.Phase.FinalizeIR); - assertEquals("Stop[ return is_even( arg); return Phi(Region,Phi(Region,is_even( ((Parm_x(is_even,int,arg,Sub)-1)-1)),0),1); ]", code._stop.toString()); assertEquals("1", Eval2.eval(code, 0)); assertEquals("0", Eval2.eval(code, 1)); assertEquals("1", Eval2.eval(code, 2)); diff --git a/chapter25/src/test/java/com/seaofnodes/simple/Chapter23Test.java b/chapter25/src/test/java/com/seaofnodes/simple/Chapter23Test.java index 22be3af1..1dd21d08 100644 --- a/chapter25/src/test/java/com/seaofnodes/simple/Chapter23Test.java +++ b/chapter25/src/test/java/com/seaofnodes/simple/Chapter23Test.java @@ -115,7 +115,7 @@ var apply = { i64 x, { i64 -> u32 } fcn -> """ int a = 1; int b = 0; - + if(a++ || b++ ) { if(b == 0 && a == 2) { sys.io.p("Or"); @@ -149,7 +149,7 @@ var apply = { i64 x, { i64 -> u32 } fcn -> """ int a = 1; int b = 1; - + if(a && b) { sys.io.p("And"); } else { @@ -186,14 +186,14 @@ var apply = { i64 x, { i64 -> u32 } fcn -> String src2 = """ struct S { !S? !fld; }; - + val ptr = arg == 1 ? null : new S{fld = arg==1 ? null : new S{fld = null;};}; if( ptr && ptr.fld ) { sys.io.p("true"); } else { sys.io.p("false"); } - + return 0; """; CodeGen code = new CodeGen(src).driver(CodeGen.Phase.TypeCheck); @@ -221,13 +221,13 @@ var apply = { i64 x, { i64 -> u32 } fcn -> """ int a = 1; int b = 1; - + int x=1; int y=1; int z=0; - + int g = x++ && y++ && z++; - + if(x == 2 && y == 2 && z == 1 && g == 0) { sys.io.p("Effected"); } else { @@ -259,13 +259,13 @@ var apply = { i64 x, { i64 -> u32 } fcn -> """ int a = 1; int b = 1; - + int x = -1; int y=1; int z= -1; - + int g = x++ || y++ || z++; - + int switch = 0; if(x == 4 || y == 4 || z == 4) { switch = -1; @@ -305,11 +305,11 @@ var apply = { i64 x, { i64 -> u32 } fcn -> // -*- mode: java; -*- int a = 1; int b = 1; - + var sq = { int x -> x*x; }; - + if(a && sq(0)) { sys.io.p("And"); } else { diff --git a/chapter25/src/test/java/com/seaofnodes/simple/Chapter24Test.java b/chapter25/src/test/java/com/seaofnodes/simple/Chapter24Test.java index 3e5dc2a9..6e0fd95f 100644 --- a/chapter25/src/test/java/com/seaofnodes/simple/Chapter24Test.java +++ b/chapter25/src/test/java/com/seaofnodes/simple/Chapter24Test.java @@ -373,17 +373,17 @@ sys.io.p(str); return 0; """; - TestC.runSF("stacked_r_3", src, null, "In Range", 5); + TestC.runSF("stacked_r_3", src, null, "In Range", 3); // Evaluate on RISC5 emulator - EvalRisc5 R5 = TestRisc5.build("stacked_r_3", src, 0, 5, false); + EvalRisc5 R5 = TestRisc5.build("stacked_r_3", src, 0, 3, false); int trap = R5.step(100); assertEquals(0,trap); assertEquals(0,R5.regs[riscv.A0]); assertEquals("In Range",R5._stdout.toString()); // Evaluate on ARM emulator - EvalArm64 arm = TestArm64.build("stacked_r_3", src, 0, 5, false); + EvalArm64 arm = TestArm64.build("stacked_r_3", src, 0, 3, false); int trap_1 = arm.step(100); assertEquals(0,trap_1); assertEquals(0,arm.regs[0]); @@ -439,17 +439,17 @@ sys.io.p("Out of range"); return 0; """; - TestC.runSF("stacked_r_5", src, null, "In Range", 5); + TestC.runSF("stacked_r_5", src, null, "In Range", 3); // Evaluate on RISC5 emulator - EvalRisc5 R5 = TestRisc5.build("stacked_r_5", src, 0, 7, false); + EvalRisc5 R5 = TestRisc5.build("stacked_r_5", src, 0, 5, false); int trap = R5.step(100); assertEquals(0,trap); assertEquals(0,R5.regs[riscv.A0]); assertEquals("In Range",R5._stdout.toString()); // Evaluate on ARM emulator - EvalArm64 arm = TestArm64.build("stacked_r_5", src, 0, 7, false); + EvalArm64 arm = TestArm64.build("stacked_r_5", src, 0, 5, false); trap = arm.step(100); assertEquals(0,trap); assertEquals(0,arm.regs[0]); diff --git a/chapter25/src/test/java/com/seaofnodes/simple/MergeSortTest.java b/chapter25/src/test/java/com/seaofnodes/simple/MergeSortTest.java index 453bd35c..324790a0 100644 --- a/chapter25/src/test/java/com/seaofnodes/simple/MergeSortTest.java +++ b/chapter25/src/test/java/com/seaofnodes/simple/MergeSortTest.java @@ -17,13 +17,13 @@ public class MergeSortTest { @Test public void testMergeSort0() throws IOException { // Test merge sort on a small array of primes String src = -""" +""" val merge_sort = { int[] a -> int[] !b = new int[a#]; copy_array (a, 0, a#, b); split_merge(a, 0, a#, b); }; - + val split_merge = { int[] !b, int begin, int end, int[] a -> if (end - begin <= 1) return 0; @@ -33,10 +33,10 @@ public class MergeSortTest { merge(b, begin, middle, end, a); return 0; }; - + val merge = { int[] !b, int begin, int middle, int end, int[] a -> int i = begin, j = middle; - + for (int k = begin; k < end; k++) { // && and || bool cond = false; @@ -48,7 +48,7 @@ public class MergeSortTest { else b[k] = a[j++]; } }; - + val copy_array = { int[] !a, int begin, int end, int[] b -> for (int k = begin; k < end; k++) b[k] = a[k]; -- cgit v1.2.3