#2873 incorrect code generated for return statement (regression)

[Eric Rullens]

One would expect the same code to be generated for these two functions.

int f1(void)
{
  return 6;
}

int f2(void)
{
  return (2 ? 3 : 4) + (2 ? 3 : 4);
}

However in recent SDCC, the code generated for f2 is incorrect. I tried this for MCS51 and STM8. This might be related to bug #2872.

The attached a file contains the code and information as shown below.

SDCC : mcs51/z80/z180/r2k/r3ka/gbz80/tlcs90/ez80_z80/ds390/pic16/pic14/TININative/ds400/hc08/s08/stm8 3.8.5 #10914 (MINGW32) published under GNU General Public License (GPL)

Compiles, but not (return) code generated for f2 when compiling for STM8, or only dpl set when compiling for MCS51.

MCS51
sdcc -S -mmcs51 test-2.c

_f1:
; test-2.c:3: return 6;
mov dptr,#0x0006
; test-2.c:4: }
ret

_f2:
; test-2.c:8: return (2 ? 3 : 4) + (2 ? 3 : 4);
mov dpl,#0x06
; test-2.c:9: }
ret

MCS51 large
sdcc -S -mmcs51 --model-large test-2.c

_f1:
; test-2.c:3: return 6;
mov dptr,#0x0006
; test-2.c:4: }
ret

_f2:
; test-2.c:8: return (2 ? 3 : 4) + (2 ? 3 : 4);
mov dpl,#0x06
; test-2.c:9: }
ret

STM8
sdcc -S -mstm8 test-2.c

_f1:
; test-2.c: 3: return 6;
ldw x, #0x0006
; test-2.c: 4: }
ret

_f2:
; test-2.c: 8: return (2 ? 3 : 4) + (2 ? 3 : 4);
; test-2.c: 9: }
ret

STM8 large
sdcc -S -mstm8 --model-large test-2.c

_f1:
; test-2.c: 3: return 6;
ldw x, #0x0006
; test-2.c: 4: }
retf

_f2:
; test-2.c: 8: return (2 ? 3 : 4) + (2 ? 3 : 4);
; test-2.c: 9: }
retf

SDCC : mcs51/z80/z180/r2k/r3ka/gbz80/tlcs90/ds390/pic16/pic14/TININative/ds400/hc08/s08/stm8 3.5.0 #9253 (Jun 20 2015) (MINGW32) published under GNU General Public License (GPL)

No issues are seen. Resulting code is ok. (STM8 large model was not supported yet.)

_f1:
; test-2.c:3: return 6;
mov dptr,#0x0006
ret

_f2:
; test-2.c:8: return (2 ? 3 : 4) + (2 ? 3 : 4);
mov dptr,#0x0006
ret

_f1:
; test-2.c: 3: return 6;
ldw x, #0x0006
ret

_f2:
; test-2.c: 8: return (2 ? 3 : 4) + (2 ? 3 : 4);
ldw x, #0x0006
ret

[Eric Rullens]

For some reason the attached file is empty. Trying again...

[Philipp Klaus Krause]

The AST looks ok for the second function, but the iCode is wrong already at dumpraw0.

Philipp

[Erik Petrich]

I believe I have a fix for this, but there are some regression failures in literalop.c:

--- FAIL: "Assertion failed" on (signed char ) -1 * (unsigned char ) 0xfffffff7 == (sizeof(int) == 2 ? 0xff09 : 0xffffff09) at gen/s08/literalop/literalop_type_char.c:82
--- FAIL: "Assertion failed" on (signed char ) -2 * (unsigned char ) 0x8004 == (sizeof(int) == 2 ? 0xfff8 : 0xfffffff8) at gen/s08/literalop/literalop_type_char.c:86
--- FAIL: "Assertion failed" on (signed char ) -1 * (unsigned char ) 0xfffffff7 == (sizeof(int) == 2 ? 0xff09 : 0xffffff09) at gen/s08/literalop/literalop_type_char.c:137
--- FAIL: "Assertion failed" on (signed char ) -2 * (unsigned char ) 0x8004 == (sizeof(int) == 2 ? 0xfff8 : 0xfffffff8) at gen/s08/literalop/literalop_type_char.c:141
--- FAIL: "Assertion failed" on (unsigned char ) -12 / (signed char ) -3 == (sizeof(int) == 2 ? 0xffaf : 0xffffffaf) at gen/s08/literalop/literalop_type_char.c:150
--- FAIL: "Assertion failed" on uc / (signed char ) -3 == (sizeof(int) == 2 ? 0xffaf : 0xffffffaf) at gen/s08/literalop/literalop_type_char.c:177

These failures all seem similar, so just consider the first one. My understanding is that 0xff09 is type unsigned int and 0xffffff09 is type unsigned long, so the integer promotion rules require the result of the conditional operator to be type unsigned long. The assertion fails because the int -247 promoted to unsigned long is not equal to the unsigned int 0xff09 promoted to unsigned long. Shouldn't these hexadecimal constants be cast to signed int / signed long?

[Philipp Klaus Krause]

Yes, it looks to me as if the constants should be cast to int (both of them).

Philipp

[Eric Rullens]

I tried the fix and can report that the issue is solved. In the test code, f1 and f2 now yield the same (correct) result.

[Erik Petrich]

I forgot to close this earlier. Fixed about a week ago in [r10979]