#983 variable load wrongly optimized away

closed
z80 port (188)
5
2008-03-17
2005-09-16
No

Dear all,

I am not sure if this is Z80 port related at all, but for
the following code SDCC#1076 produces wrong code:

====================
#include <string.h>

typedef unsigned char UINT8;
typedef signed char INT8;

void globalReplace (char *st, char *pattern, char
*replacement)
/* replaces all occurrences of <pattern> in <st> by
<replacement> */
{
UINT8 replacementLength = strlen(replacement);
INT8 shiftOffset = replacementLength - strlen(pattern);

for (;;) {
/* find the first position of pattern */
char *ptr = strstr(st, pattern);

if (ptr == NULL) {
break;
} else {
/* do an in-place character shift by shiftOffset */
if (shiftOffset == 0) {
/* no shift */
} else {
...
}
...
}
...
}
...
}
====================

The Z80 assembler code (with the i-code wrapped
around for
clarity) looks like this:

====================
;sdcc-bug3.c:17: if (ptr == NULL) {
;
;ic:20: iTemp12 [k22 lr19:20 so:0]{ ia0 a2p0 re0 rm0
nos0 ru0 dp0}
; {char} = iTemp8 [k15 lr18:50 so:0]{ ia0 a2p0 re1
rm0 nos0
; ru0 dp0}{char generic* }{ sir@
_globalReplace_ptr_2_2}
; [_globalReplace_ptr_2_2] == 0x0 {void literal-
generic* }
;
; genCmpEq
; AOP_STK for _globalReplace_ptr_2_2
; genCmpEq: left 2, right 2, result 0
ld a,-4(ix)
or a,-3(ix)
jp z,00113$
00121$:

;ic:21: if iTemp12 [k22 lr19:20 so:0]{ ia0 a2p0 re0 rm0
nos0 ru0
dp0}{char} != 0 goto _return($13)

;sdcc-bug3.c:21: if (shiftOffset == 0) {
;
;ic:28: if iTemp13 [k23 lr13:52 so:0]{ ia0 a2p0 re0 rm0
; nos0 ru0 dp0}{char} != 0 goto _ifend_1($6)
;
; genIfx
or a,a
jp nz,00106$
====================

This means that the variable <shiftOffset> is not loaded,
but the compiler seems to assume it is already in the
accumulator.

Any ideas on that?

Best regards

Thomas

Discussion

  • Thomas Tensi

    Thomas Tensi - 2005-09-16

    source file triggering the error

     
  • Philipp Klaus Krause

    Logged In: YES
    user_id=564030
    Originator: NO

    I can not reproduce this problem in sdcc 2.6.5 #4749.

     
  • Maarten Brock

    Maarten Brock - 2007-04-15

    Logged In: YES
    user_id=888171
    Originator: NO

    I had no trouble reproducing this bug with 4747. It should stay open.

    I added a global UINT8 x and replaced all "..." by "x+=1;", "x+=2;", etc. to the above to have some code there. (Didn't see the attachment)

    I used --use-stdout --debug -V --dumpall --std-sdcc99 --i-code-in-asm -mz80

     
  • Nobody/Anonymous

    Logged In: NO

    I just tried again with #4749, I get:

    ;sdcc-bug3.c:17: if (ptr == NULL) {

    ;ic:20: iTemp12 [k22 lr18:19 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{char} = iTemp8 [k15 lr17:49 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{char generic* }{ sir@ _globalReplace_ptr_2_2}[_globalReplace_ptr_2_2] == 0x0 {void literal-generic* }
    ; genCmpEq
    ; AOP_STK for _globalReplace_ptr_2_2
    ; genCmpEq: left 2, right 2, result 0
    ld a,-4(ix)
    or a,-3(ix)
    jp Z,00113$
    ; peephole redirected jump.
    ; peephole removed unused label 00119$.
    ;ic:21: if iTemp12 [k22 lr18:19 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{char} != 0 goto _return($13)
    C$sdcc_bug3.c$21$3$4 = .
    .globl C$sdcc_bug3.c$21$3$4
    ;sdcc-bug3.c:21: if (shiftOffset == 0) {

    ;ic:27: if iTemp3 [k8 lr10:51 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{char}{ sir@ _globalReplace_shiftOffset_1_1}[_globalReplace_shiftOffset_1_1] == 0 goto _ifend_1($6)
    ; genIfx
    ; AOP_STK for _globalReplace_shiftOffset_1_1
    xor a,a
    or a,-2(ix)
    ; peephole removed redundant or after or.
    jp Z,00106$
    C$sdcc_bug3.c$28$4$6 = .
    .globl C$sdcc_bug3.c$28$4$6

    shiftOffset lives in -2(ix), so it is loaded before use. The important part seems to be:

    xor a,a
    or a,-2(ix)

    the bug reporter got

    or a,a

    instead. I've used the attached .c file, compiled with:
    sdcc --use-stdout --debug -V --dumpall --std-sdcc99 --i-code-in-asm -c -mz80 sdcc-bug3.c

    Philipp

     
  • Robert Larice

    Robert Larice - 2008-03-16

    Logged In: YES
    user_id=1840151
    Originator: NO

    the offending code can be simplified.
    #5087 still fails with invokation
    sdcc -mz80 thiscode.c
    {somehow the codegenerator assumes accu a survives the second call to bar()}

    ---<<8---------------
    extern char bar();

    void foo()
    {
    char aa, bb;

    aa = bar();

    for (;;) {

    bb = bar();

    if (!bb)
    break;

    if (aa == 0)
    return;

    }
    }
    --->>8---------------

    Robert Larice

     
  • Philipp Klaus Krause

    • assigned_to: nobody --> spth
     
  • Philipp Klaus Krause

    Logged In: YES
    user_id=564030
    Originator: NO

    It seems to be a bug in the register allocator. Or two. Somehow the register allocator thinks that both the call and the preceding conditional jump preserve a.
    If I just change it so that it thinks that conditional jumps never preserve a the generated code is correct, However I'll try to find a more intelligent solution.

    Philipp

     
  • Philipp Klaus Krause

    Logged In: YES
    user_id=564030
    Originator: NO

    Fixed in #5107.

     
  • Philipp Klaus Krause

    • status: open --> closed
     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks