#2597 z80: bug in peephole rule #15

[alvin]

I came across a bug in peephole rule 15 for the z80. See:
https://sourceforge.net/p/sdcc/code/HEAD/tree/trunk/sdcc/src/z80/peeph-z80.def

I cannot supply code to reproduce the problem in sdcc but I can give the code fragment where the rule fails.

First the peephole rule itself.

replace restart {
    ld  %1, %2 (%3)
    ld  %4, %5
    ld  %7, %1
} by {
    ld  %7, %2 (%3)
    ; peephole 15 loaded %2 (%3) into %7 directly instead of going through %1.
    ld  %4, %5
} if canAssign(%7 %2 %3), notVolatile(%1), notUsed(%1), notSame(%1 %5), notSame(%7 %4), notSame(%7 %5), notSame(%4 '(hl)' '(de)' '(bc)'), notSame(%5 '(hl)' '(de)' '(bc)' '(iy)')

The code fragment before substitution by this rule:

    ld  l,4 (ix)
    ld  h,5 (ix)
    ld  (#_currentScreen),hl
    ld  a,h
    ld  hl,#_currentScreen + 1

The rule is matched with:

%1=h
%2=5
%3=ix
%4=(#currentScreen)
%5=hl
%7=a

After substitution:

    ld  l,4 (ix)
    ld     a,5 (ix)
    ld  (#_currentScreen),hl
    ld  hl,#_currentScreen + 1

where you can see register H no longer holds the correct value before the "ld (#currentScreen),hl" instruction.

The fault is in the conditional "notSame(%1 %5)" which looks for an exact match between "H" and "HL" but finds none. The conditional should be "notOperandsRelated(%1 %5)". In fact a few of the other conditionals should probably be changed too.

Proposed fix:

replace restart {
    ld  %1, %2 (%3)
    ld  %4, %5
    ld  %7, %1
} by {
    ld  %7, %2 (%3)
    ; peephole 15 loaded %2 (%3) into %7 directly instead of going through %1.
    ld  %4, %5
} if canAssign(%7 %2 %3), notVolatile(%1), notUsed(%1), notOperandsRelated(%1 %5), notOperandsRelated(%7 %4), notOperandsRelated(%7 %5), notSame(%4 '(hl)' '(de)' '(bc)'), notSame(%5 '(hl)' '(de)' '(bc)' '(iy)')

[alvin]

That should be "operandsNotRelated" and not "notOperandsRelated"

[Philipp Klaus Krause]

Fixed in revision [r9874].

Philipp