Chaining Conditionals and SCCP #230

cliffclick · 2025-09-20T17:59:55Z

Add chained conditional/relational tests.
Add SCCP.
This is an extension of
#229
and is co-authored with @Hels15

Collect callee and caller save masks (inverts of each other, except for spills). Reorg layout to be closer to each other; remove dead mask definitions; add final keyword

Callee-save registers and masks and inserting LRGs for same and spilling and picking good spills. Insert CalleeSaveNodes and edges to RetXXXs and RegMasks for those edges. Iterators for regmask. Better printing post-allocation

All regs available arm, risc. MergeSort compiles & runs

No flags on risc5. Branch takes 2 inputs (one can be imm12). Float compare only sets a GPR to 0/1. Bool has a isFloat Common risc5 imm-form handling

Missing lots of stuff still

Function sigs to calls allow wide ints

A series of minor fixes, generally making transfer functions monotonic, or p[reserving sanity for error test cases.

Most tests pass, a few failures remain. Split Opto code into a seperate file. Improve `compute` precision in several places showing up during opto. bugfix: "split-thru-phis" on Parms changes calling conventions. Not ready for that yet.

Handle non-default main. Simplify trivial inlining cutout test. ParmNode excludes most Phi opts Region idom excludes dead paths RPC now interns

Float math handles high types monotonically Minus node does not reset int widen. Phi widen handles non-int mintype Dead returns do not error complain. Better handling default vs dead main

Copilot

Pull Request Overview

Introduces chained relational/conditional parsing and an interprocedural SCCP (Sparse Conditional Constant Propagation) optimization pass, along with supporting type‑system widening and loop-phi widening mechanics. Key changes also refactor float arithmetic nodes into a shared base and add infrastructure for interprocedural call graph linking during SCCP.

Added parsing and tests for chained relational/equality expressions.
Implemented interprocedural SCCP (new Opto phase) with type widening (integers, floats, loop phis) and call graph linking.
Refactored float arithmetic nodes and expanded type lattice capabilities (widening, high forms, memory/struct helpers).

Reviewed Changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
stacked_r_12x.c / stacked_r_13x.c	Added small C harnesses for new stacked relational tests.
Eval2.java	Graceful handling of previously unimplemented constant pointer cases.
Chapter24Test.java	Added tests for chained conditionals and SCCP (one naming/spelling issue).
Chapter19/18 Tests	Adjusted expected outputs after SCCP/type changes.
BrainFuckTest.java	Simplified program initialization; updated register references.
TypeStruct/TypeMemPtr/TypeInteger/...	Added high/widening helpers and integer widen tracking.
TypeRPC.java	Added freelist allocation; made fields mutable (pooling support).
Multiple type classes	Minor correctness / equality / lattice adjustments.
Arithmetic float nodes (AddF/MulF/SubF/DivF)	Unified via new ArithFNode.
Parser.java	Added chained comparison parsing & SCCP-related casting/widening tweaks.
Opto.java	New interprocedural SCCP driver and call graph linking/unlinking.
CodeGen / IterPeeps / Node / FunNode / PhiNode	Infrastructure to support optimistic analysis and widening.
README.md / TypeAnalysis.md	New chapter documentation; some documentation issues introduced.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-20T18:36:57Z

chapter24/src/test/java/com/seaofnodes/simple/Chapter24Test.java

    };

+    @Test
+    public void testSSCP() {


[nitpick] Method name testSSCP likely intends to reference SCCP (Sparse Conditional Constant Propagation); consider renaming to testSCCP for correctness and consistency with terminology.

Suggested change

public void testSSCP() {

public void testSCCP() {

Copilot · 2025-09-20T18:36:58Z

chapter24/src/main/java/com/seaofnodes/simple/node/StoreNode.java

        Parser.ParseException err = super.err();
        if( err != null ) return err;
+        if( ptr()._type == Type.TOP )
+            return null; // This means we have an error input, report elsewher


Typo in comment: "elsewher" should be "elsewhere".

Suggested change

return null; // This means we have an error input, report elsewher

return null; // This means we have an error input, report elsewhere

Copilot · 2025-09-20T18:36:58Z

chapter24/README.md

+Expressions that mix opposite directions of comparison, like using both 
+`<=` and `=`, are not allowed, because they create ambiguous logic.


Multiple documentation issues: (1) Line 40 likely meant >= instead of = when describing opposite directions; (2) Line 48 missing the word "valid" ("are only valid if..."); (3) Line 55 should use == instead of assignment =; suggest correcting to avoid reader confusion.

Copilot · 2025-09-20T18:36:58Z

chapter24/README.md

+### Operator direction rules
+Stacked comparisons are *only* if all the comparison operators


Multiple documentation issues: (1) Line 40 likely meant >= instead of = when describing opposite directions; (2) Line 48 missing the word "valid" ("are only valid if..."); (3) Line 55 should use == instead of assignment =; suggest correcting to avoid reader confusion.

Copilot · 2025-09-20T18:36:58Z

chapter24/README.md

 - `>` with `>`
 - `>` with `>=`

+All operators above can be combined with `=` and `!=`.


Multiple documentation issues: (1) Line 40 likely meant >= instead of = when describing opposite directions; (2) Line 48 missing the word "valid" ("are only valid if..."); (3) Line 55 should use == instead of assignment =; suggest correcting to avoid reader confusion.

Copilot · 2025-09-20T18:36:59Z

chapter24/src/main/java/com/seaofnodes/simple/node/Node.java

+    public void insertDef(int idx, Node new_def) {
+        _inputs.insert(null,idx);
+        setDef(idx,new_def);


[nitpick] insertDef formerly returned the Node but now returns void; if any existing internal or external callers relied on method chaining or the return value, this is a silent breaking API change—consider either restoring the return value or auditing all usages to ensure no regressions.

Suggested change

public void insertDef(int idx, Node new_def) {

_inputs.insert(null,idx);

setDef(idx,new_def);

public Node insertDef(int idx, Node new_def) {

_inputs.insert(null,idx);

return setDef(idx,new_def);

Copilot · 2025-09-20T18:36:59Z

chapter24/src/main/java/com/seaofnodes/simple/type/TypeConAry.java


    @Override public boolean isHigh() { return this==TOP; }
    @Override boolean _isConstant() { return true; }
+    Type _glb(boolean mem) { return this; }


[nitpick] Method _glb overrides a superclass method but lacks an @OverRide annotation (present on _isGLB); adding @OverRide improves readability and helps catch signature mismatches.

Suggested change

Type _glb(boolean mem) { return this; }

@Override Type _glb(boolean mem) { return this; }

Copilot · 2025-09-20T18:36:59Z

chapter24/README.md

 - `>` with `>`
 - `>` with `>=`

+All operators above can be combined with `=` and `!=`.


Single equals = is assignment in most languages; for equality the text should use ==. Recommend: "All operators above can be combined with == and !=."

Suggested change

All operators above can be combined with `=` and `!=`.

All operators above can be combined with `==` and `!=`.

Copilot

Pull Request Overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated 4 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-22T16:55:56Z

chapter24/src/main/java/com/seaofnodes/simple/node/cpus/arm/NotARM.java

        int subs = arm.imm_inst(arm.OP_SUBS, 0, reg1, self);
        enc.add4(subs);
-        int cset = arm.cond_set(arm.OP_CSET, 31, arm.COND.EQ, 63, reg1);
+        int cset = arm.cond_set(arm.OP_CSET, 31, arm.COND.EQ, 31, reg1);


The fourth parameter should be 63 (xzr register) instead of 31. ARM64 uses register 31 as the zero register (wzr/xzr) and the instruction expects the destination register in the fourth parameter, not the condition register.

Suggested change

int cset = arm.cond_set(arm.OP_CSET, 31, arm.COND.EQ, 31, reg1);

int cset = arm.cond_set(arm.OP_CSET, 31, arm.COND.EQ, self, reg1);

Copilot · 2025-09-22T16:55:57Z

chapter24/src/main/java/com/seaofnodes/simple/node/ConFldOffNode.java

    @Override public boolean eq(Node n) {
        ConFldOffNode off = (ConFldOffNode)n; // Invariant
-        return _name==off._name && _fname==off._fname && super.eq(n);
+        return _name.equals(off._name) && _fname.equals(off._fname) && super.eq(n);


String comparison using == was changed to equals(), but this could cause NullPointerException if _name or _fname is null. Consider using Objects.equals() for null-safe comparison.

Copilot · 2025-09-22T16:55:57Z

chapter24/src/main/java/com/seaofnodes/simple/node/Node.java

+    public void insertDef(int idx, Node new_def) {
+        _inputs.insert(null,idx);
+        setDef(idx,new_def);


Method signature changed from returning Node to void, but callers might expect the fluent interface pattern. Consider returning 'this' to maintain chainability.

Suggested change

public void insertDef(int idx, Node new_def) {

_inputs.insert(null,idx);

setDef(idx,new_def);

public Node insertDef(int idx, Node new_def) {

_inputs.insert(null,idx);

setDef(idx,new_def);

return this;

Copilot · 2025-09-22T16:55:58Z

chapter24/src/main/java/com/seaofnodes/simple/type/TypeInteger.java

-    public static TypeInteger make(long lo, long hi) {
-        TypeInteger i = malloc(lo,hi);
+    private TypeInteger(long min, long max, byte widen) { super(TINT); init(min,max,widen); }
+    private TypeInteger init(long min, long max, byte widen) {


[nitpick] The assertion assumes constants always have zero widen, but this constraint isn't clearly documented. Consider adding a comment explaining why constants can't be widened.

Suggested change

private TypeInteger init(long min, long max, byte widen) {

private TypeInteger init(long min, long max, byte widen) {

// If min == max, this TypeInteger represents a constant value.

// Constants must always have a widen value of zero, because widening is only meaningful

// for ranges of values, not for single, fixed values. Allowing a widened constant would

// break assumptions elsewhere in the type system, where wideness is used to represent

// uncertainty or generalization over a range, not a specific value.

cliffclick · 2025-09-22T17:03:57Z

Approved in discord by @Hels15

cliffclick

Minor typos

Hels15 and others added 30 commits March 2, 2025 15:31

merge fixup

c44020e

More x86,risc,arm call convention fixes

4cf2db1

Collect callee and caller save masks (inverts of each other, except for spills). Reorg layout to be closer to each other; remove dead mask definitions; add final keyword

lui fix up with 32 bit immediates

fc53d38

Callee-save registers

f12f398

Callee-save registers and masks and inserting LRGs for same and spilling and picking good spills. Insert CalleeSaveNodes and edges to RetXXXs and RegMasks for those edges. Iterators for regmask. Better printing post-allocation

RegMask supports 128 bits

105f8f4

All regs available arm, risc. MergeSort compiles & runs

added flags splitting

fda8390

Fix risc5 compares

8c816a6

No flags on risc5. Branch takes 2 inputs (one can be imm12). Float compare only sets a GPR to 0/1. Bool has a isFloat Common risc5 imm-form handling

extend test cases

8f1b19e

merge again

ed8fe7f

imm encoding, flags fixed

27f4d23

grep for Todo: relocs

c3afdc7

float riscv fixed

f906be6

flags for set

96fd744

lui fix for small bits [0, 12]

ce06c50

arm init

c85071d

arm last touches

8a10715

fmov still needs fix

2869c7c

minor fixes

190f172

minor fixes(2)

aea1382

got rid of unnecessary stuff

c5cba4d

create ch21

ae344b3

merged

2c727b2

remove encodings from chapter20 dir

e88b6fa

1/2 pass cleaning up ARM encoding API

1de2f40

ARM encoding compiles

9dc958b

Risc R_TYPE and I_TYPE

dca627d

Progress RISC encodings

80ccea9

Missing lots of stuff still

progress risc encoding

883df85

risc encodings compile

1929e93

Update README.md

9a4b0b7

cliffclick added 17 commits September 7, 2025 09:48

push mid-progress

fc6d904

progress sccp

50382da

Make all the default parser integer types pre-widen

433c245

Function sigs to calls allow wide ints

progress sccp

479c27a

Update TypeInteger.java

6d612f2

progress SCCP

2045d5b

Progress SCCP

51b38bb

Progress SCCP

d391519

A series of minor fixes, generally making transfer functions monotonic, or p[reserving sanity for error test cases.

Progress on opto

1d00586

Most tests pass, a few failures remain. Split Opto code into a seperate file. Improve `compute` precision in several places showing up during opto. bugfix: "split-thru-phis" on Parms changes calling conventions. Not ready for that yet.

most tests pass

c023e6e

Handle non-default main. Simplify trivial inlining cutout test. ParmNode excludes most Phi opts Region idom excludes dead paths RPC now interns

Fix arm NOT encoding

c4d6789

all tests pass

6177c9b

Float math handles high types monotonically Minus node does not reset int widen. Phi widen handles non-int mintype Dead returns do not error complain. Better handling default vs dead main

If NewNode is high, produce high results

30eebb5

BrainFuck uses a string

fefcb0c

Merge branch 'main' into cc_pr222

58aeba3

README

93e7565

Update README.md

bada461

cliffclick requested a review from Copilot September 20, 2025 18:34

Copilot AI reviewed Sep 20, 2025

View reviewed changes

cliffclick added 4 commits September 20, 2025 15:07

Typos from copilot review

25f8042

monotonic fix MinusF

27c3cb3

Merge branch 'main' into cc_pr222

8fcb742

Update TypeAnalysis.md

020731b

cliffclick requested a review from Copilot September 22, 2025 16:54

Copilot AI reviewed Sep 22, 2025

View reviewed changes

Update TypeInteger.java

e09dd33

cliffclick requested a review from Hels15 September 22, 2025 17:03

cliffclick commented Sep 22, 2025

View reviewed changes

cliffclick merged commit e5acc8a into main Sep 22, 2025
2 checks passed

	return null; // This means we have an error input, report elsewher
	return null; // This means we have an error input, report elsewhere

		Expressions that mix opposite directions of comparison, like using both
		`<=` and `=`, are not allowed, because they create ambiguous logic.

		### Operator direction rules
		Stacked comparisons are only if all the comparison operators

	Type _glb(boolean mem) { return this; }
	@Override Type _glb(boolean mem) { return this; }

	All operators above can be combined with `=` and `!=`.
	All operators above can be combined with `==` and `!=`.

	int cset = arm.cond_set(arm.OP_CSET, 31, arm.COND.EQ, 31, reg1);
	int cset = arm.cond_set(arm.OP_CSET, 31, arm.COND.EQ, self, reg1);

-    private TypeInteger init(long min, long max, byte widen) {
+    private TypeInteger init(long min, long max, byte widen) {
+        // If min == max, this TypeInteger represents a constant value.
+        // Constants must always have a widen value of zero, because widening is only meaningful
+        // for ranges of values, not for single, fixed values. Allowing a widened constant would
+        // break assumptions elsewhere in the type system, where wideness is used to represent
+        // uncertainty or generalization over a range, not a specific value.

Chaining Conditionals and SCCP #230

Chaining Conditionals and SCCP #230

Uh oh!

Conversation

cliffclick commented Sep 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

cliffclick commented Sep 22, 2025

Uh oh!

cliffclick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cliffclick commented Sep 20, 2025 •

edited

Loading