MemStore interface (#201)

* Hook for pre-encoding work needed by M2

* Add MemStore interface

* Add both MemLoad and MemStore interfaces

* Fix NPE printing StopNode

* Attach merged mem to return when forcing a loop exit

* Made requested changes. This breaks a few tests, possibly because...

...I am not sure whether some X86 ops are loads or stores.

* Mach ops need anti-deps too

MemOp subclasses are either "Load-like" or "Store-like" based on a flag, and not on subclassing LoadNode nor StoreNode.
Mach ops pick up anti-deps, no reg mask for those.
x86 bug: do not allow "dst = [mem] + #imm", no such instruction
Empty merge-mem folds away.
x86 add-to-mem works both ways

---------

Co-authored-by: Cliff Click <cliffc@acm.org>

18 files changed, 111 insertions, 71 deletions

diff --git a/chapter21/src/main/java/com/seaofnodes/simple/codegen/BuildLRG.java b/chapter21/src/main/java/com/seaofnodes/simple/codegen/BuildLRG.java
index ca5157c7..a2bb21ed 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/codegen/BuildLRG.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/codegen/BuildLRG.java
@@ -1,6 +1,5 @@
 package com.seaofnodes.simple.codegen;
 
-import com.seaofnodes.simple.Utils;
 import com.seaofnodes.simple.node.*;
 import com.seaofnodes.simple.type.TypeMem;
 
@@ -55,8 +54,8 @@ abstract public class BuildLRG {
                         if( n.in(i)!=null ) {
                             LRG lrg2 = alloc.lrg(n.in(i));
                             if( lrg2 != null ) { // Anti-dep or other, no LRG
-                                RegMask use_mask = mach.regmap(i);
-                                if( !lrg2.machUse(mach,(short)i,use_mask.size1()).and(use_mask) )
+                                RegMask use_mask = mach.regmap(i); // use_mask is also null for anti-dep
+                                if( use_mask!=null && !lrg2.machUse(mach,(short)i,use_mask.size1()).and(use_mask) )
                                     alloc.fail(lrg2); // Empty register mask, must split
                             }
                         }
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/codegen/GlobalCodeMotion.java b/chapter21/src/main/java/com/seaofnodes/simple/codegen/GlobalCodeMotion.java
index ee8c0259..e904dc66 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/codegen/GlobalCodeMotion.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/codegen/GlobalCodeMotion.java
@@ -160,7 +160,7 @@ public abstract class GlobalCodeMotion {
                         continue outer; // Nope, await all uses done
 
                 // Loads need their memory inputs' uses also done
-                if( n instanceof LoadNode ld )
+                if( n instanceof MemOpNode ld && ld._isLoad )
                     for( Node memuse : ld.mem()._outputs )
                         if( late[memuse._nid]==null &&
                             // New makes new memory, never crushes load memory
@@ -180,8 +180,8 @@ public abstract class GlobalCodeMotion {
                 if( def!=null && late[def._nid]==null ) {
                     work.push(def);
                     // if the def has a load use, maybe the load can fire
-                    for( Node ld : def._outputs )
-                        if( ld instanceof LoadNode && late[ld._nid]==null )
+                    for( Node out : def._outputs )
+                        if( out instanceof MemOpNode ld && ld._isLoad && late[ld._nid]==null )
                             work.push(ld);
                 }
         }
@@ -197,7 +197,7 @@ public abstract class GlobalCodeMotion {
               lca = use_block(n,use, late)._idom(lca,null);
 
         // Loads may need anti-dependencies, raising their LCA
-        if( n instanceof LoadNode load )
+        if( n instanceof MemOpNode load && load._isLoad )
             lca = find_anti_dep(lca,load,early,late);
 
         // Walk up from the LCA to the early, looking for best place.  This is
@@ -235,7 +235,7 @@ public abstract class GlobalCodeMotion {
                 (lca.idepth() > best.idepth() || best instanceof IfNode);
     }
 
-    private static CFGNode find_anti_dep(CFGNode lca, LoadNode load, CFGNode early, CFGNode[] late) {
+    private static CFGNode find_anti_dep(CFGNode lca, MemOpNode load, CFGNode early, CFGNode[] late) {
         // We could skip final-field loads here.
         // Walk LCA->early, flagging Load's block location choices
         for( CFGNode cfg=lca; early!=null && cfg!=early.idom(); cfg = cfg.idom() )
@@ -243,10 +243,12 @@ public abstract class GlobalCodeMotion {
         // Walk load->mem uses, looking for Stores causing an anti-dep
         for( Node mem : load.mem()._outputs ) {
             switch( mem ) {
-            case StoreNode st:
-                assert late[st._nid]!=null;
-                lca = anti_dep(load,late[st._nid],st.cfg0(),lca,st);
-                break;
+            case MemOpNode st:
+                if( !st._isLoad ) {
+                    assert late[mem._nid] != null;
+                    lca = anti_dep( load, late[mem._nid], mem.cfg0(), lca, st );
+                }
+                break; // Loads do not cause anti-deps on other loads
             case CallNode st:
                 assert late[st._nid]!=null;
                 lca = anti_dep(load,late[st._nid],st.cfg0(),lca,st);
@@ -259,7 +261,6 @@ public abstract class GlobalCodeMotion {
                         lca = anti_dep(load,phi.region().cfg(i),load.mem().cfg0(),lca,null);
                 break;
             case NewNode st: break;
-            case LoadNode ld: break; // Loads do not cause anti-deps on other loads
             case ReturnNode ret: break; // Load must already be ahead of Return
             case MemMergeNode ret: break; // Mem uses now on ScopeMin
             case NeverNode never: break;
@@ -270,16 +271,13 @@ public abstract class GlobalCodeMotion {
     }
 
     //
-    private static CFGNode anti_dep( LoadNode load, CFGNode stblk, CFGNode defblk, CFGNode lca, Node st ) {
-        // Walk store blocks "reach" from its scheduled location to its earliest
-        for( ; stblk != defblk.idom(); stblk = stblk.idom() ) {
-            // Store and Load overlap, need anti-dependence
-            if( stblk._anti==load._nid ) {
-                lca = stblk._idom(lca,null); // Raise Loads LCA
-                if( lca == stblk && st != null && st._inputs.find(load) == -1 ) // And if something moved,
-                    st.addDef(load);   // Add anti-dep as well
-                return lca;            // Cap this stores' anti-dep to here
-            }
+    private static CFGNode anti_dep( MemOpNode load, CFGNode stblk, CFGNode defblk, CFGNode lca, Node st ) {
+        // Store and Load overlap, need anti-dependence
+        if( stblk._anti==load._nid ) {
+            lca = stblk._idom(lca,null); // Raise Loads LCA
+            if( lca == stblk && st != null && st._inputs.find(load) == -1 ) // And if something moved,
+                st.addDef(load); // Add anti-dep as well
+            return lca;          // Cap this stores' anti-dep to here
         }
         return lca;
     }
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/codegen/IFG.java b/chapter21/src/main/java/com/seaofnodes/simple/codegen/IFG.java
index 34b755b0..c1b9d3e4 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/codegen/IFG.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/codegen/IFG.java
@@ -174,7 +174,7 @@ abstract public class IFG {
             // Look for a must-use single register conflicting with some other must-def.
             if( n instanceof MachNode m ) {
                 RegMask ni_mask = m.regmap(i);
-                if( ni_mask.size1() ) { // Must-use single register
+                if( ni_mask!=null && ni_mask.size1() ) { // Must-use single register
                     // Search all current live
                     for( LRG tlrg : TMP.keySet() ) {
                         assert tlrg.leader();
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/codegen/LRG.java b/chapter21/src/main/java/com/seaofnodes/simple/codegen/LRG.java
index 018f18ab..ec254a2e 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/codegen/LRG.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/codegen/LRG.java
@@ -45,11 +45,6 @@ public class LRG {
     public MachNode _machDef, _machUse;
     short _uidx;                // _machUse input
 
-    // Mask set to empty via a kill-mask.  This is usually a capacity kill,
-    // which means we need to split and spill into memory - which means even if
-    // the def-side has many registers, it MUST spill.
-    boolean _killed;
-
     // Some splits used in biased coloring
     MachConcreteNode _splitDef, _splitUse;
 
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/LoadNode.java b/chapter21/src/main/java/com/seaofnodes/simple/node/LoadNode.java
index e5174823..b8ddeca4 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/LoadNode.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/LoadNode.java
@@ -20,7 +20,7 @@ public class LoadNode extends MemOpNode {
      * @param off   The offset inside the struct base
      */
     public LoadNode(Parser.Lexer loc, String name, int alias, Type glb, Node mem, Node ptr, Node off) {
-        super(loc, name, alias, glb, mem, ptr, off);
+        super(loc, name, alias, true, glb, mem, ptr, off);
     }
 
     // GraphVis DOT code (must be valid Java identifiers) and debugger labels
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/LoopNode.java b/chapter21/src/main/java/com/seaofnodes/simple/node/LoopNode.java
index 3a57fde7..778f2a8d 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/LoopNode.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/LoopNode.java
@@ -59,6 +59,22 @@ public class LoopNode extends RegionNode {
         // Now fold control into the exit.  Might have 1 valid exit, or an
         // XCtrl or a bunch of prior NeverNode exits.
         Node top = new ConstantNode(Type.TOP).peephole();
+        Node memout = new MemMergeNode(false);
+        memout.addDef(null); // placeholder for control
+        for( Node u : _outputs )
+            if( u instanceof PhiNode phi && phi._type.isa(TypeMem.BOT) )
+                memout.addDef(phi);
+        if( memout.nIns()>2 )
+            memout.setDef(0, ret.cfg0());
+        else {
+            if( memout.nIns()==1 ) memout=top;
+            else {
+                Node tmp=memout;
+                memout=memout.in(1);
+                tmp.setDef(1, null);
+            }
+        }
+
         Node ctrl = ret.ctrl(), mem = ret.mem(), expr = ret.expr();
         if( ctrl!=null && ctrl._type != Type.XCONTROL ) {
             // Perfect aligned exit?
@@ -73,11 +89,11 @@ public class LoopNode extends RegionNode {
             }
             // Append new Never exit
             ctrl.addDef(f  );
-            mem .addDef(top);
+            mem .addDef(memout);
             expr.addDef(top);
         } else {
             ctrl = f;
-            mem  = top;
+            mem  = memout;
             expr = top;
         }
         ret.setDef(0,ctrl);
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/MemMergeNode.java b/chapter21/src/main/java/com/seaofnodes/simple/node/MemMergeNode.java
index 40859eea..32c25a8b 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/MemMergeNode.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/MemMergeNode.java
@@ -65,11 +65,18 @@ public class MemMergeNode extends Node {
         if( inProgress() ) return null;
 
         // If not merging any memory (all memory is just the default)
-        if( nIns()==2 )
+        if( allDefault() )
             return in(1);       // Become default memory
 
         return null;
     }
+    private boolean allDefault() {
+        for( int i=2; i<nIns(); i++ )
+            if( in(1) != in(i) )
+                return false;
+        return true;
+    }
+
 
     public Node in( Var v ) { return in(v._idx); }
 
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/MemOpNode.java b/chapter21/src/main/java/com/seaofnodes/simple/node/MemOpNode.java
index 28ca9f91..e923df98 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/MemOpNode.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/MemOpNode.java
@@ -18,23 +18,40 @@ import java.util.BitSet;
  */
 public abstract class MemOpNode extends Node {
 
-    public final String _name;
+    // The equivalence alias class
     public final int _alias;
+
+    // True if load-like, false if store-like.
+    //
+    // Stores produce memory (maybe as part of a tuple with other things),
+    // loads do not.
+    //
+    // Loads might pick up anti-dependences on prior Stores, and never cause an
+    // anti-dependence themselves.
+    //
+    // Stores must maximally sink to the least dominator use.  Loads can be
+    // opportunistically hoisted.
+    public final boolean _isLoad;
+
     // Declared type; not final because it might be a forward-reference
     // which will be lazily improved when the reference is declared.
     public Type _declaredType;
+
+    // A debug name, no semantic meaning
+    public final String _name;
     // Source location for late reported errors
     public final Parser.Lexer _loc;
 
-    public MemOpNode(Parser.Lexer loc, String name, int alias, Type glb, Node mem, Node ptr, Node off) {
+    public MemOpNode(Parser.Lexer loc, String name, int alias, boolean isLoad, Type glb, Node mem, Node ptr, Node off) {
         super(null, mem, ptr, off);
         _name  = name;
         _alias = alias;
         _declaredType = glb;
         _loc = loc;
+        _isLoad = isLoad;
     }
-    public MemOpNode(Parser.Lexer loc, String name, int alias, Type glb, Node mem, Node ptr, Node off, Node value) {
-        this(loc,name, alias, glb, mem, ptr, off);
+    public MemOpNode(Parser.Lexer loc, String name, int alias, boolean isLoad, Type glb, Node mem, Node ptr, Node off, Node value) {
+        this(loc,name, alias, isLoad, glb, mem, ptr, off);
         addDef(value);
     }
     public MemOpNode( Node mach, MemOpNode mop ) {
@@ -42,7 +59,10 @@ public abstract class MemOpNode extends Node {
         _name  = mop==null ? null : mop._name;
         _alias = mop==null ? 0    : mop._alias;
         _loc   = mop==null ? null : mop._loc;
+        _isLoad= mop==null ? true : mop._isLoad;
         _declaredType = mop==null ? Type.BOTTOM : mop._declaredType;
+        if( mop==null )
+            throw Utils.TODO("Load or not");
     }
 
     //
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/StopNode.java b/chapter21/src/main/java/com/seaofnodes/simple/node/StopNode.java
index 953324e7..9be08ad9 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/StopNode.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/StopNode.java
@@ -26,7 +26,8 @@ public class StopNode extends CFGNode {
         if( ret()!=null ) return ret()._print0(sb,visited);
         sb.append("Stop[ ");
         for( Node ret : _inputs )
-            ret._print0(sb, visited).append(" ");
+            if( ret!=null )
+                ret._print0(sb, visited).append(" ");
         return sb.append("]");
     }
 
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/StoreNode.java b/chapter21/src/main/java/com/seaofnodes/simple/node/StoreNode.java
index 2d553810..7f43f53c 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/StoreNode.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/StoreNode.java
@@ -20,7 +20,7 @@ public class StoreNode extends MemOpNode {
      * @param value Value to be stored
      */
     public StoreNode(Parser.Lexer loc, String name, int alias, Type glb, Node mem, Node ptr, Node off, Node value, boolean init) {
-        super(loc, name, alias, glb, mem, ptr, off, value);
+        super(loc, name, alias, false, glb, mem, ptr, off, value);
         _init = init;
     }
 
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/arm/LoadARM.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/arm/LoadARM.java
index 7a7f06b8..daf221f3 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/arm/LoadARM.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/arm/LoadARM.java
@@ -12,8 +12,8 @@ import com.seaofnodes.simple.type.TypeFloat;
 // idx  = null
 // off  = off - offset added to base
 
-public class LoadARM extends MemOpARM{
-    LoadARM(LoadNode ld,Node base, Node idx, int off) {
+public class LoadARM extends MemOpARM {
+    LoadARM(LoadNode ld, Node base, Node idx, int off) {
         super(ld, base, idx, off, 0);
     }
     @Override public String op() { return "ld"+_sz; }
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/arm/MemOpARM.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/arm/MemOpARM.java
index ff3c41c3..d0ae385a 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/arm/MemOpARM.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/arm/MemOpARM.java
@@ -44,7 +44,7 @@ public abstract class MemOpARM extends MemOpNode implements MachNode {
         if( i==2 ) return arm.RMASK; // ptr/base
         if( i==3 ) return arm.RMASK; // off/index
         if( i==4 ) return arm.RMASK; // value
-        throw Utils.TODO();
+        return null; // Anti-dependence
     }
 
     // Shared encoding for loads and stores(int and float)
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/StoreRISC.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/StoreRISC.java
index 822e67b0..d34d94fc 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/StoreRISC.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/StoreRISC.java
@@ -27,7 +27,7 @@ public class StoreRISC extends MemOpRISC {
         if( i==2 ) return riscv.RMASK; // ptr
         // 2 - index
         if( i==4 ) return riscv.MEM_MASK; // Wide mask to store GPR and FPR
-        throw Utils.TODO();
+        return null; // Anti-dependence
     }
     @Override public RegMask outregmap() { return null; }
 
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/AddMemX86.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/AddMemX86.java
index acf4d4b9..f97a7cb8 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/AddMemX86.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/AddMemX86.java
@@ -26,10 +26,9 @@ public class AddMemX86 extends MemOpX86 {
         x86_64_v2.indirectAdr(_scale, idx, ptr, _off, dst, enc);
     }
 
-    // General form: "add  dst = src + [base + idx<<2 + 12]"
+    // General form: "add  dst += [base + idx<<2 + 12]"
     @Override public void asm(CodeGen code, SB sb) {
-        sb.p(code.reg(this)).p(" = ");
-        sb.p(val()==null ? "#"+_imm : code.reg(val())).p(" + ");
+        sb.p(code.reg(this)).p(" += ");
         asm_address(code,sb);
     }
 }
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MemOpX86.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MemOpX86.java
index 4cb94b31..aee543ca 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MemOpX86.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/MemOpX86.java
@@ -23,13 +23,13 @@ public abstract class MemOpX86 extends MemOpNode implements MachNode {
     final int _off;             // Limit 32 bits
     final int _scale;           // Limit 0,1,2,3
     final int _imm;             // Limit 32 bits
-    final char _sz = (char)('0'+(1<<_declaredType.log_size()));
+    final char _sz;             // Handy print name
     MemOpX86( Node op, MemOpNode mop, Node base, Node idx, int off, int scale, int imm ) {
         super(op,mop);
         assert base._type instanceof TypeMemPtr && !(base instanceof AddNode);
         assert (idx==null && scale==0) || (idx!=null && 0<= scale && scale<=3);
 
-        // Copy memory parts from eg the LoadNode over the opcode, e.g. an Add
+        // Copy memory parts from e.g. the LoadNode over the opcode, e.g. an Add
         if( op != mop ) {
             _inputs.set(0,mop.in(0)); // Control from mem op
             _inputs.set(1,mop.in(1)); // Memory  from mem op
@@ -41,6 +41,7 @@ public abstract class MemOpX86 extends MemOpNode implements MachNode {
         _off = off;
         _scale = scale;
         _imm = imm;
+        _sz = (char)('0'+(1<<_declaredType.log_size()));
     }
 
     // Store-based flavors have a value edge
@@ -66,7 +67,7 @@ public abstract class MemOpX86 extends MemOpNode implements MachNode {
         if( i==2 ) return x86_64_v2.RMASK;    // base  in GPR
         if( i==3 ) return x86_64_v2.RMASK;    // index in GPR
         if( i==4 ) return x86_64_v2.MEM_MASK; // value in GPR or XMM
-        throw Utils.TODO();
+        return null; // Anti-dependence
     }
 
     // "[base + idx<<2 + 12]"
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/x86_64_v2.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/x86_64_v2.java
index 7683177f..b4c382c5 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/x86_64_v2.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/x86_64_v2.java
@@ -468,7 +468,7 @@ public class x86_64_v2 extends Machine {
         Node lhs = add.in(1);
         Node rhs = add.in(2);
         if( lhs instanceof LoadNode ld && ld.nOuts() == 1 && ld._declaredType.log_size() >= 3)
-            return new AddMemX86(add, address(ld), ld.ptr(), idx, off, scale, imm(rhs), val);
+            return new AddMemX86(add, address(ld), ld.ptr(), idx, off, scale, 0, rhs);
 
 //        if(rhs instanceof LoadNode ld && ld.nOuts() == 1 && ld._declaredType.log_size() >= 3) {
 //            throw Utils.TODO(); // Swap load sides
@@ -476,7 +476,7 @@ public class x86_64_v2 extends Machine {
 
         // Attempt a full LEA-style break down.
         // Returns one of AddX86, AddIX86, LeaX86, or LHS
-        if( rhs instanceof ConstantNode off && off._con instanceof TypeInteger toff ) {
+        if( rhs instanceof ConstantNode off2 && off2._con instanceof TypeInteger toff ) {
             long imm = toff.value();
             assert imm!=0;        // Folded in peeps
             if( (int)imm != imm ) // Full 64bit immediate
@@ -639,21 +639,25 @@ public class x86_64_v2 extends Machine {
         return new ShrX86(shr);
     }
 
-    private Node st (StoreNode st ){
+    private Node st( StoreNode st ) {
         // Look for "*ptr op= val"
         Node op = st.val();
-        if(op instanceof AddNode) {
-            if(op.in(1) instanceof LoadNode ld &&
-               ld.in(0) == st.in(0) &&
-               ld.mem() == st.mem() &&
-               ld.ptr() == st.ptr() &&
-               ld.off() == st.off()) {
-               return new MemAddX86(address(st), st.ptr(), idx, off, scale, imm(op.in(2)), val);
-            }
+        if( op instanceof AddNode ) {
+            if( op.in(1) instanceof LoadNode ld && stld_match(st,ld) )
+                return new MemAddX86(address(st), st.ptr(), idx, off, scale, imm(op.in(2)), val);
+            if( op.in(2) instanceof LoadNode ld && stld_match(st,ld) )
+                return new MemAddX86(address(st), st.ptr(), idx, off, scale, imm(op.in(1)), val);
         }
-
         return new StoreX86(address(st), st.ptr(), idx, off, scale, imm(st.val()), val);
     }
+    private static boolean stld_match(StoreNode st, LoadNode ld ) {
+        return
+            ld.in(0) == st.in(0) &&
+            ld.mem() == st.mem() &&
+            ld.ptr() == st.ptr() &&
+            ld.off() == st.off();
+    }
+
 
     private Node sub (SubNode sub ){
         return sub.in(2) instanceof ConstantNode con && con._con instanceof TypeInteger ti
@@ -699,7 +703,7 @@ public class x86_64_v2 extends Machine {
         return mop;
     }
 
-    private int imm (Node xval ){
+    private int imm( Node xval ) {
         assert val == null && imm == 0;
         if( xval instanceof ConstantNode con && con._con instanceof TypeInteger ti) {
             val = null;
diff --git a/chapter21/src/test/java/com/seaofnodes/simple/BrainFuckTest.java b/chapter21/src/test/java/com/seaofnodes/simple/BrainFuckTest.java
index ba852c9e..991a2f66 100644
--- a/chapter21/src/test/java/com/seaofnodes/simple/BrainFuckTest.java
+++ b/chapter21/src/test/java/com/seaofnodes/simple/BrainFuckTest.java
@@ -63,7 +63,7 @@ for( int pc = 0; pc < program#; pc++ ) {
 return output;
 """;
         Chapter21Test.testCPU(src,"x86_64_v2", "SystemV",40,null);
-        Chapter21Test.testCPU(src,"riscv"    , "SystemV",30,null);
+        Chapter21Test.testCPU(src,"riscv"    , "SystemV",44,null);
         Chapter21Test.testCPU(src,"arm"      , "SystemV",26,null);
         //assertEquals("Hello World!\n", Eval2.eval(code, 0, 10000));
     }
diff --git a/chapter21/src/test/java/com/seaofnodes/simple/evaluator/Scheduler.java b/chapter21/src/test/java/com/seaofnodes/simple/evaluator/Scheduler.java
index 4bfa7628..7ba9272f 100644
--- a/chapter21/src/test/java/com/seaofnodes/simple/evaluator/Scheduler.java
+++ b/chapter21/src/test/java/com/seaofnodes/simple/evaluator/Scheduler.java
@@ -248,20 +248,20 @@ public class Scheduler {
             assert data.block != null;
             assert data.users == 0;
 
-            if (data.node instanceof LoadNode l) {
+            if (data.node instanceof MemOpNode l && l._isLoad) {
                 assert isValid(data);
                 // Handle anti-deps of load nodes.
                 // At this point all anti-dep nodes are scheduled,
                 // but they did not refine the placement
                 // so do that now.
-                var mem = l.in(1);
+                var mem = data.node.in(1);
                 for(var out : mem._outputs) {
                     if (out instanceof PhiNode p) {
                         var r = p.in(0);
                         for (int i = 1; i < p.nIns(); i++) {
                             if (p.in(i) == mem) optionalRefinePlacement(data, r.in(i));
                         }
-                    } else if (!(out instanceof LoadNode)) {
+                    } else if (!(out instanceof MemOpNode ld && ld._isLoad)) {
                         optionalRefinePlacement(data, out);
                     }
                 }
@@ -273,11 +273,11 @@ public class Scheduler {
             for(var in:data.node._inputs) {
                 if (in!=null) update(d(in), data.block);
             }
-            if (data.node instanceof StoreNode s) {
+            if (data.node instanceof MemOpNode s && !s._isLoad) {
                 // Store nodes have anti-deps to load nodes.
                 // So decrease the uses of these loads when the store is placed.
-                for (var out: s.in(1)._outputs) {
-                    if (out instanceof LoadNode) od(out).ifPresent(this::decUsers);
+                for (var out: ((Node)s).in(1)._outputs) {
+                    if (out instanceof MemOpNode ld && ld._isLoad) od(out).ifPresent(this::decUsers);
                 }
             }
         }
@@ -471,14 +471,14 @@ public class Scheduler {
             } else {
                 for (var in : node._inputs) if (in != null && !(in instanceof CFGNode)) markAlive(dataQueue, in, false);
             }
-            if (node instanceof StoreNode) mem.push(data);
+            if (node instanceof MemOpNode s && !s._isLoad) mem.push(data);
         }
         // Handle store nodes and increase load with an anti-dep to the store.
         while (!mem.isEmpty()) {
             var data = mem.pop();
             node = data.node;
             for(var out:node.in(1)._outputs) {
-                if (out instanceof LoadNode) od(out).ifPresent(d->d.users++);
+                if (out instanceof MemOpNode ld && ld._isLoad) od(out).ifPresent(d->d.users++);
             }
         }
     }