diff options
11 files changed, 170 insertions, 32 deletions
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<n;i++) { if(i==1) continue; if(i==3) break; x+=i+1; }" + }; + int[][] expected = {{0,0,1,1,1,1}, {0,1,1,4,4,4}}; + for( int i=0; i<bodies.length; i++ ) + for( long seed : new long[]{123,456,789} ) { + String src = "struct C { int x; new C={int n -> "+bodies[i]+"}; }; return new C(arg).x;"; + var code = new CodeGen(src,com.seaofnodes.simple.type.TypeInteger.BOT,seed,true) + .driver(CodeGen.Phase.TypeCheck); + for( int arg=0; arg<expected[i].length; arg++ ) + assertEquals("case="+i+" seed="+seed+" arg="+arg, + Integer.toString(expected[i][arg]),Eval2.eval(code,arg)); + } + } + private static final String CONSTRUCT = """ struct Box { int x=7; diff --git a/chapter26/src/test/java/com/seaofnodes/simple/IncompleteTypesTest.java b/chapter26/src/test/java/com/seaofnodes/simple/IncompleteTypesTest.java index f2f7d882..419cac51 100644 --- a/chapter26/src/test/java/com/seaofnodes/simple/IncompleteTypesTest.java +++ b/chapter26/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/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<n;i++) { if(i==1) continue; if(i==3) break; x+=i+1; }" + }; + int[][] expected = {{0,0,1,1,1,1}, {0,1,1,4,4,4}}; + for( int i=0; i<bodies.length; i++ ) + for( long seed : new long[]{123,456,789} ) { + String src = "struct C { int x; new C={int n -> "+bodies[i]+"}; }; return new C(arg).x;"; + var code = new CodeGen(src,com.seaofnodes.simple.type.TypeInteger.BOT,seed,true) + .driver(CodeGen.Phase.TypeCheck); + for( int arg=0; arg<expected[i].length; arg++ ) + assertEquals("case="+i+" seed="+seed+" arg="+arg, + Integer.toString(expected[i][arg]),Eval2.eval(code,arg)); + } + } + private static final String CONSTRUCT = """ struct Box { int x=7; diff --git a/chapter27/src/test/java/com/seaofnodes/simple/IncompleteTypesTest.java b/chapter27/src/test/java/com/seaofnodes/simple/IncompleteTypesTest.java index f2f7d882..419cac51 100644 --- a/chapter27/src/test/java/com/seaofnodes/simple/IncompleteTypesTest.java +++ b/chapter27/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/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<n;i++) { - if(i==1) continue; - if(i==3) break; - x+=i+1; - } - }; -}; -return new C(arg).x; -``` - -`IterPeeps.progressOnList` asserts during Iter with seed 123. The expected value -for arg 3 is 4. This is independent of final-field mutability; retain the -reproducer while reviewing constructor coverage. Scratch probe: -`build/readme-review/mutable_break.smp`. - ### SCCP function revival: Chapter 24 `Opto.linkCG` uses `code.link(fidx)`, which clears a linker entry when its diff --git a/docs/stage-audit.md b/docs/stage-audit.md index dac2dc77..abf90ace 100644 --- a/docs/stage-audit.md +++ b/docs/stage-audit.md @@ -57,6 +57,14 @@ expectations remain unchanged. No allocator goldens were updated. ## Implemented, with narrower limits than earlier prose suggested +The constructor-loop assertion is fixed in Chapters 25–27. Load forwarding now +waits for a stored loop Phi to acquire a type at least as precise as the Load, +and records a dependency to retry. The reduced case needs only a conditional +assignment in a loop; break/continue and final fields are not required. +Regressions cover the reduced case and the original constructor loop across +worklist seeds and execution inputs. Chapters 28a/28b already had the guard; +the corresponding earlier-chapter probes pass in 23/24a/24b. + - Separate compilation imports types and optimized IR without source parsing; it reruns analysis and type checking with the new callers. - Loading is by object and recorded dependencies, not individual used members. @@ -76,8 +84,6 @@ expectations remain unchanged. No allocator goldens were updated. - The ideal evaluator does not truncate narrow stores in Chapter 23. A byte update can therefore disagree with generated code; the postfix-only reproducer is retained in the [queue](chapter-backports.md). -- A constructor loop with both break and continue still reproduces a worklist - assertion in Chapter 26. It is not caused by final-field mutability. - `testModule0` is an ignored historical incremental-build experiment. It assumes per-source output objects and unchanged explicit-input builds, neither of which matches the present compiler's contract. |