Codestück für Mega8 verkleinern (Flash reicht nicht).

[Sam .. (sam1994)]

Hi

Ich hab durch zufall herausgefunden, dass folgendes Codestück 3KB des 
Megas verbraucht, und darum reicht mir der Speicherplatz nicht.

switch(key)
        {
            case 2: 
                if(digit == 0)
                    digit = 8;
                while(!((uint8_t)(1<<--digit) & mask))
                    if(digit < 1)
                        digit = 8;
                _delay_ms(100);
                seconds10 = 0;
                break;
            case 4:
            case 6:
                pre = key - 5;
                switch(mode)
                {
                    case MODE_HOUR_MIN:
                         Add(&currentGame[digit / 4].time, valency[digit % 4] * pre, MAX_TIME);
                         break;
                    case MODE_SEC:
                         Add(&currentGame[digit / 4].time, valency[digit % 4] * pre, MAX_TIME);
                         break;
                    case MODE_GUILLOTINE1:
                    case MODE_GUILLOTINE2:
                    case MODE_GUILLOTINE3:
                    case MODE_GUILLOTINE4:
                    case MODE_GUILLOTINE5:
                    case MODE_GUILLOTINE6:
                    case MODE_GUILLOTINE7:
                        for(uint8_t i = 0; i < 2; i++)
                            Add(&currentGame[i].guillotine[mode - MODE_GUILLOTINE1], valency[digit % 4] * pre, MAX_GUILLOTINE);
                        break;
                    case MODE_BONUS:
                        for(uint8_t i = 0; i < 2; i++)
                            Add(&currentGame[i].bonus, valency[digit % 4] * pre, MAX_BONUS);
                    case MODE_GUILLOTINE_COUNT:
                        for(uint8_t i = 0; i < 2; i++)
                            Add(&currentGame[current].options, (1 << 5), 224);
                        break;
                    case MODE_MOVES_COUNT1:
                    case MODE_MOVES_COUNT2:
                    case MODE_MOVES_COUNT3:
                    case MODE_MOVES_COUNT4:
                    case MODE_MOVES_COUNT5:
                    case MODE_MOVES_COUNT6:
                    case MODE_MOVES_COUNT7:   
                        for(uint8_t i = 0; i < 2; i++)
                            Add(&currentGame[current].moves[mode - MODE_MOVES_COUNT1], valency10[digit % 4] * pre, 7);
                        break;
                    case MODE_MOVES:
                        Add(&moves, valency10[digit % 4] * pre, MAX_MOVES);
                        break;
                    _delay_ms(50);
                };
                break;
            case 5:
                WaitForKeyUp();
                return FINISHED;
                break;
            case 8:
            if(digit > 6)
                digit = 255;
                while(!((uint8_t)(1<<++digit) & mask))
                    if(digit > 6)
                        digit = 255;
                _delay_ms(100);
                seconds10 = 0;
                break;
            case 9:
                return ABORT;
                break;
        };

Weiß jemand wie ich das verkleinern kann. Compiler Optimiert schon auf 
Größe.
Das komische: Wenn ich die case 4 und 6 auseinandernehme und das ganze 
ohne die Addfunktion schreibe, komm ich auf ein viertel der Größe, 
allerdings steht dann alles doppelt da und ich würde es lieber mit so 
und damit übersichtlicher machen.

[Floh (Gast)]

Add funktion zeigen!
Ich hoffe du hast nirgends floats verwendet?

[Sam .. (sam1994)]

Nein hab ich nicht.

void Add(uint32_t* source, int32_t add, uint32_t max)
{
    int64_t tmp = (int64_t)*source + add;
    if(tmp > 0 && tmp <= max)
        *source = tmp;
}

Sry, hab ich vergessen.

[Peter (Gast)]

int64_t wird wohl das Problem sein, das ist auf einem 8bitter nicht mehr 
so sinnvoll.

[Sam .. (sam1994)]

Danke, Peter. Du hattest Recht das hat 2KB gekostet.

Kann man den Code vielleicht noch mehr optimieren, bzw. ist es so 
sinnvoll das mit switch zu machen, oder gibt es da bessere 
Möglichkeiten?

[Peter (Gast)]

Samuel K. schrieb:
> Kann man den Code vielleicht noch mehr optimieren, bzw. ist es so
> sinnvoll das mit switch zu machen, oder gibt es da bessere
> Möglichkeiten?
so schlimm ist der code nicht. Aber am meisten kann man sparen wenn man 
die genau funktion eines Programmes kennt. Leider kann man anhand der 
paar zeilen nicht wirklihc erkennen was das ding genau macht.

[Sam .. (sam1994)]

Die Funktion regelt die Eingabe einer Schachuhr. Die Funktion KeyRo 
bekommt da als Argumente ein Modus, der angibt was eingestellt wird, und 
das wird in der switch abgefragt. Die Maske bestimmt dabei nur welche 
Digits der 8stelligen 7Segment Anzeige benutzt werden.

GetKey gibt eine Taste entprellt zurück sobald sie gedrückt ist.

Hier ist nochmal der korrigierte Code, da ein paar kleine Fehler in 
KeyRo drin waren (ich konnte ihmnja nicht testen, da er zu groß war).

uint8_t KeyRo(uint8_t mask, uint8_t mode)
{
    uint8_t digit = 255;
    while(!((uint8_t)(1<<++digit) & mask))
        if(digit > 6)
            return ABORT;
    while(1)
    {
        int8_t pre = 5;
        uint8_t key = 255;
        while(key == 255)
            key = GetKey();
        switch(key)
        {
            case 2: 
                if(digit == 0)
                    digit = 8;
                while(!((uint8_t)(1<<--digit) & mask))
                    if(digit < 1)
                        digit = 8;
                _delay_ms(100);
                seconds10 = 0;
                break;
            case 4:
            case 6:
                pre -= key;
                switch(mode)
                {
                    case MODE_HOUR_MIN:
                         Add(&currentGame[digit / 4].time, (int32_t)valency[digit % 4] * pre, MAX_TIME);
                         break;
                    case MODE_SEC:
                         Add(&currentGame[digit / 4].time, valency[digit % 4] * pre, MAX_TIME);
                         break;
                    case MODE_GUILLOTINE1:
                    case MODE_GUILLOTINE2:
                    case MODE_GUILLOTINE3:
                    case MODE_GUILLOTINE4:
                    case MODE_GUILLOTINE5:
                    case MODE_GUILLOTINE6:
                    case MODE_GUILLOTINE7:
                        for(uint8_t i = 0; i < 2; i++)
                            Add(&currentGame[i].guillotine[mode - MODE_GUILLOTINE1], valency[digit % 4] * pre, MAX_GUILLOTINE);
                        break;
                    case MODE_BONUS:
                        for(uint8_t i = 0; i < 2; i++)
                            Add(&currentGame[i].bonus, valency[digit % 4] * pre, MAX_BONUS);
                    case MODE_GUILLOTINE_COUNT:
                        for(uint8_t i = 0; i < 2; i++)
                            Add(&currentGame[current].options, (1 << 5), 224);
                        break;
                    case MODE_MOVES_COUNT1:
                    case MODE_MOVES_COUNT2:
                    case MODE_MOVES_COUNT3:
                    case MODE_MOVES_COUNT4:
                    case MODE_MOVES_COUNT5:
                    case MODE_MOVES_COUNT6:
                    case MODE_MOVES_COUNT7:   
                        for(uint8_t i = 0; i < 2; i++)
                            Add(&currentGame[current].moves[mode - MODE_MOVES_COUNT1], valency10[digit % 4] * pre, 7);
                        break;
                    case MODE_MOVES:
                        Add(&moves, valency10[digit % 4] * pre, MAX_MOVES);
                        break;
                };
                _delay_ms(50);
                break;
            case 5:
                WaitForKeyUp();
                return FINISHED;
                break;
            case 8:
            if(digit > 6)
                digit = 255;
                while(!((uint8_t)(1<<++digit) & mask))
                    if(digit > 6)
                        digit = 255;
                _delay_ms(100);
                seconds10 = 0;
                break;
            case 9:
                return ABORT;
                break;
        };
        //Show on Display, but first set the time to show
        SetTime();
        //Now we can call Show to show the time
        Show(digit, mode);
    }

[Peter D. (peda)]

Switch ist schon recht effektiv.
Aber man kennt von Deinen Variablen nicht den Typ, daher kann man keine 
Optimierungen angeben.

Versuche, möglichst kleine Typen zu nehmen (uint8_t) und möglichst wenig 
globale Variablen.

Wenn Du Code postest, sollte er auch compilierbar sein, d.h. Prototypen 
für alle benutzten Variablen, Funktionen und Macrodefinitionen!
Im Assemblerlisting kann man nämlich am besten sehen, wo es klemmt.


Peter

[Sam .. (sam1994)]

Ok, hier ist der ganze Code zum kompilieren. Die Funktion oben ist die 
viertletzte.

PS: Wer zufällig einen Max7219 an einem Maga hängen hat, kann es sogar 
testen. Allerdings braucht man noch eine Matrixtastur.

[Sam .. (sam1994)]

In dem http://www.mikrocontroller.net/articles/AVR-GCC-Codeoptimierung 
steht folgendes:

>> Switch-Statements werden durch -mno-tablejump manchmal deutlich kürzer.

Was ist das?

[Frank M. (ukw) (Moderator)]

Peter Dannegger schrieb:
> Switch ist schon recht effektiv.

Bei Mehrfach-Cases (wie in diesem Fall) kann der Schuss aber auch nach 
hinten losgehen. Ich habe schon erlebt, dass hier eine if-else-if-Kette 
mit Or-Verknüpfung statt der Mehrfach-Cases wesentlich kleineren Code 
erzeugt. Vielleicht sollte Samuel den "Unter-Switch" für die Fälle 4 und 
6 mal als if-else-if-Kette umformulieren.

Gruß,

Frank

[Peter D. (peda)]

Samuel K. schrieb:
> Ok, hier ist der ganze Code zum kompilieren.


Paßt doch bequem rein:

AVR Memory Usage
----------------
Device: atmega8

Program:    6290 bytes (76.8% Full)
(.text + .data + .bootloader)

Data:        138 bytes (13.5% Full)
(.data + .bss + .noinit)

Hier mal ein paar Optimierungsschalter:

-fno-inline-small-functions
-fno-split-wide-types
-fno-move-loop-invariants
-Wl,--relax
--combine -fwhole-program

Allerdings solltest Du Dich mal um die Warnungen kümmern, z.B.:

CHESS.C:62: warning: excess elements in array initializer
CHESS.C:62: warning: (near initialization for 'Games')

Entweder Du läßt die [] leer oder Du schreibst die richtige Anzahl rein.


Peter

[Peter D. (peda)]

Frank M. schrieb:
> Ich habe schon erlebt, dass hier eine if-else-if-Kette
> mit Or-Verknüpfung statt der Mehrfach-Cases wesentlich kleineren Code
> erzeugt.

Ich habe immer nur das Gegenteil erlebt.
Beim Switch versucht er nämlich, ähnlichen Code von Cases zusammen zu 
fassen.
Man sieht eigentlich immer, daß er in andere Cases hineinspringt, obwohl 
das nicht so im Code steht.

Bei if/else läuft aber alles nacheinander ab und daher kann er nicht 
mehr ähnlichen Code mehrfach nutzen.


Peter

[Sam .. (sam1994)]

Danke für die Optimierungargumente. Kannst du mir vielleicht noch sagen 
wie ich die im MAkefile einbinde (hab noch nie ein Makefile selber 
geschrieben)?

[Sam .. (sam1994)]

OK ich hab das jetzt geschafft. Aber wenn ich die letzte Zeile als 
Compiler Flag einsetze kommt ein Linker Error, dass er meine 2 
Funktionen in main nicht findet.

[Andreas F. (aferber)]

Samuel K. schrieb:
> Aber wenn ich die letzte Zeile als
> Compiler Flag einsetze kommt ein Linker Error, dass er meine 2
> Funktionen in main nicht findet.

Diese Flags darfst du nur verwenden, wenn dein Projekt entweder nur aus 
einer einzigen Sourcedatei besteht, oder wenn alle Sourcen mit einem 
einzigen Compileraufruf in einem Rutsch kompiliert und gelinkt werden. 
Letzteres wird wohl in deinem Makefiles nicht gemacht (ist auch eher 
selten der Fall), deshalb kannst du die Flags so nicht verwenden.

Andreas

[Peter D. (peda)]

Samuel K. schrieb:
> Aber wenn ich die letzte Zeile als
> Compiler Flag einsetze kommt ein Linker Error

Dann laß sie weg, paßt trotzdem:

AVR Memory Usage
----------------
Device: atmega8

Program:    6698 bytes (81.8% Full)
(.text + .data + .bootloader)

Data:        138 bytes (13.5% Full)
(.data + .bss + .noinit)

Peter