#3640 mcs51 regression: missed constant evaluations

[Oleg Endo]

While updating my local tree from SDCC 4.1.6 SVN r12575 to SDCC 4.3.2 SVN r14334 I've noticed some code size increases on mcs51.

It looks like some constant evaluations aren't done at compile time anymore. Please see the attached screenshot for a side-by-side diff which exhibits the issue. Most cases in my software are of that type.

I'm not sure whether it's the constant propagation, the register tracking in mcs51 or something else. However, I was able to extract a small example reproducer.

enum mask_1_e
{
  shift_1 = 0,
  mask_1 = 1 << shift_1
};

enum mask_2_e
{
  shift_2 = 4,
  mask_2 = 1 << shift_2
};

inline uint8_t make_val_1 (bool val)
{
  return (val << shift_1) & mask_1;
}
inline uint8_t make_val_2 (bool val)
{
  return (val << shift_2) & mask_2;
}

__xdata uint8_t __at (0xFFFA) x_reg;

void set_x_reg (void)
{
  x_reg = make_val_1 (true) | make_val_2 (true);
}

Compiled with: sdcc -c --model-large --std-sdcc2x -mmcs51 --stack-auto --nogcse --fverbose-asm

_set_x_reg:
    ar7 = 0x07
    ar6 = 0x06
    ar5 = 0x05
    ar4 = 0x04
    ar3 = 0x03
    ar2 = 0x02
    ar1 = 0x01
    ar0 = 0x00
;   bug.c:907: return (val << shift_1) & mask_1;
;   genAnd
    mov a,#0x01
    anl a,#0x01
    mov r7,a
;   bug.c:911: return (val << shift_2) & mask_2;
;   genAnd
;   genFromRTrack 0x10==swap(0x01)
    swap    a
    anl a,#0x10
;   bug.c:918: x_reg = make_val_1 (true) | make_val_2 (true);
;   genOr
;   Peephole 302    mov r6,a removed
    mov dptr,#_x_reg
;   Peephole 177.d  removed redundant mov
    orl a,r7
    movx    @dptr,a
;   Peephole 500    removed redundant label 00103$
;   bug.c:919: }
    ret

[Oleg Endo]

These are the code size stats (in bytes) for one of my apps.
Before: SVN r12575 + my mcs51 peephole work
After: SVN r14340 + my mcs51 peephole work

   CSEG     13310  ->  13368   +58
   BANK1    38195  ->  41658   +3463
   BANK2    32634  ->  32593   -41
   BANK3    44236  ->  44263   +27
   BANK4     9922  ->  11536   +1614
   BANK5    10309  ->  11781   +1472

[Oleg Endo]

Can anybody reproduce/confirm/refute this?

[Konstantin Kim]

SDCC 4.2.8_13656 - ok
SDCC_4.3.1_14235 - buggy

[Philipp Klaus Krause]

I cannot reproduce this issue in current SDCC [r14403] on my Debian GNU/Linux testing system. The generated code is:

_set_x_reg:
    ar7 = 0x07
    ar6 = 0x06
    ar5 = 0x05
    ar4 = 0x04
    ar3 = 0x03
    ar2 = 0x02
    ar1 = 0x01
    ar0 = 0x00
    mov dptr,#_x_reg
    mov a,#0x11
    movx    @dptr,a
    ret

At the end of CSE, I already see the x_reg = 0x11, though the computations from all the functions are still there, too; but dead code elimination does optimize them out quickly.
Can you please recheck using recent svn or snapshot?

[Konstantin Kim]

buggy..
SDCC : mcs51/z80/z180/r2k/r2ka/r3ka/sm83/tlcs90/ez80_z80/z80n/r800/ds390/pic16/pic14/TININative/ds400/hc08/s08/stm8/pdk13/pdk14/pdk15/mos6502 TD- 4.3.3 #14398 (MINGW64)

sdcc -c --model-large --std-sdcc2x -mmcs51 --stack-auto --nogcse test.c

_set_x_reg:
    ar7 = 0x07
    ar6 = 0x06
    ar5 = 0x05
    ar4 = 0x04
    ar3 = 0x03
    ar2 = 0x02
    ar1 = 0x01
    ar0 = 0x00
;   test.c:18: return (val << shift_1) & mask_1;
    mov a,#0x01
    anl a,#0x01
    mov r7,a
;   test.c:22: return (val << shift_2) & mask_2;
    swap    a
    anl a,#0x10
;   test.c:29: x_reg = make_val_1 (true) | make_val_2 (true);
    mov dptr,#_x_reg
    orl a,r7
    movx    @dptr,a
;   test.c:30: }
    ret

"--std-sdcc2x" is suspected one

sdcc -c -mmcs51 --std-sdcc2x -c test.c -> deoptimized

sdcc -c -mmcs51 -c test.c -> ok

[Philipp Klaus Krause]

On my laptop (Debian GNU/Linux testing on amd64), all these 3 different command lines result in the expected optimization.
I'll try to test a bit on other systems, too.

Can you post the exact source file you use (after all your code posted above is a bit incomplete)? For my tests, I used the attached one.

P.S.: Having tested on two other systems (Debian GNU/Linux unstable on ppc64 and Debian GNU/Linux stable on aarch64), I also get the optimized result there.

[Konstantin Kim]

your one seems not affected.
I use original Oleg code .

following line made it sensitive to "--std-sdcc2x" :

x_reg = make_val_1 (1) | make_val_2 (1);

[Philipp Klaus Krause]

Yes, thanks! What matters is apparently the type of true. The optimization apparently works it is of type int, but not when it is of type _Bool.

[Philipp Klaus Krause]

Looks like we have two bugs here:
1) The old optimization in CSE doesn't work correctly for true of type _Bool here.
2) Generalized constant propagation had a bug that resulted in it not working correctly on the & here.

2) is fixed in [r14404], but 1) is still there, so when compiling with --nogenconstprop, one still gets the unoptimized code.

[Philipp Klaus Krause]

1) is a mcs51-specific interaction between two optimizations: for mc51, the _Boolgets optimized into a __bit here, which then apparently prevents further optimization in CSE. And this issue only gets triggered if we use the C2X keyword true (as opposed to something like (_Bool)1).

Fixed in [r14405].