Skip to content

Commit 1bc4c28

Browse files
Aaron Wohlclaude
andcommitted
Fix code review issues: label detection bug, duplicate patterns, mixed indentation
- Fix label detection after .strip() being a no-op in 11+ locations; add _is_label_line() helper that checks raw unstripped lines - Remove 6 patterns duplicated between Phase 1 and Phase 2; combine both phases into a single fixpoint loop - Unify output indentation to tabs (Phase 1 was using 4 spaces) - Fix version mismatch: __init__.py 0.1.0 -> 0.2.1 to match pyproject.toml - Remove redundant ld_a_a pattern (subset of ld_r_r) - Remove 3 pointless condition=lambda ops: True - Remove unused proc_label variable - Fix comment using 8080 mnemonic "mvi" instead of Z80 "ld h,0" - Fix dw directive double-tab inconsistency - Add 37 new tests covering _parse_const, inc_hl_const, multiply strength reduction, inc/dec memory, DJNZ, 8080-style shift, subde zero elimination, ld de,(addr), jump threading, relative jumps, dead store elimination, output indentation, and version consistency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 9b66605 commit 1bc4c28

3 files changed

Lines changed: 299 additions & 121 deletions

File tree

tests/test_peephole.py

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import pytest
44
from upeepz80 import optimize, PeepholeOptimizer
5+
from upeepz80.peephole import PeepholeOptimizer as _PeepholeOptimizer
56

67

78
class TestBasicPatterns:
@@ -414,3 +415,258 @@ def test_labels_break_patterns(self):
414415
# Pattern should not match across label
415416
assert "push hl" in result
416417
assert "pop hl" in result
418+
419+
420+
class TestParseConst:
421+
"""Test _parse_const for various number formats."""
422+
423+
@pytest.fixture
424+
def opt(self):
425+
return _PeepholeOptimizer()
426+
427+
def test_decimal(self, opt):
428+
assert opt._parse_const("42") == 42
429+
430+
def test_zero(self, opt):
431+
assert opt._parse_const("0") == 0
432+
433+
def test_hex_suffix(self, opt):
434+
assert opt._parse_const("0FFH") == 255
435+
436+
def test_hex_suffix_lower(self, opt):
437+
assert opt._parse_const("10h") == 16
438+
439+
def test_hex_prefix_0x(self, opt):
440+
assert opt._parse_const("0x10") == 16
441+
442+
def test_binary(self, opt):
443+
assert opt._parse_const("10101B") == 21
444+
445+
def test_octal_o(self, opt):
446+
assert opt._parse_const("77O") == 63
447+
448+
def test_octal_q(self, opt):
449+
assert opt._parse_const("77Q") == 63
450+
451+
def test_label_returns_none(self, opt):
452+
assert opt._parse_const("MYLABEL") is None
453+
454+
def test_empty_returns_none(self, opt):
455+
assert opt._parse_const("") is None
456+
457+
458+
class TestIncHlConst:
459+
"""Test ld de,N; add hl,de -> inc hl (repeated) optimization."""
460+
461+
def test_ld_de_1_add_hl_de(self):
462+
"""ld de,1; add hl,de -> inc hl"""
463+
result = optimize("\tld de,1\n\tadd hl,de")
464+
assert result.strip() == "inc hl"
465+
466+
def test_ld_de_2_add_hl_de(self):
467+
"""ld de,2; add hl,de -> inc hl; inc hl"""
468+
result = optimize("\tld de,2\n\tadd hl,de")
469+
assert result.strip().count("inc hl") == 2
470+
471+
def test_ld_de_3_add_hl_de(self):
472+
"""ld de,3; add hl,de -> inc hl; inc hl; inc hl"""
473+
result = optimize("\tld de,3\n\tadd hl,de")
474+
assert result.strip().count("inc hl") == 3
475+
476+
def test_ld_de_4_not_optimized(self):
477+
"""ld de,4; add hl,de should NOT be converted to inc hl."""
478+
result = optimize("\tld de,4\n\tadd hl,de")
479+
assert "add hl,de" in result
480+
481+
482+
class TestMulStrengthReduction:
483+
"""Test multiply by power-of-2 strength reduction."""
484+
485+
def test_mul_by_2(self):
486+
"""ld de,2; call ??mul16 -> add hl,hl"""
487+
result = optimize("\tld de,2\n\tcall ??mul16")
488+
assert "add hl,hl" in result
489+
assert result.strip().count("add hl,hl") == 1
490+
assert "call" not in result
491+
492+
def test_mul_by_4(self):
493+
"""ld de,4; call ??mul16 -> add hl,hl; add hl,hl"""
494+
result = optimize("\tld de,4\n\tcall ??mul16")
495+
assert result.strip().count("add hl,hl") == 2
496+
assert "call" not in result
497+
498+
def test_mul_by_8(self):
499+
"""ld de,8; call ??mul16 -> 3x add hl,hl"""
500+
result = optimize("\tld de,8\n\tcall ??mul16")
501+
assert result.strip().count("add hl,hl") == 3
502+
503+
def test_mul_by_non_power_of_2(self):
504+
"""ld de,3; call ??mul16 should NOT be strength-reduced."""
505+
result = optimize("\tld de,3\n\tcall ??mul16")
506+
assert "call" in result
507+
508+
def test_mul_at_mul16(self):
509+
"""ld de,2; call @mul16 -> add hl,hl"""
510+
result = optimize("\tld de,2\n\tcall @mul16")
511+
assert "add hl,hl" in result
512+
513+
def test_mul_dunder_mul16(self):
514+
"""ld de,2; call __mul16 -> add hl,hl"""
515+
result = optimize("\tld de,2\n\tcall __mul16")
516+
assert "add hl,hl" in result
517+
518+
519+
class TestIncDecMem:
520+
"""Test ld a,(addr); inc/dec a; ld (addr),a -> ld hl,addr; inc/dec (hl)."""
521+
522+
def test_inc_mem(self):
523+
"""ld a,(COUNT); inc a; ld (COUNT),a -> ld hl,COUNT; inc (hl)"""
524+
asm = "\tld a,(COUNT)\n\tinc a\n\tld (COUNT),a"
525+
result = optimize(asm)
526+
assert "inc (hl)" in result
527+
assert "ld hl,COUNT" in result
528+
529+
def test_dec_mem(self):
530+
"""ld a,(COUNT); dec a; ld (COUNT),a -> ld hl,COUNT; dec (hl)"""
531+
asm = "\tld a,(COUNT)\n\tdec a\n\tld (COUNT),a"
532+
result = optimize(asm)
533+
assert "dec (hl)" in result
534+
assert "ld hl,COUNT" in result
535+
536+
537+
class TestDjnz:
538+
"""Test dec b; jr/jp nz,label -> djnz label conversion."""
539+
540+
def test_dec_b_jr_nz(self):
541+
"""dec b; jr nz,LOOP -> djnz LOOP"""
542+
asm = "LOOP:\n\tnop\n\tdec b\n\tjr nz,LOOP"
543+
result = optimize(asm)
544+
assert "djnz LOOP" in result
545+
assert "dec b" not in result
546+
547+
def test_dec_b_jp_nz(self):
548+
"""dec b; jp nz,LOOP -> djnz LOOP (if in range)"""
549+
asm = "LOOP:\n\tnop\n\tdec b\n\tjp nz,LOOP"
550+
result = optimize(asm)
551+
assert "djnz LOOP" in result
552+
553+
554+
class TestShiftToZ80:
555+
"""Test 8080-style 16-bit right shift to Z80 native."""
556+
557+
def test_shr_hl(self):
558+
"""or a; ld a,h; rra; ld h,a; ld a,l; rra; ld l,a -> srl h; rr l"""
559+
asm = "\tor a\n\tld a,h\n\trra\n\tld h,a\n\tld a,l\n\trra\n\tld l,a"
560+
result = optimize(asm)
561+
assert "srl h" in result
562+
assert "rr l" in result
563+
assert "rra" not in result
564+
565+
566+
class TestSubdeZero:
567+
"""Test ld de,0; call ??subde -> elimination."""
568+
569+
def test_subde_zero_eliminated(self):
570+
"""ld de,0; call ??subde -> nothing"""
571+
result = optimize("\tld de,0\n\tcall ??subde")
572+
assert result.strip() == ""
573+
574+
def test_subde_zero_at_variant(self):
575+
"""ld de,0; call @subde -> nothing"""
576+
result = optimize("\tld de,0\n\tcall @subde")
577+
assert result.strip() == ""
578+
579+
580+
class TestLdDeAddr:
581+
"""Test push hl; ld hl,(addr); ex de,hl; pop hl -> ld de,(addr)."""
582+
583+
def test_ld_de_from_mem(self):
584+
"""push hl; ld hl,(DATA); ex de,hl; pop hl -> ld de,(DATA)"""
585+
asm = "\tpush hl\n\tld hl,(DATA)\n\tex de,hl\n\tpop hl"
586+
result = optimize(asm)
587+
assert "ld de,(DATA)" in result
588+
assert "push" not in result
589+
assert "pop" not in result
590+
assert "ex" not in result
591+
592+
593+
class TestJumpThreading:
594+
"""Test jump threading optimization."""
595+
596+
def test_thread_through_unconditional(self):
597+
"""jp A where A: jp B -> jp B"""
598+
asm = "\tjp LBL_A\nLBL_A:\n\tjp LBL_B\nLBL_B:\n\tret"
599+
result = optimize(asm)
600+
# Should jump directly to LBL_B
601+
assert "jp LBL_A" not in result or "jr LBL_A" not in result
602+
603+
def test_chain_threading(self):
604+
"""jp A; A: jp B; B: jp C -> jp C"""
605+
asm = "\tjp L1\nL1:\n\tjp L2\nL2:\n\tjp L3\nL3:\n\tret"
606+
result = optimize(asm)
607+
# Should not contain jp L1 or jp L2 (threaded to L3 or ret)
608+
lines = [l.strip() for l in result.split("\n") if l.strip()]
609+
jump_targets = [l.split()[-1] for l in lines if l.startswith(("jp ", "jr "))]
610+
# All jumps should target L3 or be eliminated
611+
for target in jump_targets:
612+
assert target in ("L3", "ret"), f"Unexpected jump target: {target}"
613+
614+
615+
class TestRelativeJumps:
616+
"""Test jp to jr conversion."""
617+
618+
def test_jp_to_jr_nearby_label(self):
619+
"""jp LABEL -> jr LABEL when target is close."""
620+
asm = "LOOP:\n\tnop\n\tjp LOOP"
621+
result = optimize(asm)
622+
assert "jr LOOP" in result or "djnz" in result
623+
624+
def test_jp_conditional_to_jr(self):
625+
"""jp z,LABEL -> jr z,LABEL when close."""
626+
asm = "TARGET:\n\tnop\n\tor a\n\tjp z,TARGET"
627+
result = optimize(asm)
628+
assert "jr z,TARGET" in result
629+
630+
631+
class TestDeadStoreElimination:
632+
"""Test dead store elimination at procedure entry."""
633+
634+
def test_dead_store_removed(self):
635+
"""Store to unused memory location is removed."""
636+
asm = "myproc:\n\tld (PARAM),a\n\tadd a,b\n\tret"
637+
result = optimize(asm)
638+
assert "myproc:" in result
639+
assert "(PARAM)" not in result
640+
641+
def test_live_store_kept(self):
642+
"""Store to memory location that is later loaded is kept."""
643+
asm = "myproc:\n\tld (PARAM),a\n\tcall other\n\tld a,(PARAM)\n\tret"
644+
result = optimize(asm)
645+
assert "(PARAM)" in result
646+
647+
648+
class TestOutputIndentation:
649+
"""Test that output uses consistent indentation."""
650+
651+
def test_pattern_replacement_uses_tabs(self):
652+
"""Pattern replacements should use tab indentation."""
653+
result = optimize("\tld a,0")
654+
# Should be tab-indented xor a
655+
assert "\txor a" in result or result.strip() == "xor a"
656+
657+
def test_push_pop_replacement_uses_tabs(self):
658+
"""Push/pop copy replacements should use tab indentation."""
659+
result = optimize("\tpush hl\n\tpop de")
660+
for line in result.split("\n"):
661+
stripped = line.strip()
662+
if stripped and not stripped.endswith(":"):
663+
assert line.startswith("\t"), f"Line not tab-indented: {repr(line)}"
664+
665+
666+
class TestVersionConsistency:
667+
"""Test that version numbers are consistent."""
668+
669+
def test_version_matches(self):
670+
"""__init__.py version should match pyproject.toml."""
671+
import upeepz80
672+
assert upeepz80.__version__ == "0.2.1"

upeepz80/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
use upeep80 instead.
99
"""
1010

11-
__version__ = "0.1.0"
11+
__version__ = "0.2.1"
1212
__author__ = "upeepz80 project"
1313

1414
from .peephole import (

0 commit comments

Comments
 (0)