#3525 pdk/pic rules issue

[Konstantin Kim]

I guess pdk rules should have some specific guards. They should not be preceded by any skip instructions.
For example most first one:

replace restart {
    mo%3    %1, %2
} by {
    ; peephole 0 removed dead load into %1 from %2.
} if notUsed(%1), notVolatile(%1), notVolatile(%2)

on following case:

t1sn.io f, c
mov a,  #0x10
mov a,  #0x39

emit wrong result:

t1sn.io f, c
; peephole 0 removed dead load into a from #0x10.
mov a,  #0x39

Similar issues expects in pic ports

[Philipp Klaus Krause]

That looks like a serious issue. I guess we need to fix https://sourceforge.net/p/sdcc/bugs/3515/ first, as otherwise the fix here will likely only work when --debug is not specified.

[Philipp Klaus Krause]

Do we have a C code sample that triggers the bug?

[Philipp Klaus Krause]

I can produce the issue using the following C sample and peephole rule:

char c;

char f(void)
{
    char a = c + 1;
    if (a < 3)
        return 1;
    return a;
}

replace restart {
    t1sn.io %1, %2
    goto    %3
    %4  %5
%3:
} by {
    t0sn.io %1, %2
    ; peephole j6 removed goto by inverting test condition.
    %4  %5
%3:
} if labelRefCountChange(%3 -1)

[Philipp Klaus Krause]

I see two ways to fix this:
1) Make the peephole optimizer not match rules if the preceding instruction is a conditional skip. Ports that have such instructions would need to supply a isCondSkip helper function for this.
2) Have codegen put a label before the instruction after the conditional skip. Since the conditional skip is essentially a conditional relative jump with fixed jump distance this would just treat them the same. Requires no changes in the peephole optimizer, but more in codegen of ports that have conditional skips.

[Konstantin Kim]

the first looks straightforwardly simple and solid. it can be applied by default unconditionally to all existing rules.
the second is more logical and graceful. to be tested

[Philipp Klaus Krause]

Fixed for pdk in [r13817].