From 471f33fcd0b78361a71d7aa32dfc047592ed4e24 Mon Sep 17 00:00:00 2001 From: Mattias Hansson Date: Sat, 1 Aug 2026 02:01:19 +0200 Subject: [PATCH] Fixed bug in BYTE REGISTER. Made POINTER use one register --- internal/commands/point.go | 12 +- internal/commands/point_test.go | 131 +++++++++++++++++ internal/optimizer/optimizer_test.go | 203 +++++++++++++++++++++++++++ internal/optimizer/pass_regdead.go | 40 ++++-- 4 files changed, 369 insertions(+), 17 deletions(-) create mode 100644 internal/commands/point_test.go diff --git a/internal/commands/point.go b/internal/commands/point.go index a4e83a2..4249f80 100644 --- a/internal/commands/point.go +++ b/internal/commands/point.go @@ -106,29 +106,29 @@ func (c *PointerCommand) Generate(ctx *compiler.CompilerContext) ([]string, erro // Label reference if c.isLabel { - asm = append(asm, fmt.Sprintf("\tldx #<%s", c.targetLabel)) + asm = append(asm, fmt.Sprintf("\tlda #<%s", c.targetLabel)) + asm = append(asm, fmt.Sprintf("\tsta %s", c.pointerVarName)) asm = append(asm, fmt.Sprintf("\tlda #>%s", c.targetLabel)) asm = append(asm, fmt.Sprintf("\tsta %s+1", c.pointerVarName)) - asm = append(asm, fmt.Sprintf("\tstx %s", c.pointerVarName)) return asm, nil } // Variable reference if c.isVar { - asm = append(asm, fmt.Sprintf("\tldx #<%s", c.targetVarName)) + asm = append(asm, fmt.Sprintf("\tlda #<%s", c.targetVarName)) + asm = append(asm, fmt.Sprintf("\tsta %s", c.pointerVarName)) asm = append(asm, fmt.Sprintf("\tlda #>%s", c.targetVarName)) asm = append(asm, fmt.Sprintf("\tsta %s+1", c.pointerVarName)) - asm = append(asm, fmt.Sprintf("\tstx %s", c.pointerVarName)) return asm, nil } // Numeric address - create temp label tempLabel := ctx.GeneralStack.Push() asm = append(asm, fmt.Sprintf("%s = %d", tempLabel, c.targetAddress)) - asm = append(asm, fmt.Sprintf("\tldx #<%s", tempLabel)) + asm = append(asm, fmt.Sprintf("\tlda #<%s", tempLabel)) + asm = append(asm, fmt.Sprintf("\tsta %s", c.pointerVarName)) asm = append(asm, fmt.Sprintf("\tlda #>%s", tempLabel)) asm = append(asm, fmt.Sprintf("\tsta %s+1", c.pointerVarName)) - asm = append(asm, fmt.Sprintf("\tstx %s", c.pointerVarName)) return asm, nil } diff --git a/internal/commands/point_test.go b/internal/commands/point_test.go new file mode 100644 index 0000000..7a25562 --- /dev/null +++ b/internal/commands/point_test.go @@ -0,0 +1,131 @@ +package commands + +import ( + "strings" + "testing" + + "c65gm/internal/compiler" + "c65gm/internal/preproc" +) + +func TestPointerCommand_Generate(t *testing.T) { + tests := []struct { + name string + line string + setupVars func(*compiler.SymbolTable) + wantAsm []string + }{ + { + name: "pointer to label — uses A only", + line: "POINTER ptr -> TARGET", + setupVars: func(st *compiler.SymbolTable) { + st.AddVar("ptr", "", compiler.KindWord, 0, preproc.Line{Filename: "test.c65", LineNo: 1}) + }, + wantAsm: []string{ + "\tlda #TARGET", + "\tsta ptr+1", + }, + }, + { + name: "pointer to variable — uses A only", + line: "POINTER ptr TO targetVar", + setupVars: func(st *compiler.SymbolTable) { + st.AddVar("ptr", "", compiler.KindWord, 0, preproc.Line{Filename: "test.c65", LineNo: 1}) + st.AddVar("targetVar", "", compiler.KindByte, 0, preproc.Line{Filename: "test.c65", LineNo: 1}) + }, + wantAsm: []string{ + "\tlda #targetVar", + "\tsta ptr+1", + }, + }, + { + name: "pointer to numeric address — uses A only", + line: "POINTER ptr -> 53280", + setupVars: func(st *compiler.SymbolTable) { + st.AddVar("ptr", "", compiler.KindWord, 0, preproc.Line{Filename: "test.c65", LineNo: 1}) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pragma := preproc.NewPragma() + ctx := compiler.NewCompilerContext(pragma) + tt.setupVars(ctx.SymbolTable) + + cmd := &PointerCommand{} + line := preproc.Line{ + Text: tt.line, + Kind: preproc.Source, + PragmaSetIndex: pragma.GetCurrentPragmaSetIndex(), + } + + if err := cmd.Interpret(line, ctx); err != nil { + t.Fatalf("Interpret() error = %v", err) + } + + asm, err := cmd.Generate(ctx) + if err != nil { + t.Fatalf("Generate() error = %v", err) + } + + if tt.name == "pointer to numeric address — uses A only" { + foundLo := false + foundHi := false + foundSta := false + for _, a := range asm { + if strings.Contains(a, "lda #<") { + foundLo = true + } + if strings.Contains(a, "lda #>") { + foundHi = true + } + if strings.Contains(a, "sta ptr+1") { + foundSta = true + } + } + if !foundLo || !foundHi || !foundSta { + t.Errorf("expected A-only pattern (lda #< / sta ptr / lda #> / sta ptr+1), got:\n%s", strings.Join(asm, "\n")) + } + return + } + + if !equalAsm(asm, tt.wantAsm) { + t.Errorf("Generate() mismatch\ngot:\n%s\nwant:\n%s", + strings.Join(asm, "\n"), + strings.Join(tt.wantAsm, "\n")) + } + }) + } +} + +func TestPointerCommand_NoXRegister(t *testing.T) { + pragma := preproc.NewPragma() + ctx := compiler.NewCompilerContext(pragma) + ctx.SymbolTable.AddVar("ptr", "", compiler.KindWord, 0, preproc.Line{Filename: "test.c65", LineNo: 1}) + + cmd := &PointerCommand{} + line := preproc.Line{ + Text: "POINTER ptr -> $0400", + Kind: preproc.Source, + PragmaSetIndex: pragma.GetCurrentPragmaSetIndex(), + } + + if err := cmd.Interpret(line, ctx); err != nil { + t.Fatalf("Interpret() error = %v", err) + } + + asm, err := cmd.Generate(ctx) + if err != nil { + t.Fatalf("Generate() error = %v", err) + } + + joined := strings.Join(asm, "\n") + if strings.Contains(joined, "ldx") || strings.Contains(joined, "stx") { + t.Errorf("POINTER should not use X register, got:\n%s", joined) + } +} diff --git a/internal/optimizer/optimizer_test.go b/internal/optimizer/optimizer_test.go index ec813df..164524b 100644 --- a/internal/optimizer/optimizer_test.go +++ b/internal/optimizer/optimizer_test.go @@ -866,3 +866,206 @@ func TestPassRegDead_NotGloballyDeadWithLda(t *testing.T) { t.Errorf("expected sta spill_me_bg to be kept, got:\n%s", joined) } } + +func TestPassRegDead_ReadModifyWrite(t *testing.T) { + tests := []struct { + name string + opcode string + lines []string + }{ + { + name: "dec reads stored value", + opcode: "dec", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tdec rmw_var", + "\trts", + }, + }, + { + name: "inc reads stored value", + opcode: "inc", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tinc rmw_var", + "\trts", + }, + }, + { + name: "adc reads stored value", + opcode: "adc", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tclc", + "\tadc rmw_var", + "\trts", + }, + }, + { + name: "sbc reads stored value", + opcode: "sbc", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tsec", + "\tsbc rmw_var", + "\trts", + }, + }, + { + name: "and reads stored value", + opcode: "and", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tand rmw_var", + "\trts", + }, + }, + { + name: "ora reads stored value", + opcode: "ora", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tora rmw_var", + "\trts", + }, + }, + { + name: "eor reads stored value", + opcode: "eor", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\teor rmw_var", + "\trts", + }, + }, + { + name: "cmp reads stored value", + opcode: "cmp", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tcmp rmw_var", + "\trts", + }, + }, + { + name: "ldx reads stored value", + opcode: "ldx", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tldx rmw_var", + "\trts", + }, + }, + { + name: "ldy reads stored value", + opcode: "ldy", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tldy rmw_var", + "\trts", + }, + }, + { + name: "asl reads stored value", + opcode: "asl", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tasl rmw_var", + "\trts", + }, + }, + { + name: "lsr reads stored value", + opcode: "lsr", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tlsr rmw_var", + "\trts", + }, + }, + { + name: "rol reads stored value", + opcode: "rol", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\trol rmw_var", + "\trts", + }, + }, + { + name: "ror reads stored value", + opcode: "ror", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tror rmw_var", + "\trts", + }, + }, + { + name: "bit reads stored value", + opcode: "bit", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tbit rmw_var", + "\trts", + }, + }, + { + name: "cpx reads stored value", + opcode: "cpx", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tcpx rmw_var", + "\trts", + }, + }, + { + name: "cpy reads stored value", + opcode: "cpy", + lines: []string{ + "\tlda #$05", + "\tsta rmw_var", + "\tcpy rmw_var", + "\trts", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &Config{ + EnableRegisterVars: true, + RegisterVars: map[string]bool{ + "rmw_var": true, + }, + } + + output, dissolved := Optimize(tt.lines, cfg) + + if dissolved["rmw_var"] { + t.Errorf("rmw_var should NOT be dissolved — %s reads its value", tt.opcode) + } + + joined := strings.Join(output, "\n") + if !strings.Contains(joined, "sta rmw_var") { + t.Errorf("expected sta rmw_var to be kept (followed by %s), got:\n%s", tt.opcode, joined) + } + }) + } +} diff --git a/internal/optimizer/pass_regdead.go b/internal/optimizer/pass_regdead.go index c41e512..1c0d423 100644 --- a/internal/optimizer/pass_regdead.go +++ b/internal/optimizer/pass_regdead.go @@ -2,19 +2,19 @@ package optimizer // passRegDead eliminates dead stores to REGISTER variables. // Two-phase approach: -// 1. Global pre-scan: if no lda regVar exists anywhere in the input, +// 1. Global pre-scan: if no instruction reads regVar anywhere in the input, // all stores to that variable are provably dead (labels are irrelevant). -// 2. Per-store local scan: for variables that DO have at least one lda, +// 2. Per-store local scan: for variables that DO have at least one read, // scan forward through the current basic block. // // This is safe because REGISTER variables have a contract: nothing external // can observe their memory location (ASM/Script/Macro references are compile errors). func passRegDead(lines []asmLine, registerVars map[string]bool) []asmLine { - // Phase 1 — global pre-scan: which REGISTER vars have at least one lda? - hasLda := make(map[string]bool, len(registerVars)) + // Phase 1 — global pre-scan: which REGISTER vars have at least one read? + hasRead := make(map[string]bool, len(registerVars)) for _, l := range lines { - if l.isCode && l.opcode == "lda" && l.operand != "" && registerVars[l.operand] { - hasLda[l.operand] = true + if l.isCode && l.operand != "" && registerVars[l.operand] && readsFrom(l.opcode, l.operand, l.operand) { + hasRead[l.operand] = true } } @@ -24,8 +24,8 @@ func passRegDead(lines []asmLine, registerVars map[string]bool) []asmLine { line := lines[i] if line.isCode && line.opcode == "sta" && line.operand != "" && registerVars[line.operand] { - if !hasLda[line.operand] { - // Globally dead — no lda anywhere in the program + if !hasRead[line.operand] { + // Globally dead — no read anywhere in the program continue } if isRegStoreDead(lines, i+1, line.operand) { @@ -41,8 +41,8 @@ func passRegDead(lines []asmLine, registerVars map[string]bool) []asmLine { // isRegStoreDead scans forward from start to determine whether a store to a // REGISTER variable operand is dead. Returns true if the store can be removed. -// A store is dead only if no matching lda is found anywhere forward before -// a label or control-flow-ending instruction. +// A store is dead only if no instruction that reads the operand is found +// anywhere forward before a label or control-flow-ending instruction. func isRegStoreDead(lines []asmLine, start int, operand string) bool { for i := start; i < len(lines); i++ { l := lines[i] @@ -63,7 +63,7 @@ func isRegStoreDead(lines []asmLine, start int, operand string) bool { return true // rts/jmp/brk/rti — execution ends here } - if l.isCode && l.opcode == "lda" && l.operand == operand { + if l.isCode && readsFrom(l.opcode, l.operand, operand) { return false // the value IS read from memory later } } @@ -71,6 +71,24 @@ func isRegStoreDead(lines []asmLine, start int, operand string) bool { return true // end of block reached with no matching load } +// readsFrom returns true if the instruction reads from the given memory operand. +// Covers loads (lda/ldx/ldy), compares (cmp/cpx/cpy), ALU ops (adc/sbc/and/ora/eor/bit), +// and read-modify-write instructions (dec/inc/asl/lsr/rol/ror). +func readsFrom(opcode, operand, varName string) bool { + if operand != varName { + return false + } + switch opcode { + case "lda", "ldx", "ldy": + return true + case "adc", "sbc", "and", "ora", "eor", "cmp", "cpx", "cpy", "bit": + return true + case "dec", "inc", "asl", "lsr", "rol", "ror": + return true + } + return false +} + // blocksFlow returns true for instructions that unconditionally end the current // execution path: rts, jmp, brk, rti func blocksFlow(line asmLine) bool {