Chyba kolejny mały kwiatek.

int GetEncoder(void) {
cli();
int e = *pEncoderValue;
sei();
return e;
}

zostało przetłumaczone na:

int GetEncoder(void) {
cli();
1476: f8 94 cli
int e = *pEncoderValue;
sei();
1478: 78 94 sei
147a: e0 91 3a 01 lds r30, 0x013A
147e: f0 91 3b 01 lds r31, 0x013B
return e;
}
1482: 80 81 ld r24, Z
1484: 91 81 ldd r25, Z+1 ; 0x01
1486: 08 95 ret

Nie wiem czemu kompilator wywalił pobieranie zmiennej poza sekcję
cli/sei ? To samo jest w najnowszej wersji.

Moim skromnym zdaniem powinno być tak:

int GetEncoder(void) {
f8 94 cli
e0 91 3a 01 lds r30, 0x013A
f0 91 3b 01 lds r31, 0x013B
80 81 ld r24, Z
91 81 ldd r25, Z+1 ; 0x01
78 94 sei
08 95 ret

lub tak:

int GetEncoder(void) {
e0 91 3a 01 lds r30, 0x013A
f0 91 3b 01 lds r31, 0x013B
f8 94 cli
80 81 ld r24, Z
91 81 ldd r25, Z+1 ; 0x01
78 94 sei
08 95 ret


Pozdrawiam
Grzegorz

do góry

On Fri, 12 Jun 2009 22:15:03 +0200, Grzegorz Kurczyk wrote:
>Chyba kolejny mały kwiatek.
>int GetEncoder(void) {
> cli();
> int e = *pEncoderValue;
> sei();
> return e;
>}

A cos tu jest volatile ?
Bo moze mial prawo, jesli uwaza ze tak mu wygodniej ..


>
>zostało przetłumaczone na:
>
>int GetEncoder(void) {
> cli();
> 1476: f8 94 cli
> int e = *pEncoderValue;
> sei();
> 1478: 78 94 sei
> 147a: e0 91 3a 01 lds r30, 0x013A
> 147e: f0 91 3b 01 lds r31, 0x013B
> return e;
>}
> 1482: 80 81 ld r24, Z
> 1484: 91 81 ldd r25, Z+1 ; 0x01
> 1486: 08 95 ret
>
>Nie wiem czemu kompilator wywalił pobieranie zmiennej poza sekcję
>cli/sei ? To samo jest w najnowszej wersji.
>
>Moim skromnym zdaniem powinno być tak:
>
>int GetEncoder(void) {
>f8 94 cli
>e0 91 3a 01 lds r30, 0x013A
>f0 91 3b 01 lds r31, 0x013B
>80 81 ld r24, Z
>91 81 ldd r25, Z+1 ; 0x01
>78 94 sei
>08 95 ret
>
>lub tak:
>
>int GetEncoder(void) {
>e0 91 3a 01 lds r30, 0x013A
>f0 91 3b 01 lds r31, 0x013B
>f8 94 cli
>80 81 ld r24, Z
>91 81 ldd r25, Z+1 ; 0x01
>78 94 sei
>08 95 ret
>
>
>Pozdrawiam
>Grzegorz

do góry

Grzegorz Kurczyk pisze:

> Nie wiem czemu kompilator wywalił pobieranie zmiennej poza sekcję
> cli/sei ? To samo jest w najnowszej wersji.

Tutaj akurat kompilator ma rację - może przestawiać instrukcje do woli
póki nie wpływa to na wynik obliczeń (ani cli, ani sei nie wpływa na
obliczenia, atomowość nie jest brana pod uwagę). Żeby kompilator nie
wywlekał obliczeń poza sekcję krytyczną trzeba zrobić barierę na
pamięci. Twój program powinien wyglądać tak:

int GetEncoder(void) {
asm volatile ("cli":::"memory");
int e = *pEncoderValue;
asm volatile ("sei":::"memory");
return e;
}

Lepiej będzie jednak jak użyjesz makr zdefiniowanych w pliku atomic.h

--
przeciez moje rozumowanie bylo bez skazy,
no sam bym wskoczyl do tego wulkanu,
ale kto by tak pieknie gwizdal...

do góry

Użytkownik Zbych napisał:
> Tutaj akurat kompilator ma rację - może przestawiać instrukcje do woli
> póki nie wpływa to na wynik obliczeń (ani cli, ani sei nie wpływa na
> obliczenia, atomowość nie jest brana pod uwagę). Żeby kompilator nie
> wywlekał obliczeń poza sekcję krytyczną trzeba zrobić barierę na
> pamięci. Twój program powinien wyglądać tak:
>
> int GetEncoder(void) {
> asm volatile ("cli":::"memory");
> int e = *pEncoderValue;
> asm volatile ("sei":::"memory");
> return e;
> }
>
> Lepiej będzie jednak jak użyjesz makr zdefiniowanych w pliku atomic.h
>

Pamiętam, że jakaś starsza wersja (chyba z 2006) kompilowała to bez
przestawiania. Funkcja jest z biblioteki, którą napisałem dawno temu i
sprawdzałem kod wynikowy. W wersji WinAVR20090313 faktycznie pomogło
volatile przy definicji wskaźnika int *pEncoderValue. Zaskoczyło mnie,
że volatile może być tu przydatne, bo jednak bardziej służy ono do
lokalnego wyłączenia optymalizacji dotyczącej danej zmiennej, a nie do
zmiany kolejności operacji nie mających nic wspólnego z tą zmienną.
Z punktu widzenia zmiennej *pEncoderValue kod jest tak samo optymalny, a
niemalże identyczny:

int *pEncoderValue;
daje w wyniku:
int GetEncoder(void) {
151c: f8 94 cli
151e: 78 94 sei
1520: e0 91 3a 01 lds r30, 0x013A
1524: f0 91 3b 01 lds r31, 0x013B
1528: 80 81 ld r24, Z
152a: 91 81 ldd r25, Z+1 ; 0x01
152c: 08 95 ret
}

volatile int *pEncoderValue;
daje
int GetEncoder(void) {
1520: f8 94 cli
1522: e0 91 3a 01 lds r30, 0x013A
1526: f0 91 3b 01 lds r31, 0x013B
152a: 20 81 ld r18, Z
152c: 31 81 ldd r19, Z+1 ; 0x01
152e: 78 94 sei
1530: c9 01 movw r24, r18
1532: 08 95 ret

Z drugiej strony taka zmiana sekwencji rozkazów sterujących przez
kompilator wydaje mi się trochę dziwna, bo w pewnych sytuacjach ma ona
wpływ na wartość obliczeń (właśnie aby tego uniknąć blokowałem
przerwania). Idąc tym tropem kompilator mógłby "dojść do wniosku", że
sekwencję:
sbi(PORTB, 1);
sbi(PORTB, 2);
sbi(PORTB, 3);
można zamienić na:
sbi(PORTB, 3);
sbi(PORTB, 2);
sbi(PORTB, 1);
lub jeszcze optymalniej (3 takty zegara zamiast 6) na:
in r24, PORTB
ori r24, 0x0E
out PORTB, r24
bo w sumie efekt końcowy (całkowity wynik operacji) jest ten sam:
wyjścia 1,2 i 3 portu B zostały ustawione w stan wysoki. Tyle, że jeśli
te linie są sygnałami CS/CLK/DATA dla jakiegoś rejestru szeregowego, to
już reszta układu tak samo działać nie będzie. Na szczęście nigdy nie
zauważyłem aby robił takie podmianki :-)


Dzięki serdeczne za podpowiedź.
Pozdrawiam
Grzegorz

do góry

A teraz coś czego już zupełnie nie rozumiem:

int *pEncoderValue; // celowo bez volatile

int GetEncoder(void) {
cli();
int e = *pEncoderValue;
sei();
return e;
}

void SetEncoderValue(int Value) {
cli();
*pEncoderValue = Value;
sei();
}

kompiluje się na:

int GetEncoder(void) {
151c: f8 94 cli
151e: 78 94 sei
1520: e0 91 3a 01 lds r30, 0x013A
1524: f0 91 3b 01 lds r31, 0x013B
1528: 80 81 ld r24, Z
152a: 91 81 ldd r25, Z+1 ; 0x01
152c: 08 95 ret
}
void SetEncoderValue(int Value) {
152e: f8 94 cli
1530: e0 91 3a 01 lds r30, 0x013A
1534: f0 91 3b 01 lds r31, 0x013B
1538: 91 83 std Z+1, r25 ; 0x01
153a: 80 83 st Z, r24
153c: 78 94 sei
153e: 08 95 ret
}

Na jakiej podstawie kompilator (najnowszy WinAVR_20090313) stwierdził,
że w pierwszej funkcji kolejność operacji nie ma znaczenia, a w drugiej
już ma ? Bug czy tzw. "feature" ;-)

Jednak trzeba do tego pliku .lss regularnie zaglądać, bo można się mocno
zdziwić przy uruchamianiu maszynki... ;-) Takie przestawienie rozkazu
sei w funkcji może być wyjątkowo upierdliwe, bo niby wszystko jest ok i
program może całymi dniami działać dobrze i nagle funkcja jednorazowo
oddaje nieprawidłową wartość, aby potem znowu długo wszystko było ok.

Pozdrawiam
Grzegorz

do góry

Grzegorz Kurczyk pisze:

> Jednak trzeba do tego pliku .lss regularnie zaglądać, bo można się mocno
> zdziwić przy uruchamianiu maszynki... ;-) Takie przestawienie rozkazu
> sei w funkcji może być wyjątkowo upierdliwe, bo niby wszystko jest ok i
> program może całymi dniami działać dobrze i nagle funkcja jednorazowo
> oddaje nieprawidłową wartość, aby potem znowu długo wszystko było ok.

Dlatego napisałem, że do takich rzeczy jak sekcje atomowe są makra
zdefiniowane w pliku atomic.h, a ty uparcie chcesz rzeźbić ręcznie
(pomijając przy tym barierę).

Zrób to tak:

#include <util/atomic.h>

ATOMIC_BLOCK(ATOMIC_FORCEON)
{
int e = *pEncoderValue;
}

Albo bezpieczniej:

ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
{
int e = *pEncoderValue;
}

--
przeciez moje rozumowanie bylo bez skazy,
no sam bym wskoczyl do tego wulkanu,
ale kto by tak pieknie gwizdal...

do góry

On Sat, 13 Jun 2009 03:35:54 +0200, Grzegorz Kurczyk wrote:
>Pamiętam, że jakaś starsza wersja (chyba z 2006) kompilowała to bez
>przestawiania. Funkcja jest z biblioteki, którą napisałem dawno temu i
>sprawdzałem kod wynikowy. W wersji WinAVR20090313 faktycznie pomogło
>volatile przy definicji wskaźnika int *pEncoderValue. Zaskoczyło mnie,
>że volatile może być tu przydatne, bo jednak bardziej służy ono do
>lokalnego wyłączenia optymalizacji dotyczącej danej zmiennej, a nie do
>zmiany kolejności operacji nie mających nic wspólnego z tą zmienną.

Volatile nie sluzy do optymalizowania, tylko mowi kompilatorowi ze
zmienna moze sie zmienic w dowolnym momencie. W zwiazku z czym nie
wolno mu optymalizowac kodu zakladajac ze sie nie zmienia.

Bo jak zalozyc ze nie zmienia sie w przerwaniach/samoczynnie, to
kompilator moze przeprowadzic bardzo gleboka optymalizacje - np
zalozyc ze zmienna jest zawsze 0 i zlikwidowac caly program.

>Z drugiej strony taka zmiana sekwencji rozkazów sterujących przez
>kompilator wydaje mi się trochę dziwna, bo w pewnych sytuacjach ma ona
>wpływ na wartość obliczeń

Programisci sa przed tym wyraznie ostrzegani, ze program moze to
zrobic, i maja stosownie pisac program.
Albo rybki albo akwarium - albo efektywny program, albo robiacy
dokladnie to co programista zapisal :-)

>(właśnie aby tego uniknąć blokowałem
>przerwania). Idąc tym tropem kompilator mógłby "dojść do wniosku", że
>sekwencję:
>sbi(PORTB, 1);
>sbi(PORTB, 2);
>sbi(PORTB, 3);
>można zamienić na:
>sbi(PORTB, 3);
>sbi(PORTB, 2);
>sbi(PORTB, 1);

Tak to nie, bo to jednak i operacja specjalna i taka "wyjsciowa",
ale juz kolejnosc odczytow z portow moglby przestawic.

J.

do góry

T.M.F. pisze:

> Skad masz makra sei() i cli()? To twoja wlasna definicja czy z
> biblioteki avr-glibc - pliku interrupts.h?
> Zakladam, ze twoja wlasna, stad problemy. Oryginalna prawidlowa
> definicja wyglada tak:
> #define sei() __asm__ __volatile__ ("sei" ::)
>
> Zwroc uwage na slowo volatile. Swoja droga przeczytaj jak volatile
> dziala to rozwiaze sie wiele twoich dziwnych klopotow.

Volatile w tym przypadku zabezpiecza tylko przed usunięciem i zamianą
miejscami instrukcji "cli" i "sei" a nie tego co jest pomiędzy nimi.


--
przeciez moje rozumowanie bylo bez skazy,
no sam bym wskoczyl do tego wulkanu,
ale kto by tak pieknie gwizdal...

do góry

Użytkownik Zbych napisał:
>
> Dlatego napisałem, że do takich rzeczy jak sekcje atomowe są makra
> zdefiniowane w pliku atomic.h, a ty uparcie chcesz rzeźbić ręcznie
> (pomijając przy tym barierę).

Nie, no nie chcę nic robić uparcie. Sugestie Kolegi bardzo mi pomogły,
za co serdecznie dziękuję. Akurat w moim przypadku wystarczyło volatile,
ale gdybym nie spojrzał do .lss to żyłbym w błogiej nieświadomości będąc
pewnym, że zablokowałem przerwania w krytycznej sekcji programu i
zastanawiając się dlaczego program kiksuje raz na ruski miesiąc. Tym
bardziej, że w poprzedniej wersji kompilatora kolejność działań była
taka jak w źródłówce. Z biblioteki atomic.h nie miałem jeszcze okazji
korzystać, ale widzę, że chyba najwyższy czas :-) Nie narzekam na
WinAVR, bo to niezły kompilator, ale główny problem w tym, że program,
który w danej wersji WinAVR bez problemu się kompilował i co
najważniejsze działał poprawnie, w nowszej wywala błędy kompilacji (co
nie jest problemem), ale co gorsza kompilacja przechodzi bezbłędnie,
tylko program chodzi nie do końca tak jak powinien. Z tego powodu z dużą
rezerwą podchodzę do nowych wersji kompilatora.
Problem w tym skąd mam wiedzieć (poza "brutalnym" zajrzeniem do pliku
.lss), w którym momencie muszę posiłkować się sztuczką typu ATOMIC_BLOCK
lub czymś podobnym, bo kompilator może wygenerować kod, nie do końca
zgodny z założeniami autora kodu źródłowego. W podanym wcześniej
przykładzie w jednej procedurze było źle, a w następnej już dobrze i
nijak nie mogę wydedukować z czego to wynika. W przypadku volatile
sytuacja jest jasna, ale z tym sei to kompilator zrobił mi psikusa, bo
jest to pierwszy taki przypadek. Przecież sekcji cli() {..} sei() używa
się dość często. W większych programach mam ich dużo i to jest jak na
razie jedyna funkcja, w której takie zjawisko mi wystąpiło (choć po tym
kwiatku nie jestem już tego taki pewien i dla pewności skorzystam z rady
Kolegi Zbycha i porobię klamerki ATOMIC_BLOK).


Pozdrawiam i dziękuję za udział w dyskusji.
Grzegorz

do góry

Grzegorz Kurczyk pisze:

> nie jest problemem), ale co gorsza kompilacja przechodzi bezbłędnie,
> tylko program chodzi nie do końca tak jak powinien.

Ja mam na to jeden sposób, porównuję pliki lss z dwóch kolejnych wersji
kompilatora. Jeśli zmian jest niewiele, to jest to szybsza metoda niż
rozpoczynanie od nowa testów programu.

> Problem w tym skąd mam wiedzieć (poza "brutalnym" zajrzeniem do pliku
> .lss), w którym momencie muszę posiłkować się sztuczką typu ATOMIC_BLOCK

Powinieneś użyć jej wszędzie tam gdzie do tej pory miałeś

cli();
[...]
sei();



--
przeciez moje rozumowanie bylo bez skazy,
no sam bym wskoczyl do tego wulkanu,
ale kto by tak pieknie gwizdal...

do góry