#290 STM8 Peephole optimizer fix "notUsed()"

[Visenri]

Hello.
I was playing with the "new" STM8 port and found a bug in peephole rule interpreter, a simple code like this failed to be optimized:

PB_ODR ^= 0x20;
if(PC_ODR)
{
...
}

Pre-patch version generated:

;   ../src/blinky.c: 331: PB_ODR ^= 0x20;
    ld  a, 0x5005
    xor a, #0x20
    ld  0x5005, a
;   ../src/blinky.c: 338: if(PC_ODR)
    ld  a, 0x500a

Patch version generates:

;   ../src/blinky.c: 331: PB_ODR ^= 0x20;
    bcpl    20485, #5
;   ../src/blinky.c: 338: if(PC_ODR)
    ld  a, 0x500a

The problem is in "argCont" function (in src/stm8/peep.c), it was failing to execute correctly the following rule:

replace restart {
    ld  a, %1
    xor a, #0x20
    ld  %1, a
} by {
    bcpl    %2, #5
    ; peephole 21-5 replaced or by bcpl.
} if notUsed('a'), operandsLiteral(%1), immdInRange(0 65535 '+' 0 %1 %2)

Because 'a' was being evaluated as used, because the instruction " ld a, 0x500a" contained 'a' in the source operand (0x500a). Looking at the code even other ways of failing were possible, for example, using symbols with x or y in its name using direct or indexed mode (ld a, _testx+0) would incorrectly evaluate x or y as being used .

New proposed code checks for direct memory and indirect addressing in a more reliable way, and searches explicitly for the argument after the "," if it detects the correct addressing mode.

I think every case should work ok now, but not fully tested.

[Visenri]

Here the regression tests, done with all my last patches, including:
https://sourceforge.net/p/sdcc/patches/362/

[Philipp Klaus Krause]

I'll leave it to you to iron out any remaining code style issues and apply this patch.

[Visenri]

Ok, I will have to recheck it with current version.
Also, I think that some issues may arise because of the last changes done to calling convention.
If in doubt I will remove the improvements I added for tail call optimization.

[Visenri]

A tiny modification in [r12425]:
Fixes many of the problematic cases for 'x' register being flagged as found in '0x' expressions.

This and many more problems were handled by the patch of this ticket.
An small improvement until I find the time to recheck this patch.

[Philipp Klaus Krause]

What is the current situation? Has everything that makes sense been applied (so we could close the ticket)? Are there parts left, that we might to apply, for which I should do some testing?

[Visenri]

I have been busy with other projects and this ticket has been open for too long.
I really would like to fix the problems described in this topic.
But maybe we have to rethink how to fix them, because other backends also suffer from similar problems:
See [#3291]

I see multiple issues:

1. Defects in asm parsing, some times flagging 'x' or 'y' as used when they are not, both in generated asm from compiler or inline asm.
2. Defects in asm parsing when some extra space or tab is present: my set of extra functions fixed this, but because other changes have been applied to the affected files, I have to review my old code to adapt it to the new changes. Also, I did other changes to code generator in other commits to normalize compiler output (SMT8), so this problem could be non existing now, but I'm not entirely sure about it.
3. Defects parsing inline ASM when spaces, comments or caps are used.

Issue number 1 is specific to STM8 and should be fixed with after me reviewing the code of this ticket.

I think issues 2 & 3 could and should be handled elsewhere, because those problems seem to exist in more backends.

Examples of possibly more generic solutions:

• The functions used to generate asm in code generators should make sure no extra spaces are present (STM8 seems to be OK after my changes to code generator, but nothing is checked systematically by the code itself).
• The functions that execute the replace in the rules should also parse their inputs to normalize the resulting asm code (no extra spaces, no extra comments and no uppercase).