#3533 stm8 erratic a register value before subroutine jump

[Francesco]

Hi,
3: toolchain-sdcc @ 1.40200.0 (4.2.0)
2:

sdcc: sdcpp -nostdinc -Wall -std=c11 -D"F_CPU=16000000L" -D"PLATFORMIO=60105" -D"STM8S_BLUE" -D"STM8S103" -I"include" -I"src" -obj-ext=.rel -D__SDCC_STACK_AUTO -D__SDCC_CHAR_UNSIGNED -D__SDCC_MODEL_MEDIUM -D__SDCC_INT_LONG_REENT -D__SDCC_FLOAT_REENT
-D__SDCCCALL=1 -D__SDCC=4_2_0 -D__SDCC_VERSION_MAJOR=4 -D__SDCC_VERSION_MINOR=2 -D__SDCC_VERSION_PATCH=0 -D__SDCC_REVISION=13081 -D__SDCC_stm8 -D__STDC_NO_COMPLEX__=1 -D__STDC_NO_THREADS__=1 -D__STDC_NO_ATOMICS__=1 -D__STDC_NO_VLA__=1 -D__STDC_ISO_10646__=201409L -D__STDC_UTF_16__=1 -D__STDC_UTF_32__=1 -isystem "C:\Users\fra.platformio\packages\toolchain-sdcc\bin..\include\stm8" -isystem "C:\Users\fra.platformio\packages\toolchain-sdcc\bin..\include" "src\stm8_009_lcd-HD44780\main.c"
sdcc: Generating code...
sdcc: Calling assembler...
sdcc: sdasstm8 -plosgffw "".pio\build\stm8sblue\src\stm8_009_lcd-HD44780\main".asm"
sdcc: Calling preprocessor...
sdcc: sdcpp -nostdinc -Wall -std=c11 -D"F_CPU=16000000L" -D"PLATFORMIO=60105" -D"STM8S_BLUE" -D"STM8S103" -I"include" -I"src" -obj-ext=.rel -D__SDCC_STACK_AUTO -D__SDCC_CHAR_UNSIGNED -D__SDCC_MODEL_MEDIUM -D__SDCC_INT_LONG_REENT -D__SDCC_FLOAT_REENT
-D__SDCCCALL=1 -D__SDCC=4_2_0 -D__SDCC_VERSION_MAJOR=4 -D__SDCC_VERSION_MINOR=2 -D__SDCC_VERSION_PATCH=0 -D__SDCC_REVISION=13081 -D__SDCC_stm8 -D__STDC_NO_COMPLEX__=1 -D__STDC_NO_THREADS__=1 -D__STDC_NO_ATOMICS__=1 -D__STDC_NO_VLA__=1 -D__STDC_ISO_10646__=201409L -D__STDC_UTF_16__=1 -D__STDC_UTF_32__=1 -isystem "C:\Users\fra.platformio\packages\toolchain-sdcc\bin..\include\stm8" -isystem "C:\Users\fra.platformio\packages\toolchain-sdcc\bin..\include" "src\stm8_009_lcd-HD44780\stm8_HD44780.c"
sdcc: Generating code...
sdcc: Calling assembler...
sdcc: sdasstm8 -plosgffw "".pio\build\stm8sblue\src\stm8_009_lcd-HD44780\stm8_HD44780".asm"

1: the assembly generated for the "void LCD_goto" function seems to be erratic. In particular, as you can see from the generated assembly snippet below, the accumulator register "a" doesn't contain the proper value before jumping to the LCD_write subroutine. There is a "pop a" instruction that shouldn't be there.

LCD_goto function

void LCD_goto(uint8_t col, uint8_t row)
{
    LCD_PIN_WRITE(LCD_RS_PORT, LCD_RS_PIN, 0);

    // Attention! if the next line is missing the code doesn't
    // work properly. It might be a problem of the SDCC compiler
    //LCD_delay_ms(1);

    if (row == 0)
        LCD_write(0x80 + col); // DB7=1, 0x80 + offset
    else
        LCD_write(0xC0 + col); // 0x80+0x40 = 0xC0
}

ASM for LCD_goto

_LCD_goto:
    ld  xl, a
;   src\stm8_009_lcd-HD44780\stm8_HD44780.c: 35: LCD_PIN_WRITE(LCD_RS_PORT, LCD_RS_PIN, 0);
    bres    0x500a, #6
;   src\stm8_009_lcd-HD44780\stm8_HD44780.c: 42: LCD_write(0x80 + col); // DB7=1, 0x80 + offset
    ld  a, xl
;   src\stm8_009_lcd-HD44780\stm8_HD44780.c: 41: if (row == 0)
    tnz (0x03, sp)
    jrne    00102$
;   src\stm8_009_lcd-HD44780\stm8_HD44780.c: 42: LCD_write(0x80 + col); // DB7=1, 0x80 + offset
    add a, #0x80
    ldw x, (1, sp)
    ldw (2, sp), x
    pop a
    jp  _LCD_write

Thanks for your support.

[Basil Hussain]

Yes, that code is erroneous. I can see it has attempted to optimise the call to LCD_write() by shuffling the return address up the stack, overwriting the row argument and discarding the excess byte, before jumping to the new subroutine. But as you say, the discarding of a stack byte with pop a is overwriting the argument to LCD_write(), which should be the value in A register.

Can you try compiling with the --fverbose-asm option? This might be a problem caused by the peephole optimiser. Compiling with that option will show if any code has been rewritten or removed as a result of an optimisation rule. Please post the assembly code.

Also, you could try compiling with --no-peep to disable peephole optimisations and see if that avoids the problem.

[Francesco]

Hi, thanks a lot for your response.
In the attachment below you can find the assembly code generated as in the original example, with --fverbose-asm and also with --no-peep.

Even disabling peep optimization doesn't seem to solve the problem.
Thanks for your work.

[Basil Hussain]

Seems not to be caused by the peephole optimiser.

I notice that LCD_goto() is not the only place where a function argument value is clobbered before the function is called with that argument. There is code in LCD_write() where this happens as well. The un-optimised assembly:

;   src\stm8_009_lcd-HD44780_buggy\stm8_HD44780.c: 18: LCD_write4(cmd & 0x0f);
; genAnd
    ld  a, (0x01, sp)
    and a, #0x0f
; genSend
; genCall
    pop a
    jp  _LCD_write4

There is a rogue "pop a" there too before the call to LCD_write4() that wipes out the value it has just and-ed with 0x0f. Even worse, in the optimised code, this error also causes the "and" to be transformed into something even more incorrect:

;   src\stm8_009_lcd-HD44780_buggy\stm8_HD44780.c: 18: LCD_write4(cmd & 0x0f);
; genAnd
    ld  a, (0x01, sp)
    bcp a, #0x0f
; peephole 500 replaced 'and' by 'bcp'.
; genSend
; genCall
    pop a
    jp  _LCD_write4

The "and" is replaced with "bcp", because rule 500's condition that "a" register's value is unused is satisfied due to the subsequent "pop a".

It would seem the problem is being caused by the actions of SDCC's genCall() code in stm8/gen.c. I am not familiar with the inner workings of code generation, so someone else will have to look at this issue.

[Philipp Klaus Krause]

I cannot reproduce this pop a issue. Which compiler version did you use?

I suspect that this pop a issue is a duplicate of [bugs:#3379], which was fixed in April.

This hold for both the pop a in LCD_write and the one in LCD_goto.

[Basil Hussain]

They appear to have been using 4.2.0 #13081. If that bug was fixed in #13380, then trying a recent snapshot build of SDCC is the course of action to resolve the problem.

However, Francesco also appears to be using PlatformIO, so unsure how to suggest they should go about updating the copy of SDCC toolchain bundled there.

[Francesco]

I confirm that the problem is not occurring using 4.2.0 #13780.
Thanks for your help.

As for the platformIO environment, the only way I could find to make it use a specific snapshot build is the following:

• download the latest sdcc snapshot build (win64 in my case)
• extract the content in a local folder (e.g /sdcc)
• within this folder create a new file package.json similar to the one you could find in the C:/users/USER/.platformio/packages/toolchain-sdcc , preferably with the correctly updated version info.

Now in the project relatedplatformio.ini file we need to override the defaulttoolchain-sdcc package by adding the following instruction:

;override toolchain-sdcc package folder
platform_packages = 
   toolchain-sdcc@file://C:/Users/fra/sdcc

The following is an example platformio.ini for stm8

[env:stm8sblue]
platform = ststm8
board = stm8sblue
upload_protocol = stlinkv2

;override toolchain-sdcc package folder
platform_packages = 
   toolchain-sdcc@file://C:/Users/fra/sdcc