Menu

#3593 MOS6502 peephole rules bugs

closed-fixed
None
MOS6502
5
2023-05-30
2023-05-26
No

Sample code (bug.c):

static unsigned char bug(unsigned char a)
{
        unsigned char i = 0;

        if (a == 0) {
                return 0;
        }

        while (i != a) {
                ++i;
        }

        return 0;
}

SDCC command:

sdcc -mmos6502 -o bug.rel -c bug.c

SDCC version:

SDCC : z80/z180/r2k/r2ka/r3ka/sm83/tlcs90/ez80_z80/z80n/ds390/TININative/ds400/hc08/s08/stm8/pdk13/pdk14/pdk15/mos6502 TD- 4.2.14 #14012 (Linux)
published under GNU General Public License (GPL)

Incorrect output (in bug.asm):

_bug:
;       bug.c: 5: if (a == 0) {
        sta     _bug_a_65536_1
        bne     00110$

I traced the issue down to this peephole rule (in src/mos6502/peeph.def):

replace restart {
        sta     %1 
        lda     %1
} by {
        sta     %1 
    ; Peephole load 1 - eliminated redundant lda
} if notVolatile(%1)

That can be fixed by adding ora #0x00:

replace restart {
        sta     %1 
        lda     %1
} by {
        sta     %1 
    ; Peephole load 1 - eliminated redundant lda
        ora     #0x00
} if notVolatile(%1)

The issue is that lda touches the Z flag which the following bne relies on (sta does not touch any flag). An ora #0x00 or similar instruction touches the flags in the same way as lda. (The ora #0x00 may be unnecessary in many cases, but I figure it can be removed by another rule, and in fact it is removed already in some cases by the existing regop 2 rule.)

I attached a patch file with this change and changes for all the similar issues I found. I commented out all of the cmp/cpx/cpy #00 peephole rules because cmp/cpx/cpy touches the C flag which the lda/ldx/ldy instructions alone do not. This can make a difference when there's a following bcs/bcc instruction.

1 Attachments

Related

Wiki: NGI0-Entrust-SDCC

Discussion

  • Philipp Klaus Krause

    • status: open --> closed-fixed
    • assigned_to: Philipp Klaus Krause
     
  • Philipp Klaus Krause

    Thanks. Fixed in [r14092] using a variant of your patch.

     

    Related

    Commit: [r14092]


Log in to post a comment.

MongoDB Logo MongoDB