#3857 Ternary operator in struct assignment using a function call fails

[Under4Mhz]

Using the ternary operator in the assignment below results in the incorrect value being assigned to the struct.

/// GPL 2.0 or later
#include <stdio.h>
#include <stdint.h>
#include <stdbool.h>

typedef struct {

    uint8_t x;
    uint8_t y;

} maths_point_u4;

maths_point_u4 room1 = { 11, 12 };

const maths_point_u4 *RoomGet(void) { return &room1; }

static void PositionAdjust2( const maths_point_u4 *room, maths_point_u4 *room_new ) {

    printf("*Room %p\n", room);

    *room_new = room ? *room : *RoomGet();
}

void main( void ) {

    maths_point_u4 room2 = { 5, 6 };

    PositionAdjust2( 0, &room2 );

    printf("Room1 %d, %d\n", room1.x, room1.y);
    printf("Room2 %d, %d\n", room2.x, room2.y);
}

#ifdef __SDCC
__sfr __at 0xff sif;
int putchar( int c ) {

    sif = 'p';
    sif = c;

    return c;
}
#endif

$ sdcc -mz80 --fverbose-asm ./assign_struct2.c  -o assign_struct2.ihx && 0> Loading from assign_struct2.ihx
Simulation started, PC=0x000000
*Room 0x0000
Room1 11, 12
Room2 253, 43

$ gcc ./assign_struct2.c && ./a.out
*Room (nil)
Room1 11, 12
Room2 11, 12

SDCC : z80/sm83/z80n/mos6502/mos65c02/f8 TD- 4.5.2 #15451 (Linux)

[Under4Mhz]

sdcc -mz80 --fverbose-asm ./assign_struct2.c  -o assign_struct2.ihx && ucsim_z80 -I if=outputs[0xff] assign_struct2.ihx

[Janko Stamenović]

I've simplified it.. Starting from:

typedef struct {
    char x;
} S;

S s = { 11 };
const S* f(void) { return &s; }

S result;

static void g( const S* p ) {
    result = p ? *p : *f();
}

int main() {
    g( 0 );
    assert( result.x == 11 );
    return 0;
}

The generated code of the function g is:

_g:
    ex  de, hl
;testbug.c:23: result = p ? *p : *f();
    ld  a, d
    or  a, e
    jr  NZ, 00104$
    call    _f
    ex  de,hl
    ld  e, (hl)
    inc hl
    ld  d, (hl)
00104$:
    ex  de, hl
    ld  de, #_result
    ld  a, (hl)
    ld  (de), a
;testbug.c:24: }
    ret

and the bad part is what happens after the call _f. The correct code without unnecessary steps would be:

_g:    
       ; HL contains p 
        ld      a, h
        or      a, l
        jr      NZ, 00104$   ; if ( p ) goto 00104$;
        call    _f           ; DE := f()
        ex      de, hl       ; p := DE
00104$:
        ld      de, #_result  
        ld      a, (hl)       ; A := *HL
        ld      (de), a       ; result := A
        ret

Comparing with what we have now we see that the bad code does more than it should, and it's after the function call:

    ld  e, (hl)
    inc hl
    ld  d, (hl)

and there we see that it's clearly one unneeded dereference (like that dereference would get a 2-byte value, e.g. a pointer!) which doesn't match the rest of the code.

So the function returns a pointer to a structure, but somehow expression handles that part as if the function would return a pointer to a pointer to a structure, and dereferences that before a pointer is propagated to the common result := *HL.

[Janko Stamenović]

The relevant verbose part:

    call    _f
;   genMove_o size 2 result type 2 source type 2 hl_dead 1
    ex  de, hl
;   genMove_o size 0 result type 2 source type 1 hl_dead 1
;ic:11:     iTemp3 [k8 lr7:13 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-struct __00000000 __code* fixed}[e d ] = @[iTemp5 [k11 lr10:11 so:0]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{const-struct __00000000 generic* fixed}[l h ] + 0x0 {const-unsigned-char literal}]
;   genPointerGet
;fetchPairLong
;   _moveFrom_tpair_()
    ld  e, (hl)
    inc hl
;   _moveFrom_tpair_()
    ld  d, (hl)

[Janko Stamenović]

If the function would use a "workaround":

result = *( p ? p : f() );

then the result would be correct, even if it's still one ex de, hlmore than it would be optimal:

; Function g
; ---------------------------------
_g:
    ex  de, hl
;testbug.c:24: result = *( p ? p : f() );
    ld  a, d
    or  a, e
    jr  NZ, 00104$
    call    _f
00104$:
    ex  de, hl
    ld  de, #_result
    ld  a, (hl)
    ld  (de), a
;testbug.c:25: }
    ret

verbose:

;   ---------------------------------
; Function g
; ---------------------------------
;   Register assignment is optimal.
; Stack space usage: 0 bytes.
_g:
;ic:2: iTemp0 [k2 lr3:7 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{const-struct __00000000 generic* fixed}{ sir@ _g_p_10000_3}[e d ] = recv _g [k1 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void function ( const-struct __00000000 generic* auto) fixed}
;   genReceive
;   genMove_o size 2 result type 2 source type 2 hl_dead 1
    ex  de, hl
;   genMove_o size 0 result type 2 source type 1 hl_dead 1
;check.c:11: result = *( p ? p : f() );
;ic:3:  iTemp1 [k6 lr4:5 so:0]{ ia0 a2p0 re0 rm1 nos0 ru0 dp0}{struct __00000000 near* fixed}[remat] = &[_result [k5 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{struct __00000000 fixed} , 0x0 {const-unsigned-char literal}]
;   skipping iCode since result will be rematerialized
;ic:4:  iTemp2 [k7 lr5:15 so:0]{ ia0 a2p0 re0 rm1 nos0 ru0 dp0}{void generic* fixed}[remat] = (void generic* fixed)iTemp1 [k6 lr4:5 so:0]{ ia0 a2p0 re0 rm1 nos0 ru0 dp0}{struct __00000000 near* fixed}[remat]
;   skipping iCode since result will be rematerialized
;ic:5:  if iTemp0 [k2 lr3:7 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{const-struct __00000000 generic* fixed}{ sir@ _g_p_10000_3}[e d ] == 0 goto iTempLbl0($3)
;   genIfx
    ld  a, d
    or  a, e
    jp  Z, 00103$
;ic:6:  iTemp3 [k8 lr7:12 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-struct __00000000 generic* fixed}[e d ] := iTemp0 [k2 lr3:7 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{const-struct __00000000 generic* fixed}{ sir@ _g_p_10000_3}[e d ]
;   genAssign
;   (locations are the same)
;ic:7:   goto iTempLbl1($4)
;   genGoto
    jp  00104$
;ic:8:  iTempLbl0($3) :
;   genLabel
00103$:
;ic:9:  iTemp3 [k8 lr7:12 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-struct __00000000 generic* fixed}[e d ] = call _f [k9 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-struct __00000000 generic* function ( void ) fixed}
;   genCall
    call    _f
;   genMove_o size 2 result type 2 source type 2 hl_dead 1
;ic:11:  iTempLbl1($4) :
;   genLabel
00104$:
;ic:12:     iTemp5 [k11 lr12:15 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-void generic* fixed}[e d ] = (const-void generic* fixed)iTemp3 [k8 lr7:12 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-struct __00000000 generic* fixed}[e d ]
;   genCast
;   (locations are the same)
;ic:13:     send iTemp2 [k7 lr5:15 so:0]{ ia0 a2p0 re0 rm1 nos0 ru0 dp0}{void generic* fixed}[remat]{argreg = 1}
;   genBuiltIn
;   genMove_o size 2 result type 2 source type 2 hl_dead 1
    ex  de, hl
;   genMove_o size 0 result type 2 source type 1 hl_dead 1
;   genMove_o size 2 result type 2 source type 6 hl_dead 0
    ld  de, #_result
    ld  a, (hl)
    ld  (de), a
;ic:14:     send iTemp5 [k11 lr12:15 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-void generic* fixed}[e d ]{argreg = 2}
;   skipping generated iCode
;ic:15:     send 0x1 {unsigned-int literal}{argreg = 0}
;   skipping generated iCode
;ic:16:     iTemp6 [k15 lr16:16 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void generic* fixed} = call ___builtin_memcpy [k4 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void generic* function __builtin__ ( void generic* fixed, const-void generic* fixed, unsigned-int fixed) fixed}
;   skipping generated iCode
;ic:18:  _return($1) :
;   genLabel
00101$:
;check.c:12: }

[Janko Stamenović]

This test that tests three forms of the expression and both branches. Only !p in g1 fails currently.

typedef struct {
    char x;
} S;

S s1 = { 11 };
S s2 = { 22 };
const S* f(void) { return &s1; }

S result;

static void g1( const S* p ) {
    result = p ? *p : *f();
}

static void g2( const S* p ) {
    result = *( p ? p : f() );
}

static void g3( const S* p ) {
    if ( p ) { result = *p;
    } else result = *f();
}


int main() {
    g1( 0 );
    assert( result.x == 11 );
    g1( &s2 );
    assert( result.x == 22 );
    g2( 0 );
    assert( result.x == 11 );
    g2( &s2 );
    assert( result.x == 22 );
    g3( 0 );
    assert( result.x == 11 );
    g3( &s2 );
    assert( result.x == 22 );
    return 0;
}