From b6cbd3d7b7698922e60e2c3eadadab51a430efe1 Mon Sep 17 00:00:00 2001
From: Cliff Click <cliffc@acm.org>
Date: Sat, 15 Mar 2025 12:06:11 -0700
Subject: Progress towards no crashes

Multi-part constant expansion was buggy when splitting down into functions.
Improved printing for RISC multi-part constants.
---
 .../src/main/java/com/seaofnodes/simple/SB.java    | 24 +++++-----
 .../com/seaofnodes/simple/node/ConstantNode.java   |  2 +-
 .../seaofnodes/simple/node/cpus/arm/MemOpARM.java  |  2 +-
 .../simple/node/cpus/riscv/AddIRISC.java           | 11 +++--
 .../seaofnodes/simple/node/cpus/riscv/ImmRISC.java | 21 ++++++--
 .../com/seaofnodes/simple/node/cpus/riscv/LUI.java | 10 ++--
 .../seaofnodes/simple/node/cpus/riscv/NewRISC.java |  4 +-
 .../simple/node/cpus/riscv/StoreRISC.java          |  2 +-
 .../seaofnodes/simple/node/cpus/riscv/riscv.java   | 30 +++---------
 .../com/seaofnodes/simple/print/ASMPrinter.java    | 56 +++++++++-------------
 .../java/com/seaofnodes/simple/Chapter20Test.java  |  2 +-
 11 files changed, 79 insertions(+), 85 deletions(-)

diff --git a/chapter21/src/main/java/com/seaofnodes/simple/SB.java b/chapter21/src/main/java/com/seaofnodes/simple/SB.java
index 72649a39..31fbb7d4 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/SB.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/SB.java
@@ -29,23 +29,25 @@ public final class SB {
   public SB p( int    s ) { _sb.append(s); return this; }
   public SB p( long   s ) { _sb.append(s); return this; }
   public SB p( boolean s) { _sb.append(s); return this; }
-  // 2 hex digits
-  public SB hex2(int s) {
+  // 1 byte, 2 hex digits, 8 bits
+  public SB hex1(int s) {
     int digit = (s>>4) & 0xf;
     _sb.append((char)((digit <= 9 ? '0' : ('A'-10))+digit));
     digit = s & 0xf;
     _sb.append((char)((digit <= 9 ? '0' : ('A'-10))+digit));
     return this;
   }
-  // 4 hex digits
-  public SB hex4(int s) {
-    assert (s>>4*4)==0; // Fits in 16 bits
-    for( int i=0; i<4; i++ ) {
-      int digit = (s>>((3-i)*4)) & 0xf;
-      _sb.append((char)((digit <= 9 ? '0' : ('A'-10))+digit));
-    }
-    return this;
-  }
+  // 2 bytes, 4 hex digits, 16 bits, Big Endian
+  public SB hex2(int s) { return hex1(s>> 8).hex1(s); }
+  // 4 bytes, 8 hex digits, 32 bits, Big Endian
+  public SB hex4(int s) { return hex2(s>>16).hex2(s); }
+  //public SB hex4(int s) {
+  //for( int i=0; i<8; i++ ) {
+  //  int digit = (s>>((7-i)*4)) & 0xf;
+  //  _sb.append((char)((digit <= 9 ? '0' : ('A'-10))+digit));
+  //}
+  //return this;
+  //
   // Fixed width field
   public SB fix( int sz, String s ) {
     for( int i=0; i<sz; i++ )
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/ConstantNode.java b/chapter21/src/main/java/com/seaofnodes/simple/node/ConstantNode.java
index f7ebc006..699ee76f 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/ConstantNode.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/ConstantNode.java
@@ -48,7 +48,7 @@ public class ConstantNode extends Node {
             if( fun._name != null )
                 return sb.append("{ ").append(fun._name).append("}");
         }
-        return sb.append(_con.print(new SB()));
+        return sb.append(_con==null ? "---" : _con.print(new SB()));
     }
 
     @Override public boolean isConst() { return true; }
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 0d49d025..922276d4 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
@@ -40,7 +40,7 @@ public abstract class MemOpARM extends MemOpNode implements MachNode {
     // Wider mask to store both GPRs and FPRs
     @Override public RegMask regmap(int i) {
         // 0 - ctrl
-        // 1 - memory
+        if( i==1 ) return null; // memory
         if( i==2 ) return arm.RMASK; // ptr/base
         if( i==3 ) return arm.RMASK; // off/index
         if( i==4 ) return arm.RMASK; // value
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/AddIRISC.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/AddIRISC.java
index fce8c499..bdd851ec 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/AddIRISC.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/AddIRISC.java
@@ -1,12 +1,17 @@
 package com.seaofnodes.simple.node.cpus.riscv;
 
+import com.seaofnodes.simple.SB;
 import com.seaofnodes.simple.Utils;
+import com.seaofnodes.simple.codegen.*;
 import com.seaofnodes.simple.node.Node;
+import com.seaofnodes.simple.type.TypeInteger;
 
 public class AddIRISC extends ImmRISC {
-    AddIRISC( Node add, int imm ) { super(add,imm); }
+    // Used to inst-selection as a direct match against an ideal Add/Sub
+    AddIRISC( Node add, int imm, boolean pop ) { super(add,imm,pop); }
+    @Override public String op() { return "addi"; }
+    @Override public String glabel() { return "+"; }
     @Override int opcode() {  return riscv.I_TYPE; }
     @Override int func3() {return 0;}
-    @Override public String glabel() { return "+"; }
-    @Override public String op() { return "addi"; }
+    @Override public AddIRISC copy() { return new AddIRISC(this,_imm12,false); }
 }
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/ImmRISC.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/ImmRISC.java
index 825d9449..4e73aeab 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/ImmRISC.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/ImmRISC.java
@@ -3,7 +3,7 @@ package com.seaofnodes.simple.node.cpus.riscv;
 import com.seaofnodes.simple.*;
 import com.seaofnodes.simple.codegen.*;
 import com.seaofnodes.simple.node.*;
-
+import com.seaofnodes.simple.type.TypeInteger;
 import java.util.BitSet;
 
 abstract public class ImmRISC extends MachConcreteNode implements MachNode {
@@ -11,15 +11,27 @@ abstract public class ImmRISC extends MachConcreteNode implements MachNode {
     ImmRISC( Node add, int imm12, boolean pop ) {
         super(add);
         _imm12 = imm12;
+        // If pop, we are moving from a ideal Node to MachNode; the ideal Node
+        // inputs got copied here, but we are absorbing the immediate into the
+        // MachNode and so do not want the constant input.
         if( pop ) _inputs.pop();
+        // if NOT pop, we are the 2nd-half of a 2-part expansion, reset the
+        // input edges to no-control
+        else { _inputs.set(0,null); _inputs.setX(1,add); }
     }
     ImmRISC( Node add, int imm12 ) { this(add,imm12,true); }
     @Override public RegMask regmap(int i) { return riscv.RMASK; }
     @Override public RegMask outregmap() { return riscv.WMASK; }
+    @Override public boolean isClone() {
+        // Allow cloning of the 2nd half of a 2-part constant; this might
+        // trigger cloning the first part also.
+        return in(1) instanceof LUI || in(1) instanceof AUIPC;
+    }
 
     @Override public StringBuilder _print1(StringBuilder sb, BitSet visited) {
+        int imm12 = (_imm12<<20)>>20; // Sign extend 12 bits
         in(1)._print0(sb.append("( "), visited);
-        return sb.append(String.format(" %s #%d )",glabel(),_imm12));
+        return sb.append(String.format(" %s #%d )",glabel(),imm12));
     }
 
     abstract int opcode();
@@ -35,6 +47,9 @@ abstract public class ImmRISC extends MachConcreteNode implements MachNode {
 
     // General form: "addi  rd = rs1 + imm"
     @Override public void asm(CodeGen code, SB sb) {
-        sb.p(code.reg(this)).p(" = ").p(code.reg(in(1))).p(" ").p(glabel()).p(" #").p(_imm12);
+        int imm12 = (_imm12<<20)>>20; // Sign extend 12 bits
+        sb.p(code.reg(this)).p(" = ").p(code.reg(in(1))).p(" ").p(glabel()).p(" #").p(imm12);
+        if( in(1) instanceof LUI lui )
+            sb.p(" // #").hex4((int)(((TypeInteger)lui._con).value()) + imm12);
     }
 }
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/LUI.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/LUI.java
index 38e7e4b1..df0d6b4b 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/LUI.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/LUI.java
@@ -8,14 +8,14 @@ import com.seaofnodes.simple.type.TypeInteger;
 
 // Load upper 20bits.
 public class LUI extends ConstantNode implements MachNode {
-    LUI( TypeInteger ti ) {
-        super(ti);
-        assert riscv.imm20Exact(ti);
+    LUI( int imm20 ) {
+        super(TypeInteger.constant(imm20));
+        assert riscv.imm20Exact((TypeInteger)_con);
     }
     @Override public RegMask regmap(int i) { return null; }
     @Override public RegMask outregmap() { return riscv.WMASK; }
     @Override public boolean isClone() { return true; }
-    @Override public LUI copy() { return new LUI((TypeInteger)_con); }
+    @Override public LUI copy() { return new LUI((int)((TypeInteger)_con).value()); }
     @Override public String op() { return "lui"; }
     @Override public void encoding( Encoding enc ) {
         long x = ((TypeInteger)_con).value();
@@ -27,6 +27,6 @@ public class LUI extends ConstantNode implements MachNode {
 
     @Override public void asm(CodeGen code, SB sb) {
         String reg = code.reg(this);
-        _con.print(sb.p(reg).p(" = #"));
+        sb.p(reg).p(" = #").hex4((int)(((TypeInteger)_con).value()));
     }
 }
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/NewRISC.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/NewRISC.java
index 9b6eb98e..7002e0a8 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/NewRISC.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/NewRISC.java
@@ -32,8 +32,8 @@ public class NewRISC extends NewNode implements MachNode {
         enc.add4(body);
     }
 
-    // General form: "alloc #bytes"
+    // General form: "alloc #bytes  PC"
     @Override public void asm(CodeGen code, SB sb) {
-        sb.p(code.reg(size()));
+        sb.p(code.reg(size())).p(" ").p(code.reg(in(nIns()-1)));
     }
 }
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 b78eac1f..383e37b5 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
@@ -23,7 +23,7 @@ public class StoreRISC extends MemOpRISC {
     @Override public String op() { return "st"+_sz; }
     @Override public RegMask regmap(int i) {
         // 0 - ctrl
-        // 1 - mem
+        if( i==1 ) return null; // mem
         if( i==2 ) return riscv.RMASK; // ptr
         // 2 - index
         if( i==4 ) return riscv.MEM_MASK; // Wide mask to store GPR and FPR
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/riscv.java b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/riscv.java
index 16b09172..fcea5e28 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/riscv.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/node/cpus/riscv/riscv.java
@@ -202,24 +202,6 @@ public class riscv extends Machine {
         };
     }
 
-    public static void push_4_bytes(int value, ByteArrayOutputStream bytes) { throw Utils.TODO(); }
-
-//    public static void print_as_hex(ByteArrayOutputStream outputStream) {
-//        StringBuilder hexString = new StringBuilder();
-//        for (byte b : outputStream.toByteArray()) {
-//            hexString.append(String.format("%02X", b));  // Format as uppercase hex without space
-//        }
-//        System.out.println(hexString.toString());
-//    }
-
-//    // rs1 - rs2
-//    public static int r_source(int rs1, int rs2) {
-//        return (rs2 << 4) | rs1;
-//    }
-//    public static int r_func7(int f) {
-//        return f & 7;
-//    }
-
     static RegMask callInMask( TypeFunPtr tfp, int idx ) {
         if( idx==0 ) return RPC_MASK;
         if( idx==1 ) return null;
@@ -363,7 +345,7 @@ public class riscv extends Machine {
 
     private Node add(AddNode add) {
         if( add.in(2) instanceof ConstantNode off && off._con instanceof TypeInteger ti && imm12(ti) )
-            return new AddIRISC(add, (int)ti.value());
+            return new AddIRISC(add, (int)ti.value(),true);
         return new AddRISC(add);
     }
 
@@ -427,13 +409,15 @@ public class riscv extends Machine {
         return switch( con._con ) {
         case TypeInteger ti -> {
             if( imm12(ti) ) yield new IntRISC(con);
-            if( imm20Exact(ti) ) yield new LUI(ti);
             long x = ti.value();
-            if( (x<<32)>>32 == x ) // Signed lower 32-bit immediate
+            if( imm20Exact(ti) ) yield new LUI((int)x);
+            if( (x<<32)>>32 == x ) { // Signed lower 32-bit immediate
                 // Here, the low 12 bits get sign-extended, which means if
                 // bit11 is set, the value is negative and lowers the LUI
                 // value.  Add a bit 12 to compensate
-                yield new AddIRISC(new LUI(((x>>11)&1)==1 ? TypeInteger.constant(x+0x1000) : ti), (int)(x & 0xFFF));
+                if( ((x>>11)&1)==1 ) x += 0x1000;
+                yield new AddIRISC(new LUI((int)(x & ~0xFFF)), (int)(x & 0xFFF),false);
+            }
             // Need more complex sequence for larger constants... or a load
             // from a constant pool, which does not need an extra register
             throw Utils.TODO();
@@ -489,7 +473,7 @@ public class riscv extends Machine {
 
     private Node sub(SubNode sub) {
         return sub.in(2) instanceof ConstantNode con && con._con instanceof TypeInteger ti && imm12(ti) && imm12(ti)
-            ? new AddIRISC(sub, (int)(-ti.value()))
+            ? new AddIRISC(sub, (int)(-ti.value()),true)
             : new SubRISC(sub);
     }
 
diff --git a/chapter21/src/main/java/com/seaofnodes/simple/print/ASMPrinter.java b/chapter21/src/main/java/com/seaofnodes/simple/print/ASMPrinter.java
index 77d891a2..55a45a32 100644
--- a/chapter21/src/main/java/com/seaofnodes/simple/print/ASMPrinter.java
+++ b/chapter21/src/main/java/com/seaofnodes/simple/print/ASMPrinter.java
@@ -48,44 +48,32 @@ public abstract class ASMPrinter {
         if( bb instanceof CallNode ) return iadr;
         final boolean postAlloc = code._phase.ordinal() > CodeGen.Phase.RegAlloc.ordinal();
 
-        // Count Phis
-        int nPhi=0;
-        for( ; nPhi<bb.nOuts(); nPhi++ )
-            if( !(bb.out(nPhi) instanceof PhiNode) )
-                break;
-
-        if( nPhi>0 ) {
-            // Post- RegAlloc phi prints all on one line
+        boolean once=false;
+        for( Node n : bb.outs() ) {
+            if( !(n instanceof PhiNode phi) ) continue;
+            if( phi._type instanceof TypeMem || phi._type instanceof TypeRPC ) continue; // Nothing for the hidden ones
+            // Post-RegAlloc phi prints all on one line
             if( postAlloc ) {
-                sb.fix(4," ").p(" ").fix(encWidth,"").p("  ");
-                for( int i=0; i<nPhi; i++ ) {
-                    PhiNode phi = (PhiNode)bb.out(i);
-                    if( !(phi._type instanceof TypeMem || phi._type instanceof TypeRPC) ) // Nothing for the hidden ones
-                        sb.p(phi._label).p(':').p(code.reg(phi)).p(',');
-                }
-                sb.unchar().nl();
-
+                if( !once ) { once=true; sb.fix(4," ").p(" ").fix(encWidth,"").p("  "); }
+                sb.p(phi._label).p(':').p(code.reg(phi)).p(',');
             } else {
-                // Pre- RegAlloc phi prints one line per
-                for( int j=0; j<nPhi; j++ ) {
-                    PhiNode phi = (PhiNode)bb.out(j);
-                    if( phi._type instanceof TypeMem || phi._type instanceof TypeRPC ) continue; // Nothing for the hidden ones
-                    sb.fix(4," ").p(" ").fix(encWidth,"").p("  ").fix(opWidth,phi._label).p(" ");
-                    sb.p(code.reg(phi));
-                    if( !(phi instanceof ParmNode) ) {
-                        sb.p(" = phi( ");
-                        for( int i=1; i<phi.nIns(); i++ )
-                            sb.p("N").p(phi.in(i)._nid).p(",");
-                        sb.unchar().p(" )");
-                    }
-                    sb.nl();
+                // Pre-RegAlloc phi prints one line per
+                sb.fix(4," ").p(" ").fix(encWidth,"").p("  ").fix(opWidth,phi._label).p(" ").p(code.reg(phi));
+                if( phi.getClass() == PhiNode.class ) {
+                    sb.p(" = phi( ");
+                    for( int i=1; i<phi.nIns(); i++ )
+                        sb.p("N").p(phi.in(i)._nid).p(",");
+                    sb.unchar().p(" )");
                 }
+                sb.nl();
             }
         }
+        if( once ) sb.unchar().nl();
 
         // All the non-phis
-        for( int i=nPhi; i<bb.nOuts(); i++ )
-            iadr = doInst(iadr, sb,code, cfgidx, bb.out(i),postAlloc );
+        for( int i=0; i<bb.nOuts(); i++ )
+            if( !(bb.out(i) instanceof PhiNode) )
+                iadr = doInst(iadr, sb,code, cfgidx, bb.out(i),postAlloc );
 
         return iadr;
     }
@@ -106,7 +94,7 @@ public abstract class ASMPrinter {
                     break;      // Has code in the block, need to jump around
                 // No code in the block, can fall through it
             }
-            sb.hex4(iadr++).p(" ").fix(encWidth,"??").p("  ").fix(opWidth,"JMP").p(" ").fix(argWidth,label(cfg)).nl();
+            sb.hex2(iadr++).p(" ").fix(encWidth,"??").p("  ").fix(opWidth,"JMP").p(" ").fix(argWidth,label(cfg)).nl();
             return iadr;
         }
 
@@ -120,14 +108,14 @@ public abstract class ASMPrinter {
 
         // ADDR ENCODING  Op--- dst = src op src       // Comment
         // 1234 abcdefgh  ld4   RAX = [Rbase + off]    // Comment
-        sb.hex4(iadr);
+        sb.hex2(iadr);
         sb.p(" ");
 
         // Encoding
         if( code._encoding != null ) {
             int size = code._encoding._opLen[n._nid];
             for( int i=0; i<size; i++ )
-                sb.hex2(code._encoding._bits.buf()[iadr++]);
+                sb.hex1(code._encoding._bits.buf()[iadr++]);
             for( int i=size*2; i<encWidth; i++ )
                 sb.p(" ");
         } else
diff --git a/chapter21/src/test/java/com/seaofnodes/simple/Chapter20Test.java b/chapter21/src/test/java/com/seaofnodes/simple/Chapter20Test.java
index 7f5cabc4..a18822c2 100644
--- a/chapter21/src/test/java/com/seaofnodes/simple/Chapter20Test.java
+++ b/chapter21/src/test/java/com/seaofnodes/simple/Chapter20Test.java
@@ -10,6 +10,7 @@ import static org.junit.Assert.*;
 
 public class Chapter20Test {
 
+    //-3834621514626535010L
     @Test
     public void testJig() {
         CodeGen code = new CodeGen("return 0;");
@@ -23,7 +24,6 @@ public class Chapter20Test {
         code.parse().opto().typeCheck().instSelect(PORTS,cpu,os).GCM().localSched().regAlloc();
         int delta = spills>>3;
         if( delta==0 ) delta = 1;
-        code.asm();
         assertEquals("Expect spills:",spills,code._regAlloc._spillScaled,delta);
         if( stop != null )
             assertEquals(stop, code._stop.toString());
-- 
cgit v1.2.3