Fixed optimizer bugs

internal/compiler/compiler.go

@@ -234,7 +234,7 @@ func (c *Compiler) Compile(lines []preproc.Line) ([]string, error) {
 		}
 
 		codeOutput = append(codeOutput, fmt.Sprintf("; %s", line.Text))
-		if len(asmLines) > 0 && c.isOptimizing() {
+		if len(asmLines) > 0 && c.isMarkersEnabled() {
 			codeOutput = append(codeOutput, fmt.Sprintf("; @@OPT:%s:%s", classString(cmd.GetClass()), cmd.GetName()))
 		}
 		codeOutput = append(codeOutput, asmLines...)
@@ -296,6 +296,16 @@ func (c *Compiler) isOptimizing() bool {
 	return cfg.Any()
 }
 
+// isMarkersEnabled returns true if _P_OPT_MARKERS is active
+func (c *Compiler) isMarkersEnabled() bool {
+	if c.ctx == nil || c.ctx.Pragma == nil {
+		return false
+	}
+	idx := c.ctx.Pragma.GetCurrentPragmaSetIndex()
+	ps := c.ctx.Pragma.GetPragmaSetByIndex(idx)
+	return ps.GetPragma("_P_OPT_MARKERS") != "" && ps.GetPragma("_P_OPT_MARKERS") != "0"
+}
+
 // getOptimizerConfig returns optimizer config if optimization is enabled, nil otherwise
 func (c *Compiler) getOptimizerConfig() *optimizer.Config {
 	if c.ctx == nil || c.ctx.Pragma == nil {

internal/optimizer/config.go

@@ -5,22 +5,24 @@ import (
 )
 
 type Config struct {
-	EnableLoad bool
+	EnableLoad  bool
-	EnableImm  bool
+	EnableImm   bool
-	EnableJmp  bool
+	EnableJmp   bool
-	EnableSelf bool
+	EnableSelf  bool
-	Debug      bool
+	Debug       bool
+	ShowMarkers bool
 }
 
 func NewConfig(ps preproc.PragmaSet) *Config {
 	all := ps.GetPragma("_P_OPT_ALL") != "" && ps.GetPragma("_P_OPT_ALL") != "0"
 
 	return &Config{
-		EnableLoad: all || (ps.GetPragma("_P_OPT_LOAD") != "" && ps.GetPragma("_P_OPT_LOAD") != "0"),
+		EnableLoad:  all || (ps.GetPragma("_P_OPT_LOAD") != "" && ps.GetPragma("_P_OPT_LOAD") != "0"),
-		EnableImm:  all || (ps.GetPragma("_P_OPT_IMM") != "" && ps.GetPragma("_P_OPT_IMM") != "0"),
+		EnableImm:   all || (ps.GetPragma("_P_OPT_IMM") != "" && ps.GetPragma("_P_OPT_IMM") != "0"),
-		EnableJmp:  all || (ps.GetPragma("_P_OPT_JMP") != "" && ps.GetPragma("_P_OPT_JMP") != "0"),
+		EnableJmp:   all || (ps.GetPragma("_P_OPT_JMP") != "" && ps.GetPragma("_P_OPT_JMP") != "0"),
-		EnableSelf: all || (ps.GetPragma("_P_OPT_SELF") != "" && ps.GetPragma("_P_OPT_SELF") != "0"),
+		EnableSelf:  all || (ps.GetPragma("_P_OPT_SELF") != "" && ps.GetPragma("_P_OPT_SELF") != "0"),
-		Debug:      (ps.GetPragma("_P_OPT_DEBUG") != "" && ps.GetPragma("_P_OPT_DEBUG") != "0"),
+		Debug:       (ps.GetPragma("_P_OPT_DEBUG") != "" && ps.GetPragma("_P_OPT_DEBUG") != "0"),
+		ShowMarkers: (ps.GetPragma("_P_OPT_MARKERS") != "" && ps.GetPragma("_P_OPT_MARKERS") != "0"),
 	}
 }
 

internal/optimizer/config_test.go

@@ -0,0 +1,54 @@
+package optimizer
+
+import (
+	"testing"
+	"c65gm/internal/preproc"
+)
+
+func TestNewConfigWithAll(t *testing.T) {
+	ps := preproc.NewPragmaSet(map[string]string{
+		"_P_OPT_ALL":   "1",
+		"_P_OPT_DEBUG": "1",
+	})
+	cfg := NewConfig(ps)
+	if !cfg.Any() {
+		t.Error("expected Any()=true")
+	}
+	if !cfg.EnableLoad {
+		t.Error("expected EnableLoad=true from _P_OPT_ALL")
+	}
+	if !cfg.EnableImm {
+		t.Error("expected EnableImm=true from _P_OPT_ALL")
+	}
+	if !cfg.Debug {
+		t.Error("expected Debug=true")
+	}
+}
+
+func TestNewConfigPragmasAccumulate(t *testing.T) {
+	// Pragma sets COPY previous values — so last set has ALL pragmas
+	p := preproc.NewPragma()
+	p.AddPragma("_P_OPT_ALL", "1")
+	p.AddPragma("_P_OPT_DEBUG", "1")
+	p.AddPragma("_P_SOMETHING_ELSE", "1")
+
+	// Last set should have all three
+	ps := p.GetPragmaSetByIndex(p.GetCurrentPragmaSetIndex())
+	if v := ps.GetPragma("_P_OPT_ALL"); v != "1" {
+		t.Errorf("expected _P_OPT_ALL=1, got %q", v)
+	}
+	if v := ps.GetPragma("_P_OPT_DEBUG"); v != "1" {
+		t.Errorf("expected _P_OPT_DEBUG=1, got %q", v)
+	}
+	if v := ps.GetPragma("_P_SOMETHING_ELSE"); v != "1" {
+		t.Errorf("expected _P_SOMETHING_ELSE=1, got %q", v)
+	}
+
+	cfg := NewConfig(ps)
+	if !cfg.Any() {
+		t.Error("expected Any()=true")
+	}
+	if !cfg.Debug {
+		t.Error("expected Debug=true")
+	}
+}

internal/optimizer/debug.go

@@ -2,10 +2,62 @@ package optimizer
 
 import "fmt"
 
-// debugWithOriginals wraps the current state with a debug comment header
+// debugDiff produces annotated output showing original vs optimized code.
-func debugWithOriginals(lines []asmLine, phase string) []asmLine {
+// Lines that were removed are shown with "[removed]" annotation.
-	header := fmt.Sprintf("; --- %s ---", phase)
+func debugDiff(original, optimized []asmLine) []asmLine {
-	result := []asmLine{{text: header, isComment: true}}
+	origCount := countCodeLines(original)
-	result = append(result, lines...)
+	optCount := countCodeLines(optimized)
+	saved := origCount - optCount
+
+	header := fmt.Sprintf("; --- peephole: %d asm lines → %d (%d saved) ---", origCount, optCount, saved)
+	var result []asmLine
+	result = append(result, asmLine{text: header, isComment: true})
+
+	// Walk both arrays in sync. When optimized doesn't have a line
+	// that original has, it was removed.
+	oi := 0 // index into original (pre-stripped)
+	opi := 0 // index into optimized
+
+	for oi < len(original) {
+		origLine := original[oi]
+
+		if opi < len(optimized) && linesMatch(origLine, optimized[opi]) {
+			result = append(result, optimized[opi])
+			oi++
+			opi++
+		} else if !origLine.isCode {
+			// Non-code line that doesn't match anything — pass through
+			result = append(result, origLine)
+			oi++
+		} else {
+			// Code line in original but not in optimized → removed
+			result = append(result, asmLine{
+				text:      fmt.Sprintf("; [removed] %s", origLine.text),
+				isComment: true,
+			})
+			oi++
+		}
+	}
+
+	// Append any remaining optimized lines (shouldn't happen in practice)
+	for ; opi < len(optimized); opi++ {
+		result = append(result, optimized[opi])
+	}
+
 	return result
 }
+
+// linesMatch returns true if two asmLines represent the same instruction.
+func linesMatch(a, b asmLine) bool {
+	return a.isCode == b.isCode && a.isLabel == b.isLabel && a.isComment == b.isComment && a.text == b.text
+}
+
+func countCodeLines(lines []asmLine) int {
+	n := 0
+	for _, l := range lines {
+		if l.isCode {
+			n++
+		}
+	}
+	return n
+}

internal/optimizer/optimizer.go

@@ -9,10 +9,7 @@ func Optimize(lines []string, cfg *Config) []string {
 	}
 
 	parsed := parseLines(lines)
-
+	original := parsed
-	if cfg.Debug {
-		parsed = debugWithOriginals(parsed, "before peephole")
-	}
 
 	if cfg.EnableLoad {
 		parsed = passLoadElimination(parsed)
@@ -27,10 +24,10 @@ func Optimize(lines []string, cfg *Config) []string {
 		parsed = passSelfAssignment(parsed)
 	}
 
-	if cfg.Debug {
-		parsed = debugWithOriginals(parsed, "after peephole")
-	}
-
 	parsed = stripOptMarkers(parsed)
+	if cfg.Debug {
+		original = stripOptMarkers(original)
+		parsed = debugDiff(original, parsed)
+	}
 	return linesToString(parsed)
 }

internal/optimizer/optimizer_test.go

@@ -207,3 +207,46 @@ func TestOptimizeIntegration(t *testing.T) {
 		t.Errorf("expected 5 lines, got %d:\n%v", len(output), output)
 	}
 }
+
+func TestOptimizeWithDebug(t *testing.T) {
+	input := lines(
+		"\tlda x",
+		"; @@OPT:LINEAR:LET",
+		"\tlda x",
+		"\tsta y",
+	)
+
+	cfg := &Config{EnableLoad: true, Debug: true}
+	output := Optimize(input, cfg)
+
+	// Header + 2 kept lines + 1 removed annotation = 4
+	if len(output) != 4 {
+		t.Errorf("expected 4 lines, got %d:\n%v", len(output), output)
+	}
+	if !containsPrefix(output, "; --- peephole:") {
+		t.Errorf("expected peephole summary line:\n%v", output)
+	}
+	if !containsSubstring(output, "[removed]") {
+		t.Errorf("expected [removed] annotation:\n%v", output)
+	}
+}
+
+func containsPrefix(lines []string, prefix string) bool {
+	for _, l := range lines {
+		if len(l) >= len(prefix) && l[:len(prefix)] == prefix {
+			return true
+		}
+	}
+	return false
+}
+
+func containsSubstring(lines []string, sub string) bool {
+	for _, l := range lines {
+		for i := 0; i <= len(l)-len(sub); i++ {
+			if l[i:i+len(sub)] == sub {
+				return true
+			}
+		}
+	}
+	return false
+}

internal/optimizer/pass_imm.go

@@ -1,5 +1,7 @@
 package optimizer
 
+import "strings"
+
 // passImmElimination removes redundant immediate loads.
 // Pattern: lda #N; ... (no A modification) ...; lda #N → remove second
 // Applies to A, X, Y.
@@ -9,9 +11,6 @@ func passImmElimination(lines []asmLine) []asmLine {
 
 	for _, line := range lines {
 		if !line.isCode {
-			if line.optMarker {
-				continue
-			}
 			if line.isLabel {
 				rs.reset()
 			}
@@ -37,31 +36,42 @@ func passImmElimination(lines []asmLine) []asmLine {
 }
 
 func isRedundantImm(line asmLine, rs *regState) bool {
+	if !isImmediate(line.operand) {
+		return false
+	}
+	val := parseValueAfterHash(line.operand)
+	if val < 0 {
+		return false // unparseable (label ref, <label, >label)
+	}
 	switch line.opcode {
 	case "lda":
-		if isImmediate(line.operand) {
+		if rs.a.val == val {
-			val := parseHexOrDec(line.operand[1:])
+			return true
-			if rs.a.val == val {
-				return true
-			}
-			rs.loadImm("a", val)
 		}
+		rs.loadImm("a", val)
 	case "ldx":
-		if isImmediate(line.operand) {
+		if rs.x.val == val {
-			val := parseHexOrDec(line.operand[1:])
+			return true
-			if rs.x.val == val {
-				return true
-			}
-			rs.loadImm("x", val)
 		}
+		rs.loadImm("x", val)
 	case "ldy":
-		if isImmediate(line.operand) {
+		if rs.y.val == val {
-			val := parseHexOrDec(line.operand[1:])
+			return true
-			if rs.y.val == val {
-				return true
-			}
-			rs.loadImm("y", val)
 		}
+		rs.loadImm("y", val)
 	}
 	return false
 }
+
+// parseValueAfterHash extracts the numeric value after '#'. Returns -1 for unparseable.
+func parseValueAfterHash(operand string) int {
+	if len(operand) < 2 {
+		return -1
+	}
+	s := operand[1:]
+	// ACME high/low byte operators are not evaluable at optimize time
+	if strings.HasPrefix(s, "<") || strings.HasPrefix(s, ">") {
+		return -1
+	}
+	return parseHexOrDec(s)
+}

internal/optimizer/pass_jmp.go

@@ -16,10 +16,6 @@ func passJmpNext(lines []asmLine) []asmLine {
 
 		line := lines[i]
 
-		if skipJmpMarker(line) {
-			continue
-		}
-
 		// Check for jmp L followed by label L:
 		if line.isCode && line.opcode == "jmp" && i+1 < len(lines) {
 			next := lines[i+1]

internal/optimizer/pass_load.go

@@ -1,5 +1,7 @@
 package optimizer
 
+import "strings"
+
 // passLoadElimination removes redundant memory-to-register loads.
 // Pattern: lda X; ... (X not modified) ...; lda X  → remove second
 // Applies to A, X, Y registers identically.
@@ -9,9 +11,6 @@ func passLoadElimination(lines []asmLine) []asmLine {
 
 	for _, line := range lines {
 		if !line.isCode {
-			if line.optMarker {
-				continue
-			}
 			if line.isLabel {
 				rs.reset()
 			}
@@ -27,20 +26,30 @@ func passLoadElimination(lines []asmLine) []asmLine {
 
 		// Check for redundant memory load
 		if line.opcode == "lda" && !isImmediate(line.operand) {
-			if rs.a.src == line.operand {
+			if isIndexedOperand(line.operand) {
+				// Indexed/indirect addressing: address depends on X/Y, can't track by operand text alone
+				updateRegState(line, rs)
+			} else if rs.a.src == line.operand {
 				continue // redundant, remove
+			} else {
+				rs.a.src = line.operand
 			}
-			rs.a.src = line.operand
 		} else if line.opcode == "ldx" && !isImmediate(line.operand) {
-			if rs.x.src == line.operand {
+			if isIndexedOperand(line.operand) {
+				updateRegState(line, rs)
+			} else if rs.x.src == line.operand {
 				continue
+			} else {
+				rs.x.src = line.operand
 			}
-			rs.x.src = line.operand
 		} else if line.opcode == "ldy" && !isImmediate(line.operand) {
-			if rs.y.src == line.operand {
+			if isIndexedOperand(line.operand) {
+				updateRegState(line, rs)
+			} else if rs.y.src == line.operand {
 				continue
+			} else {
+				rs.y.src = line.operand
 			}
-			rs.y.src = line.operand
 		} else {
 			updateRegState(line, rs)
 		}
@@ -51,6 +60,13 @@ func passLoadElimination(lines []asmLine) []asmLine {
 	return result
 }
 
+// isIndexedOperand returns true for addressing modes where the effective address
+// depends on X or Y registers (e.g. (zp),y, $xxxx,x, zp,x).
+// Such loads can't be tracked by operand text alone since the register may change.
+func isIndexedOperand(operand string) bool {
+	return strings.Contains(operand, "(") || strings.Contains(operand, ",")
+}
+
 func isImmediate(operand string) bool {
 	return len(operand) > 0 && operand[0] == '#'
 }

internal/optimizer/pass_self.go

@@ -16,10 +16,6 @@ func passSelfAssignment(lines []asmLine) []asmLine {
 
 		line := lines[i]
 
-		if skipJmpMarker(line) {
-			continue
-		}
-
 		// Check for lda X; sta X
 		if line.isCode && line.opcode == "lda" && !isImmediate(line.operand) && i+1 < len(lines) {
 			next := lines[i+1]

internal/optimizer/regstate.go

@@ -92,19 +92,31 @@ func updateRegState(line asmLine, rs *regState) {
 	switch line.opcode {
 	case "lda":
 		if strings.HasPrefix(line.operand, "#") {
-			rs.loadImm("a", parseHexOrDec(line.operand[1:]))
+			if val := parseHexOrDec(line.operand[1:]); val >= 0 {
+				rs.loadImm("a", val)
+			} else {
+				rs.resetA()
+			}
 		} else {
 			rs.loadMem("a", line.operand)
 		}
 	case "ldx":
 		if strings.HasPrefix(line.operand, "#") {
-			rs.loadImm("x", parseHexOrDec(line.operand[1:]))
+			if val := parseHexOrDec(line.operand[1:]); val >= 0 {
+				rs.loadImm("x", val)
+			} else {
+				rs.resetX()
+			}
 		} else {
 			rs.loadMem("x", line.operand)
 		}
 	case "ldy":
 		if strings.HasPrefix(line.operand, "#") {
-			rs.loadImm("y", parseHexOrDec(line.operand[1:]))
+			if val := parseHexOrDec(line.operand[1:]); val >= 0 {
+				rs.loadImm("y", val)
+			} else {
+				rs.resetY()
+			}
 		} else {
 			rs.loadMem("y", line.operand)
 		}

internal/optimizer/scanner.go

@@ -5,20 +5,32 @@ import (
 	"strings"
 )
 
-// parseHexOrDec parses an ACME-style number: $ff or 255
+// parseHexOrDec parses an ACME-style number: $ff or 255.
+// Returns -1 if s is not a parseable simple number (e.g. label references, <label, >label).
 func parseHexOrDec(s string) int {
 	if strings.HasPrefix(s, "$") {
-		v, _ := strconv.ParseInt(s[1:], 16, 16)
+		v, err := strconv.ParseInt(s[1:], 16, 16)
+		if err != nil {
+			return -1
+		}
 		return int(v)
 	}
 	// Strip trailing characters after the number (e.g., ",y" in "5,y")
 	if idx := strings.IndexAny(s, ",)"); idx >= 0 {
 		s = s[:idx]
 	}
-	v, _ := strconv.ParseInt(s, 10, 16)
+	v, err := strconv.ParseInt(s, 10, 16)
+	if err != nil {
+		return -1
+	}
 	return int(v)
 }
 
+// isSimpleNumber returns true if s is a plain hex/dec number (no label refs, <, >, etc.)
+func isSimpleNumber(s string) bool {
+	return parseHexOrDec(s) >= 0
+}
+
 type asmLine struct {
 	text      string
 	isLabel   bool