#3291 inline assembler comments prevents optimizations

[Konstantin Kim]

SDCC : mcs51/z80/z180/r2k/r2ka/r3ka/gbz80/tlcs90/ez80_z80/z80n/ds390/pic16/pic14/TININative/ds400/hc08/s08/stm8/pdk13/pdk14/pdk15 4.1.12 #12697 (MINGW32)

command line:
sdcc -mpdk13 -DPMS150C --opt-code-size --fverbose-asm --peep-asm --peep-return test.c

test code:

volatile __sfr __at(0x10)   _pa;

void main() {
    __asm__("mov a,#00 ;");
    __asm__("mov a,#01; ");
    __asm__("mov a,#02; some comments");
    __asm__("mov a,#03");
    __asm__("mov __pa,a");
}

resilt is:

      000000                        105 _main:
                                    106 ;   ../test.c: 4: __asm__("mov a,#00 ;");
                                    107 ;   genInline
      000000 00 17                  108     mov a,#00 ;
                                    109 ;   ../test.c: 5: __asm__("mov a,#01; ");
                                    110 ;   genInline
                                    111 ; peephole 0 removed dead load into a from #01;.
                                    112 ;   ../test.c: 6: __asm__("mov a,#02; some comments");
                                    113 ;   genInline
      000002 02 17                  114     mov a,#02; some comments
                                    115 ;   ../test.c: 7: __asm__("mov a,#03");
                                    116 ;   genInline
      000004 03 17                  117     mov a,#03
                                    118 ;   ../test.c: 8: __asm__("mov __pa,a");
                                    119 ;   genInline
      000006 D0 05                  120     mov __pa,a
                                    121 ; genLabel
                                    122 ; peephole j0 removed unused label 00101$.
                                    123 ;   ../test.c: 9: }
                                    124 ; genEndFunction
      000008 3A 00                  125     ret

Expected to be removed all loads before line "117 mov a,#03".

[Philipp Klaus Krause]

--peep-asm is rarely used, and unfortunately tends to not work as well as one would hope (for most ports). AFAIK, it hasn't been a priority in the past, though patches are of course welcome (as long as they don't break anything else).

P.S.: In particular, I think the interaction between inline asm and notUsed could be improved.

[Konstantin Kim]

I hope that there is not hidden bug as per sensitivity to space position.
Following two lines have different effects:

"mov a,#00 ;"
"mov a,#01; "

[Gabriele Gorla]

From my experiments on the mos6502 port it seems the peephole has trouble matching any line with comments.

[Gabriele Gorla]

the issue is in SDCCpeeph.c at the end of matchLine()

 /* skip trailing whitespaces */
  if (*s)
    while (ISCHARSPACE (*s))
      s++;

  if (*d)
    while (ISCHARSPACE (*d))
      d++;

  /* after all this if only one of them
     has something left over then no match */
  if (*s || *d)
    return FALSE;

to match
"mov a,#00 ;"
the trailing spaces eater should also discard comments before checking if it is a match.
something like this before checking for the last match should do the trick.

  /* skip trailing comments */
  if(*s==';')
    while (*s)
      s++;

correctly matching
"mov a,#01; "
is harder as the parser uses only ISCHARSPACE() and comma to separate the tokens. so the ; becomes part of the third token #01; instead of #01

[Konstantin Kim]

Thanks! Nice catch!

[Gabriele Gorla]

issue should be fixed with [r12908]

[Konstantin Kim]

I see that it works for case:
__asm__("mov a,#00 ;");

Still following case is not handled:
__asm__("mov a,#02; some comments");

_main:
;   ../test.c: 4: __asm__("mov a,#00 ;");
; ic:   2: inline
;   genInline
; peephole 0 removed dead load into a from #00.
;   ../test.c: 5: __asm__("mov a,#01; ");
; ic:   3: inline
;   genInline
; peephole 0 removed dead load into a from #01;.
;   ../test.c: 6: __asm__("mov a,#02; some comments");
; ic:   4: inline
;   genInline
    mov a,#02; some comments
;   ../test.c: 8: __asm__("mov a,#03");
; ic:   5: inline
;   genInline
    mov a,#03
;   ../test.c: 9: __asm__("mov __pa,a");
; ic:   6: inline
;   genInline
    mov __pa,a
; ic:   7:  _return($1) :
; genLabel
; peephole j0 removed unused label 00101$.
;   ../test.c: 10: }
; ic:   8:  eproc _main [k1 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void function ( ) fixed}
; genEndFunction
    ret

[Gabriele Gorla]

I think I found the issue in bindVar (should be fixed starting with [r13003]
The testcase provided now works as expected.

[Sebastian Riedel]

Does this work now?
Alternatively we could rewrite output assembly like we do for labels.
I’m not sure if the size counting functions can handle lines that end with comments.

[Konstantin Kim]

As per my tests I do not see issues anymore.
IMHO fixed