STM8+SDCC - Dlaczego właśnie tak? Studium pewnego przypadku

Pozostałe układy mikrokontrolerów, układy peryferyjne i inne, nie mieszczące się w powyższych kategoriach.
Awatar użytkownika
ZbeeGin
User
User
Posty: 492
Rejestracja: sobota 08 lip 2017, 17:16
Lokalizacja: Śląsko-Zagłębiowska Metropolia
Kontaktowanie:

STM8+SDCC - Dlaczego właśnie tak? Studium pewnego przypadku

Postautor: ZbeeGin » poniedziałek 11 sty 2021, 13:53

Napotkałem na pewien "irytujący problem" z kompilatorem SDCC (obecnie 4.0.0) przy przeniesieniu kodu z STM32 na STM8 w związku ze zmianą sprzętową w urządzeniu (tańsza wersja).

Korzystam z bibliotek producenta SPL również kompilowanych przez SDCC. Mamy tam plik zawierający zbiór funkcji do obsługi portów GPIO. Istotna tu będzie tu jedna z funkcji odczytu stanu pinu na porcie:

Code: Select all

/**
* @brief Reads the specified GPIO input data pin.
* @param GPIOx : Select the GPIO peripheral number (x = A to I).
* @param GPIO_Pin : Specifies the pin number.
* @retval BitStatus : GPIO input pin status.
*/
BitStatus GPIO_ReadInputPin(GPIO_TypeDef* GPIOx, GPIO_Pin_TypeDef GPIO_Pin)
{
return ((BitStatus)(GPIOx->IDR & (uint8_t)GPIO_Pin));
}


Funkcji przekazujemy adres struktury GPIOx zawierającej rejestry portu oraz maskę wyłuskującą dany pin, którą zdefiniowano tak:

Code: Select all

typedef enum
{
GPIO_PIN_0 = ((uint8_t)0x01), /*!< Pin 0 selected */
GPIO_PIN_1 = ((uint8_t)0x02), /*!< Pin 1 selected */
(...)
}GPIO_Pin_TypeDef;


W rezultacie funkcja powinna zwrócić jedną z wartości zdefiniowaną w taki sposób:

Code: Select all

typedef enum {RESET = 0, SET = !RESET} FlagStatus, ITStatus, BitStatus, BitAction;


czyli wartości RESET odpowiada 0, a wartości SET odpowiada 1, jako pierwsza najbliższa następna wartość niezerowa.

Weźmy na przykład taki prosty kod:

Code: Select all

for(;;) {
if(GPIO_ReadInputPin(GPIOC, GPIO_PIN_7))
GPIO_WriteHigh(GPIOE, GPIO_PIN_5);
else
GPIO_WriteLow (GPIOE, GPIO_PIN_5);
}


W tym wypadku kod będzie działał jak należy. W kodzie maszynowym będzie to wyglądać tak:

Code: Select all

00105$:
; main.c: 11: if(GPIO_ReadInputPin(GPIOC, GPIO_PIN_7))
push #0x80 ;maska pin7 0x80 na stos
push #0x0a
push #0x50 ;adres GPIOC na stos
call _GPIO_ReadInputPin ;wywołanie funkcji bibliotecznej
addw sp, #3 ;szybkie zdjęcie ze stosu
tnz a ;test negative or zero dla akumulatora (rezultatu)
jreq 00102$ ;przeskocz jesli zero
; main.c: 12: GPIO_WriteHigh(GPIOE, GPIO_PIN_5);
push #0x20 ; przygotuj WriteHigh
push #0x14
push #0x50
call _GPIO_WriteHigh
addw sp, #3
jra 00105$ ;i pętla się zamyka
00102$:
; main.c: 14: GPIO_WriteLow (GPIOE, GPIO_PIN_5);
push #0x20 ;przygotuj WriteLow
push #0x14
push #0x50
call _GPIO_WriteLow
addw sp, #3
jra 00105$ ;i pętla się zamyka


Kompilator sobie zamienił miejscami szyk poleceń programu by mu było łatwiej stworzyć zwarty kod.
Podobnie będzie jak zmienimy warunek na przeciwny:

Code: Select all

if(!GPIO_ReadInputPin(GPIOC, GPIO_PIN_7))


Code: Select all

tnz a ;test negative or zero
jrne 00102$ ;tym razem przeskocz jeśli nie zero


Cuda się zaczną dziać jeśli jawnie będziemy porównywać wynik funkcji ReadInputPin do wartości zgodnych z typem jakie ta powinna zwracać. W przypadku porównania z wartością RESET nic złego się nie stanie. Kod będzie identyczny i wynik kompilacji też jak wyżej. Kod jednak przestanie działać prawidłowo jak będziemy porównywać z wartością SET.

Code: Select all

if(GPIO_ReadInputPin(GPIOC, GPIO_PIN_7) == SET)


Teoretycznie powinno zadziałać umieszczone tam rzutowanie. Ale czy na pewno? I okazuje się, że nie. W głównym kodzie otrzymamy taki wynik:

Code: Select all

call _GPIO_ReadInputPin
addw sp, #3
dec a ;zmniejsz o jeden
jrne 00102$ ;i skocz jeśli nie zero
; main.c: 12: GPIO_WriteHigh(GPIOE, GPIO_PIN_5);
push #0x20 ;inaczej przygotuj WriteHigh
push #0x14
push #0x50
call _GPIO_WriteHigh
addw sp, #3
jra 00105$ ;i pętla się zamyka
00102$:
; main.c: 14: GPIO_WriteLow (GPIOE, GPIO_PIN_5);
push #0x20
push #0x14
push #0x50
call _GPIO_WriteLow
addw sp, #3
jra 00105$


Który wydaje się być poprawny bo przecież funkcja ReadInputPin w akumulatorze powinna zwrócić albo 0 albo 1, więc operacja "dec a" powinna nam z powodzeniem zastąpić wcześniej używaną "tnz a". Obie odpowiednio poustawiają flagę Z układu ALU, którą posiłkować się będą skoki warunkowe. Taki kod jednak prawie zawsze wskakuje do WriteLow, jedynie przy testowaniu GPIO_PIN_0 zachowywać się będzie normalnie.

By to rozwikłać, wypada przyjrzeć się rozwinięciu funkcji z biblioteki. Wygląda to tak:

Code: Select all

; function GPIO_ReadInputPin
; -----------------------------------------
_GPIO_ReadInputPin:
; stm8s_gpio.c: 215: return ((BitStatus)(GPIOx->IDR & (uint8_t)GPIO_Pin));
ldw x, (0x03, sp) ;rejestr wskaźnikowy X będzie zawierał adres GPIOx
ld a, (0x1, x) ;do akumulatora ładujemy stan IDR, który jest pod adresem GPIOx+1
and a, (0x05, sp) ;i wykoanamy operację AND z wartością maski GPIO_Pin
ret


Jak widać po wykonaniu tej funkcji w przypadku gdy pin GPIO_PIN_7 będzie w stanie wysokim to w akumulatorze pojawi się nie 0x01 - co odpowiadałoby SET którą tu oczekujemy - a 0x80. Czyli żadnego rzutowania wartości na BitStatus tu nie ma. Otrzymujemy jakby wartość RAW.
Nie jest to wynik tego, że kompilujemy z opcją optymalizacji do najmniejszego rozmiaru kodu lub szybkości jego wykonania. Zawsze postać tej funkcji bibliotecznej była taka sama.

I teraz zachodzi pytanie, czy traktować to jako błąd kompilatora czy błąd programistów bibliotek, że akurat tak tą funkcję napisali? Może uznali, że nikt nie będzie się bawił w porównania tylko zrobi to tak jak pokazałem na początku.
Ostatnio zmieniony poniedziałek 11 sty 2021, 16:32 przez ZbeeGin, łącznie zmieniany 1 raz.

Awatar użytkownika
xor
User
User
Posty: 169
Rejestracja: poniedziałek 05 wrz 2016, 21:44

Re: STM8+SDCC - Dlaczego właśnie tak?

Postautor: xor » poniedziałek 11 sty 2021, 16:04

Enum w C to int więc rzutowanie też zwróci int a nie bool. Jak dla mnie ewidentnie błąd programisty.
Powinni byli nasmarować

Kod: Zaznacz cały

return (!!(GPIOx->IDR & (uint8_t)GPIO_Pin));

albo/i dać sobie spokój z nazwanymi statusami
Ostatnio zmieniony wtorek 12 sty 2021, 11:54 przez xor, łącznie zmieniany 2 razy.

Awatar użytkownika
Antystatyczny
Geek
Geek
Posty: 1168
Rejestracja: czwartek 03 wrz 2015, 22:02

Re: STM8+SDCC - Dlaczego właśnie tak? Studium pewnego przypadku

Postautor: Antystatyczny » poniedziałek 11 sty 2021, 18:17

ZbeeGin pisze:Cuda się zaczną dziać jeśli jawnie będziemy porównywać wynik funkcji ReadInputPin do wartości zgodnych z typem jakie ta powinna zwracać. W przypadku porównania z wartością RESET nic złego się nie stanie. Kod będzie identyczny i wynik kompilacji też jak wyżej. Kod jednak przestanie działać prawidłowo jak będziemy porównywać z wartością SET.



Kod: Zaznacz cały

 if(GPIO_ReadInputPin(GPIOC, GPIO_PIN_7) == SET)




Kod będzie działać wyłącznie wtedy, gdy będziesz testował GPIO_PIN_0, bo wartość tego bitu w rejestrze IDR (o ile jest ustawiony) wynosi 1, czyli dokładnie tyle, ile wynosi wartość SET
"The true sign of intelligence is not knowledge but imagination" Albert Einstein.

Awatar użytkownika
ZbeeGin
User
User
Posty: 492
Rejestracja: sobota 08 lip 2017, 17:16
Lokalizacja: Śląsko-Zagłębiowska Metropolia
Kontaktowanie:

Re: STM8+SDCC - Dlaczego właśnie tak?

Postautor: ZbeeGin » wtorek 12 sty 2021, 10:49

xor pisze:Powinni byli nasmarować

Kod: Zaznacz cały

return (!!(GPIOx->IDR & (uint8_t)GPIO_Pin));

Podwójne zaprzeczenie?

Antystatyczny pisze:Kod będzie działać wyłącznie wtedy, gdy będziesz testował GPIO_PIN_0, bo wartość tego bitu w rejestrze IDR (o ile jest ustawiony) wynosi 1, czyli dokładnie tyle, ile wynosi wartość SET

Tak. Wiem, bo nawet to sprawdzałem "sprzętowo".

Jeśli bym chciał się upierać przy stosowaniu jednak jawnego równania z jakąś wartością to wychodzi na to, że najlepiej jest stosować:

Code: Select all

if(GPIO_ReadInputPin(GPIOC, GPIO_PIN_7) != RESET)

if(GPIO_ReadInputPin(GPIOC, GPIO_PIN_7) == RESET)

Awatar użytkownika
xor
User
User
Posty: 169
Rejestracja: poniedziałek 05 wrz 2016, 21:44

Re: STM8+SDCC - Dlaczego właśnie tak?

Postautor: xor » wtorek 12 sty 2021, 11:46

ZbeeGin pisze:
xor pisze:Powinni byli nasmarować

Kod: Zaznacz cały

return (!!(GPIOx->IDR & (uint8_t)GPIO_Pin));

Podwójne zaprzeczenie?

Yep! Żeby otrzymać niezanegowanego boola.

ZbeeGin pisze:Jeśli bym chciał się upierać przy stosowaniu jednak jawnego równania z jakąś wartością to wychodzi na to, że najlepiej jest stosować:

Code: Select all

if(GPIO_ReadInputPin(GPIOC, GPIO_PIN_7) != RESET)

if(GPIO_ReadInputPin(GPIOC, GPIO_PIN_7) == RESET)

OK, ale istnienie opcji SET jest nadal totalnie mylące. Nie powinno tak być. Jeśli już tak pisać to zdecydowanie wywalić opcję SET z enuma. Wtedy gdy, w chwili zapomnienia, napiszesz == SET wywali błąd.


Wróć do „Inne mikroklocki, również peryferyjne”

Kto jest online

Użytkownicy przeglądający to forum: Obecnie na forum nie ma żadnego zarejestrowanego użytkownika i 3 gości