From 69e4a21f71aca4954469063387063434af35e679 Mon Sep 17 00:00:00 2001 From: Cliff Click Date: Sun, 25 Oct 2026 10:24:58 -0700 Subject: Backport a few missing guard checks --- .../java/com/seaofnodes/simple/node/LoadNode.java | 16 +++++++- .../com/seaofnodes/simple/IncompleteTypesTest.java | 13 ++++++ .../java/com/seaofnodes/simple/node/LoadNode.java | 16 +++++++- .../com/seaofnodes/simple/ConstructionTest.java | 17 ++++++++ .../com/seaofnodes/simple/IncompleteTypesTest.java | 13 ++++++ .../java/com/seaofnodes/simple/node/LoadNode.java | 16 +++++++- .../com/seaofnodes/simple/ConstructionTest.java | 17 ++++++++ .../com/seaofnodes/simple/IncompleteTypesTest.java | 13 ++++++ .../java/com/seaofnodes/simple/Chapter28bTest.java | 47 ++++++++++++++++++++++ docs/chapter-backports.md | 24 ----------- docs/stage-audit.md | 10 ++++- 11 files changed, 170 insertions(+), 32 deletions(-) create mode 100644 chapter28b/src/test/java/com/seaofnodes/simple/Chapter28bTest.java 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 24a2f9b8..ff2d1138 100644 --- a/chapter25/src/main/java/com/seaofnodes/simple/node/LoadNode.java +++ b/chapter25/src/main/java/com/seaofnodes/simple/node/LoadNode.java @@ -170,7 +170,12 @@ public class LoadNode extends MemOpNode { // skipping it can make a compound assignment read the old value. if( st._alias==1 ) return null; addDep(st.ptr()); - if( _alias==st._alias && ptr==address(st.ptr()) && off()==st.off() ) return st; + if( _alias==st._alias && ptr==address(st.ptr()) && off()==st.off() ) { + // A full-width load cannot narrow the forwarded value. + if( _declaredType instanceof TypeInteger ti && ti.log_size()==3 && + !addDep(st.val())._type.isa(_type) ) return null; + return st; + } if( _alias==st._alias && !neverAlias(ptr,address(st.ptr())) && !addDep(off())._type.join(addDep(st.off())._type).isHigh() ) return null; mem=st.mem(); @@ -269,8 +274,15 @@ public class LoadNode extends MemOpNode { // Signed extension int shift = Long.numberOfLeadingZeros(ti._max)-1; Node shf = con(shift); - if( shf._type==TypeInteger.ZERO ) + if( shf._type==TypeInteger.ZERO ) { + // A constructor's loop Phi may still have an unsettled type. + // Wait for it instead of weakening the typed Load's users. + if( !val._type.isa(_type) ) { + addDep(val); + return null; + } return val; + } Node shl = new ShlNode(null,val,shf.keep()).peephole(); return new SarNode(null,shl,shf.unkeep()); } diff --git a/chapter25/src/test/java/com/seaofnodes/simple/IncompleteTypesTest.java b/chapter25/src/test/java/com/seaofnodes/simple/IncompleteTypesTest.java index f2f7d882..419cac51 100644 --- a/chapter25/src/test/java/com/seaofnodes/simple/IncompleteTypesTest.java +++ b/chapter25/src/test/java/com/seaofnodes/simple/IncompleteTypesTest.java @@ -6,6 +6,19 @@ import com.seaofnodes.simple.type.TypeInteger; import static org.junit.Assert.assertEquals; public class IncompleteTypesTest { + @Test public void testLoadFromUnsettledLoopPhi() { + String src = """ + struct C { int x; }; + val f={int n -> return new C { while(n--) if(n) x=1; }; }; + return f(arg).x; + """; + for( long seed : new long[]{123,456,789} ) { + var code = new CodeGen(src,TypeInteger.BOT,seed,true).driver(CodeGen.Phase.TypeCheck); + for( int arg=0; arg<6; arg++ ) + assertEquals("seed="+seed+" arg="+arg,arg<2 ? "0" : "1",Eval2.eval(code,arg)); + } + } + @Test public void testForwardFieldAndNumericMode() { // At the addition, neither the field's type nor its offset is known. String src = """ diff --git a/chapter26/src/main/java/com/seaofnodes/simple/node/LoadNode.java b/chapter26/src/main/java/com/seaofnodes/simple/node/LoadNode.java index b9dd2af6..736d91a2 100644 --- a/chapter26/src/main/java/com/seaofnodes/simple/node/LoadNode.java +++ b/chapter26/src/main/java/com/seaofnodes/simple/node/LoadNode.java @@ -208,7 +208,12 @@ public class LoadNode extends MemOpNode { // skipping it can make a compound assignment read the old value. if( st._alias==1 ) return null; addDep(st.ptr()); - if( _alias==st._alias && ptr==address(st.ptr()) && off()==st.off() ) return st; + if( _alias==st._alias && ptr==address(st.ptr()) && off()==st.off() ) { + // A full-width load cannot narrow the forwarded value. + if( _declaredType instanceof TypeInteger ti && ti.log_size()==3 && + !addDep(st.val())._type.isa(_type) ) return null; + return st; + } if( _alias==st._alias && !neverAlias(ptr,address(st.ptr())) && !addDep(off())._type.join(addDep(st.off())._type).isHigh() ) return null; mem=st.mem(); @@ -313,8 +318,15 @@ public class LoadNode extends MemOpNode { // Signed extension int shift = Long.numberOfLeadingZeros(ti._max)-1; Node shf = con(shift); - if( shf._type==TypeInteger.ZERO ) + if( shf._type==TypeInteger.ZERO ) { + // A constructor's loop Phi may still have an unsettled type. + // Wait for it instead of weakening the typed Load's users. + if( !val._type.isa(_type) ) { + addDep(val); + return null; + } return val; + } Node shl = new ShlNode(null,val,shf.keep()).peephole(); return new SarNode(null,shl,shf.unkeep()); } diff --git a/chapter26/src/test/java/com/seaofnodes/simple/ConstructionTest.java b/chapter26/src/test/java/com/seaofnodes/simple/ConstructionTest.java index dd20b746..a3317d09 100644 --- a/chapter26/src/test/java/com/seaofnodes/simple/ConstructionTest.java +++ b/chapter26/src/test/java/com/seaofnodes/simple/ConstructionTest.java @@ -7,6 +7,23 @@ import org.junit.Test; import static org.junit.Assert.*; public class ConstructionTest { + @Test public void testConditionalConstructorLoop() { + String[] bodies = { + "while(n--) if(n) x=1;", + "for(int i=0;i return new C { while(n--) if(n) x=1; }; }; + return f(arg).x; + """; + for( long seed : new long[]{123,456,789} ) { + var code = new CodeGen(src,TypeInteger.BOT,seed,true).driver(CodeGen.Phase.TypeCheck); + for( int arg=0; arg<6; arg++ ) + assertEquals("seed="+seed+" arg="+arg,arg<2 ? "0" : "1",Eval2.eval(code,arg)); + } + } + @Test public void testForwardFieldAndNumericMode() { // At the addition, neither the field's type nor its offset is known. String src = """ diff --git a/chapter27/src/main/java/com/seaofnodes/simple/node/LoadNode.java b/chapter27/src/main/java/com/seaofnodes/simple/node/LoadNode.java index b9dd2af6..736d91a2 100644 --- a/chapter27/src/main/java/com/seaofnodes/simple/node/LoadNode.java +++ b/chapter27/src/main/java/com/seaofnodes/simple/node/LoadNode.java @@ -208,7 +208,12 @@ public class LoadNode extends MemOpNode { // skipping it can make a compound assignment read the old value. if( st._alias==1 ) return null; addDep(st.ptr()); - if( _alias==st._alias && ptr==address(st.ptr()) && off()==st.off() ) return st; + if( _alias==st._alias && ptr==address(st.ptr()) && off()==st.off() ) { + // A full-width load cannot narrow the forwarded value. + if( _declaredType instanceof TypeInteger ti && ti.log_size()==3 && + !addDep(st.val())._type.isa(_type) ) return null; + return st; + } if( _alias==st._alias && !neverAlias(ptr,address(st.ptr())) && !addDep(off())._type.join(addDep(st.off())._type).isHigh() ) return null; mem=st.mem(); @@ -313,8 +318,15 @@ public class LoadNode extends MemOpNode { // Signed extension int shift = Long.numberOfLeadingZeros(ti._max)-1; Node shf = con(shift); - if( shf._type==TypeInteger.ZERO ) + if( shf._type==TypeInteger.ZERO ) { + // A constructor's loop Phi may still have an unsettled type. + // Wait for it instead of weakening the typed Load's users. + if( !val._type.isa(_type) ) { + addDep(val); + return null; + } return val; + } Node shl = new ShlNode(null,val,shf.keep()).peephole(); return new SarNode(null,shl,shf.unkeep()); } diff --git a/chapter27/src/test/java/com/seaofnodes/simple/ConstructionTest.java b/chapter27/src/test/java/com/seaofnodes/simple/ConstructionTest.java index dd20b746..a3317d09 100644 --- a/chapter27/src/test/java/com/seaofnodes/simple/ConstructionTest.java +++ b/chapter27/src/test/java/com/seaofnodes/simple/ConstructionTest.java @@ -7,6 +7,23 @@ import org.junit.Test; import static org.junit.Assert.*; public class ConstructionTest { + @Test public void testConditionalConstructorLoop() { + String[] bodies = { + "while(n--) if(n) x=1;", + "for(int i=0;i return new C { while(n--) if(n) x=1; }; }; + return f(arg).x; + """; + for( long seed : new long[]{123,456,789} ) { + var code = new CodeGen(src,TypeInteger.BOT,seed,true).driver(CodeGen.Phase.TypeCheck); + for( int arg=0; arg<6; arg++ ) + assertEquals("seed="+seed+" arg="+arg,arg<2 ? "0" : "1",Eval2.eval(code,arg)); + } + } + @Test public void testForwardFieldAndNumericMode() { // At the addition, neither the field's type nor its offset is known. String src = """ diff --git a/chapter28b/src/test/java/com/seaofnodes/simple/Chapter28bTest.java b/chapter28b/src/test/java/com/seaofnodes/simple/Chapter28bTest.java new file mode 100644 index 00000000..fb418b9a --- /dev/null +++ b/chapter28b/src/test/java/com/seaofnodes/simple/Chapter28bTest.java @@ -0,0 +1,47 @@ +package com.seaofnodes.simple; + +// +//import com.seaofnodes.simple.codegen.CodeGen; +//import com.seaofnodes.simple.codegen.RegAllocTestSupport.CheckedCodeGen; +//import com.seaofnodes.simple.codegen.ElfReader; +//import com.seaofnodes.simple.codegen.ParseAll; +//import com.seaofnodes.simple.node.CallNode; +//import com.seaofnodes.simple.node.FunNode; +//import com.seaofnodes.simple.node.LoopNode; +//import com.seaofnodes.simple.type.TypeInteger; +//import com.seaofnodes.simple.util.Ary; +//import java.io.File; +//import java.io.FileWriter; +//import java.io.IOException; +//import java.io.ByteArrayOutputStream; +//import java.io.PrintStream; +//import java.nio.charset.StandardCharsets; +//import java.nio.file.Files; +//import java.nio.file.Path; +import com.seaofnodes.simple.codegen.CodeGen; +import org.junit.Ignore; +import org.junit.Test; +// +import static org.junit.Assert.*; + +public class Chapter28bTest { + + @Ignore + @Test + public void testJig() { + String src = """ +struct C { + int x; + new C = { int n -> + while(n--) + if(n) x=1; + }; +}; +return new C(arg).x; +"""; + CodeGen code = new CodeGen(src).driver(CodeGen.Phase.Encoding,"riscv","SystemV"); + assertEquals("return (arg+3);", code.print()); + assertEquals("3",Eval2.eval(code,0)); + } + +} diff --git a/docs/chapter-backports.md b/docs/chapter-backports.md index 1c2ffb85..e7293d8c 100644 --- a/docs/chapter-backports.md +++ b/docs/chapter-backports.md @@ -132,30 +132,6 @@ Establish the first affected evaluator and check signed fields and floating rounding before propagating a fix. Do not change valid generated code to match an incorrect ideal evaluator. -### Constructor loop with break/continue: worklist assertion - -This fails in construction and in the unchanged escape compiler before the -final-field fix, even with a mutable field: - -```simple -struct C { - int x=0; - new C={int n -> - for(int i=0;i