Fix memory on Load splitting up

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() {