diff options
Diffstat (limited to 'chapter25')
| -rw-r--r-- | chapter25/src/main/java/com/seaofnodes/simple/node/LoadNode.java | 5 | ||||
| -rw-r--r-- | chapter25/src/test/java/com/seaofnodes/simple/Chapter15Test.java | 53 |
2 files changed, 56 insertions, 2 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 0a997083..c58bba39 100644 --- a/chapter25/src/main/java/com/seaofnodes/simple/node/LoadNode.java +++ b/chapter25/src/main/java/com/seaofnodes/simple/node/LoadNode.java @@ -169,8 +169,9 @@ public class LoadNode extends MemOpNode { // else ptr.x = e1; : e1; // val = ptr.x; ptr.x = val; if( mem() instanceof MemPhiNode memphi && memphi.region()._type == Type.CONTROL && memphi.nIns()== 3 && - // Array access control must not be moved across the merge. - in(0)==null && + // Retain control on both copies only if it dominates both paths. + // The Region itself, or a later range check, cannot move to its inputs. + (in(0)==null || cfg0().domLCA(memphi.region().idom(this),this)==cfg0()) && // Offset can be hoisted off() instanceof ConstantNode && // Pointer can be hoisted diff --git a/chapter25/src/test/java/com/seaofnodes/simple/Chapter15Test.java b/chapter25/src/test/java/com/seaofnodes/simple/Chapter15Test.java index 25f00474..c2071cb6 100644 --- a/chapter25/src/test/java/com/seaofnodes/simple/Chapter15Test.java +++ b/chapter25/src/test/java/com/seaofnodes/simple/Chapter15Test.java @@ -10,6 +10,59 @@ import static org.junit.Assert.*; public class Chapter15Test { + @Test public void testArrayLoadDominatingControl() { + String src = """ + int[] !a=new int[arg+1]; a[arg]=7; + if(a#>1) { + if(arg==2) a[1]=9; + return a[1]; + } + return 42; + """; + var code = new CodeGen(src).parse(); + var stop = code._stop; + var load = stop.walk(n -> n instanceof com.seaofnodes.simple.node.LoadNode ld && ld._name.equals("[]") ? ld : null); + org.junit.Assert.assertNotNull(load); + // The parser uses current control as a placeholder for a range check. + // Model an explicit check already performed before the inner diamond: + // both predecessors are below the a#>1 true projection. + var region = (com.seaofnodes.simple.node.RegionNode)load.in(0); + var checked = region.idom().cfg(0); + org.junit.Assert.assertTrue(checked instanceof com.seaofnodes.simple.node.CProjNode); + load.setDef(0,checked); + code.driver(CodeGen.Phase.TypeCheck); + org.junit.Assert.assertTrue("Common dominating control permits Phi splitting",load.isDead()); + var split = stop.walk(n -> n instanceof com.seaofnodes.simple.node.LoadNode ld && ld._name.equals("[]") ? ld : null); + org.junit.Assert.assertNotNull(split); + org.junit.Assert.assertSame("The surviving read retains the earlier check",checked,split.in(0)); + code.driver(CodeGen.Phase.LocalSched); + assertEquals("42",Eval2.eval(code,0)); + assertEquals("7",Eval2.eval(code,1)); + assertEquals("9",Eval2.eval(code,2)); + } + + + @Test public void testArrayLoadControl() { + // With arg==0, a has length 1 and a[1] must not execute. The memory + // merge must not pull the read above its array-length guard. + String src = """ + int n=1; if(arg) n=2; + int[] !a=new int[n]; a[arg]=7; + if(arg) a[1]=9; + if(a#>1) return a[1]; + return 42; + """; + var code = new CodeGen(src).parse(); + var stop = code._stop; + var load = stop.walk(n -> n instanceof com.seaofnodes.simple.node.LoadNode ld && ld._name.equals("[]") ? ld : null); + org.junit.Assert.assertNotNull(load); + org.junit.Assert.assertNotNull("Element loads carry their range-check control",load.in(0)); + code.driver(CodeGen.Phase.LocalSched); + assertEquals("42",Eval2.eval(code,0)); + assertEquals("9",Eval2.eval(code,1)); + } + + @Ignore @Test public void testJig() { |