Hallo Leute,
ich bin fast verzweifelt. Der Code unten läuft korrekt unter Studio 6
mit avr-gcc 4.8 aber mit dem neuen avr-gcc ab (einschliesslich)
Optimierung -O2 wird der Test der Adresse einfach "wegoptimiert".
Liegt das jetzt an meinem Code oder ist das ein Compiler-Bug?
Danke für eure Hilfe.
Der Code ist aus meinem Projekt.
Es werden 2 Messages dekodiert. Dabei steht am Index 5 und 6 eine 16bit
ID.
Der Dekoder soll die Messages bearbeiten, wenn:
- diese ID gleich der globalen fkt_device_id ist,
- oder einer Broadcast Maske entspricht: die oberen 3 Byte 0xeee0 und im
niederwertigsten Nibble das bit1 (0x02) gesetzt ist
Bei -O0, -O1 ist noch alles ok. Bei -O2 wird aber der Block beginend
mit:
if(((addressee & FKT_DEVICE_ID_BROADCAST_bm) ==
komplett weggelassen.
Ich habe keien Ahnung, was der Compiler da sieht, warum er das weglassen
sollte. Die Message2 sollte bei diesem if() matchen.
Grüße, Adib.
--
Wenn die Optimierung das entfernt, kann es sein, dass der Compiler hier
glaubt, dass sich am Ergebnis nie was ändern wird. Du könntest das
versuchen:
volatile bool isforme = false;
Der Compiler wird so angewiesen, immer "isforme" zu beschreiben, da sich
diese Variable ändern kann.
Gut wäre es, zusätzlich dazu hier im Forum nach "volatile" zu suchen. Da
gibt es viele Themen, die das Verhalten und den Einfluss beleuchten. Ich
bin darin nicht sonderlich firm.
Felix A. schrieb:> Wenn die Optimierung das entfernt, kann es sein, dass der Compiler hier> glaubt, dass sich am Ergebnis nie was ändern wird. Du könntest das> versuchen:
er hat damit ja sogar recht. die Funktion kann nur 0 als Ausgang
liefern.
Und damit macht das return gar keine sinn.
Adib schrieb:> übrigens die Kommandozeile lautet:
wenn ich das richtig sehe, sind dort keine Warnungen eingeschaltet.
Setze mal noch den Parameter -Wall
Felix A. schrieb:> Wenn die Optimierung das entfernt, kann es sein, dass der Compiler hier> glaubt, dass sich am Ergebnis nie was ändern wird.
Der Compiler hat nichts zu glauben.
Er soll den Code entsprechend den festgelegten Sprachregeln von C in
Binärcode umsetzen. Wenn der Quellcode nicht diesen Regeln entspricht,
soll er eine Fehlermeldung von sich geben. Und wenn er - warum auch
immer - den Verdacht hat, dass da irgendetwas komisch ist und vielleicht
so nicht der Intention des Autors entspricht, soll er eine Warnung
ausspucken.
Wenn man mal ganz genau hinschaut, dann macht dieser Code außer in
"while(1)" Kreise zu drehen, exakt nichts. Es werden kompliziert nie
wieder benutzte Variablen verändert. Das erkennt der GCC in der 4.9
Version offenbar, denn mit 4.9 gab es enrme Verbesserungen beim
Optimizer, und macht was seine Freiheit ist: weg damit!
bei -O1 macht er foo++ und bei -O2 nicht.
Habs im Simulator mit einem Brechpunkt getestet.
Mit eingeschalteten Warnings -wall, -wpedantic kommen nur Warningen,
dass die Parameter payload und len nicht benutzt werden.
Adib.
--
Adib schrieb:> bei -O1 macht er foo++ und bei -O2 nicht.
Ist doch ok. Wenn der ganze Zirkus nur dazu dient, darüber zu
entscheiden, ob die nicht benutzte Variable foo++ geändert werden soll,
ist das sein gutes Recht.
Wolfgang schrieb:> Ist doch ok. Wenn der ganze Zirkus nur dazu dient, darüber zu> entscheiden, ob die nicht benutzte Variable foo++ geändert werden soll,> ist das sein gutes Recht.
nein ist es nicht, foo ist volatile ist darf nicht wegoptimiert
werden.
> bei -O1 macht er foo++ und bei -O2 nicht.
Er kann die fkt_decoder() auch einfach zur Compile-Zeit ausführen.
Payload gibt es vermutlich gar nicht, da die nie gelesen werden und foo,
weil volatile, existiert, wird am Anfang von main() inkrementiert.
Ich finde zwar Assembler schreiben für (fast immer) überflüssig, aber
Assembler lesen (können) macht bei solchen Fragen schlau. (.lss-File)
Der Unterschied zwischen -O1 und -O2 ist der Test auf die Maske.
Der Test auf die device_id wird immer korrekt ausgeführt.
Bei -O2 sieht der generierte Code so aus:
Hallo Leute,
der Fehler ist mir in einem realen Projekt aufgefallen, wo die Daten per
serielle Schnittstelle einkommen und dann geparsed werden.
Zumindest da KANN der Compiler einfach garnicht wissen, welche Message
geparsed wird.
Ich habe den Code soweit verkleinert, dass sich der Fehler mit -O1 und
-O2 nachstellen lässt.
Ich kann mir das einfach nicht erklären, was genau abgeht.
Gibt es irgendwelche Optionen, die den Compiler zu veranlassen,
Zwischenschritte auszugeben?
Danke, Adib.
--
Ich kann da nur noch raten. Es könnte neben "isforme" nochmal zusätzlich
volatile auf "addressee" angewendet werden, da diese Prüfung
wegoptimiert wird. Aber dann wüsste ich auch nicht mehr.
Adib schrieb:> Diese Code-Sequenz wird entfernt: if((((uint16_t)addressee &> (uint16_t)0xfff0) == (uint16_t)0xeee0) ) {> isforme = true;> }
schreibe mal davor:
addressee = 0xEEE2;
(und ändere mal addressee in Adresse ab)
Adib schrieb:> Bei den beiden Durchläufen steht in addressee wie erwartet 0x0506 und> 0xeee2 drin.
wenn es aber direkt davor steht, kann man das Problem etwas weiter
eingrenzen.
Peter,
wenn ich "addressee = 0xEEE2;" schreibe willst du den Code nicht sehen
;-)
er generiert die Funktion decode nicht mehr, sondern incrementiert 2x
die volatile Variable foo.
Es findet kein Test mehr auf "device_id" noch auf die Maske statt - gut
wegoptimiert halt.
Ich hatte die Testfälle so gestaltet, dass er testen muss und dann
wenigstens einmal isforme = true setzt.
Adib.
--
Adib schrieb:> wenn ich "addressee = 0xEEE2;" schreibe willst du den Code nicht sehen> ;-)
das ist doch schon mal gut, damit wissen wir das der vergleich richtig
ist. Er geht also davon aus das in addressee niemals dieser wert stehen
kann.
Hallo Peter,
das glaube ich jetzt auch langsam.
Wie bildet man aus einem Byte-Stream korrekt einen 16bit unsignet Wert?
Dieser Code wird komplett wegoptimiert:
Adib schrieb:> Wie bildet man aus einem Byte-Stream korrekt einen 16bit unsignet Wert?
dein Schreibweise ist zwar etwas ungewöhnlich, aber so sollt es richtig
sein.
Üblicher ist
Hallo Leute,
ich glaube Peter hat mich auf die richtige Fährte geführt. Es war das
erzeugen des uint16_t Wertes aus dem uint8_t Byte strom:
so wird der Test auf die Maske nicht wegoptimiert:
Peter II schrieb:> nein ist es nicht, foo ist volatile ist darf nicht wegoptimiert> werden.
Auch volatiles durfen wegoptimiert werden, z.B. wenn es sich um toten
Code handelt wie in
Adib schrieb:> Gibt es noch eine effizientere Variante der Umwandlung.
nicht wirklich, man könnte ein memcpy schrieben, das ist dann aber nicht
mehr portabel.
Schade das sich bis jetzt keine der wirklich C Profis gemeldet hat,
scheint wirklich eine Bug zu sein. Wüsste man warum die Multiplikation
nicht gehen soll.
Zusammenfassung:
Adib schrieb:> Gibt es noch eine effizientere Variante der Umwandlung.
Ja, indem avr-gcc eine effizientere Sequenz dafür ausgibt. Mit
Rumgefrickel per Union bekommst du den Code vielleicht ein paar Bytes
kürzer, die korrekte Herangehensweise ist jedoch, dieses
Effizienzproblem im Compiler selbst zu beheben — was nicht ganz einfach
ist (wenn's einfach wäre wär's schon längst behoben).
Das ist Code der also nur unter little Endian geht. Man sollte das evtl
auf einem Cortex gegenchecken.
Ich denke, das ursprüngliche Problem mit:
1
uint16_taddressee1=(data[5]*0x100)|data[6];
ist, dass der Compiler bei dem literal 0x100 den Typ des
Multiplikationsergebnisses nicht auf int setzt, sondern bei dem Typ der
Variable bleibt.
Komischerweise setzt er das für seine Optimierung so. Der erzeugte Code
zeigt ja, dass ein 16bit Ergebnis generiert wird.
Normalerweise sollten ja alle literale als Int behandelt werden :-?
Was mache ich denn da wenn ich eine int8_t Variable mit 0x100
multiplizieren muss ... ?
Bug or feature?
Also ich tendiere eher zu Bug als zu Feature.
Adib.
--
Adib schrieb:> uint16_t addressee1 = (data[5] * 0x100) | data[6]; // Fehler
er geht scheinbar davon aus das
1
data[5]*0x100
unsigned berechnet wird, damit niemals größer als 32768 werden kann,
damit kann das höchste bit nicht gesetzte sein, damit ist dann der
vergleich niemals war.
Korrektur:
Adib schrieb:
> uint16_t addressee1 = (data[5] * 0x100) | data[6]; // Fehler
er geht scheinbar davon aus das
data[5] * 0x100
signed berechnet wird, damit niemals größer als 32768 werden kann,
damit kann das höchste bit nicht gesetzte sein, damit ist dann der
vergleich niemals war.
Peter II schrieb:> signed berechnet wird, damit niemals größer als 32768 werden kann,
ok, geh' ich mit. Ist also schlechter Source Code.
Also wenn man:
- unsigned Variablen mit Literalen Multipliziert,
dann
- das Literal auch als Unsigned markieren.
Danke an Alle.
Gute Nacht, Adib.
--
Adib schrieb:>> signed berechnet wird, damit niemals größer als 32768 werden kann,>> ok, geh' ich mit. Ist also schlechter Source Code.> Also wenn man:> - unsigned Variablen mit Literalen Multipliziert,> dann> - das Literal auch als Unsigned markieren.
eher so:
0x100 -> wird int
uint8_t * int -> wird zu int * int
damit ist das Ergebnis signed.
Peter II schrieb:> Warum nimmt du eigentlich -O2 und nicht -Os?>> Bei -Os würde ich schon erwarten, das er die Umwandlung besser> optimiert.
Mich interessiert eigentlich nur die Ausführungsgeschwindigkeit. Flash
habe ich genug.
Die Konvertierung ist wie bei den anderen -O Varianten. Das erkennt der
Compiler noch nicht, was ich will.
Einzig das Cast generiert den "optimalen" Code. Ist aber leider nicht
portabel ....
Adib.
--
Aus dem C99 Standard:
If the type of the operand with signed integer type can represent all of
the values of the type of the operand with unsigned integer type, the
operand with unsigned integer type is converted to the type of the
operand with signed integer type.
Peter II schrieb:> er geht scheinbar davon aus das> data[5] * 0x100>> unsigned berechnet wird, damit niemals größer als 32768 werden kann,> damit kann das höchste bit nicht gesetzte sein, damit ist dann der> vergleich niemals war.
Das entscheidende Stichwort ist das "undefined Behavior":
Ist data[5] <= 127, kann das Produkt maximal 32512 = 0x7f00 werden, das
höchste Bit ist also nicht gesetzt.
Ist hingegen data[5] >= 128, entsteht ein Signed-Integer-Overflow.
Dieser führt zu einem undefined Behavior, bei dem der Compiler von
beliebigen Annahmen ausgehen darf. Schlauerweise nimmt er deswegen an,
dass das höchste Bit auch in diesem Fall nicht gesetzt ist, weil der
damit ein ganzes Stück Code wegoptimieren kann.
Oder er nimmt an, dass der Programmierer den Code so geschrieben hat,
dass es gar nicht erst zum Overflow kommt, was diese Optimierung
ebenfalls rechtfertigt.
Neuere Versionen des GCC nutzen an mehreren Stellen die Regeln für
undefined Behavior und strict Aliasing für aggressive Optimierungen.
Unsauber programmierter Code, der bei früheren GCCs noch funktionierte,
führt jetzt mitunter zu seltsamen Fehlern, bei denen fälschlicherweise
der Verdacht erst einmal auf den Compiler fällt.
Hier ist die Problematik noch einmal auf das Wesentliche reduziert und
für PC-Plattformen dargestellt:
main.c:
1
#include<stdio.h>
2
#include"externfunc.h"
3
4
intinternfunc(unsignedcharuc){
5
unsignedintui=uc*100000000;
6
returnui==3000000000;
7
}
8
9
intmain(void){
10
printf("%d\n",internfunc(30));
11
printf("%d\n",externfunc(30));
12
return0;
13
}
externfunc.h:
1
#ifndef EXTERNFUNC_H
2
#define EXTERNFUNC_H
3
4
intexternfunc(unsignedcharuc);
5
6
#endif
externfunc.c:
1
#include"externfunc.h"
2
3
intexternfunc(unsignedcharuc){
4
unsignedintui=uc*100000000;
5
returnui==3000000000;
6
}
Die beiden Funktionen internfunc und externfunc sind bis auf den
Funktionsnamen exakt identisch und werden in main mit dem gleichen
Argument aufgerufen. Trotzdem liefern sie bei Optimierungsstufe -O2, -O3
und -Os unterschiedliche Ergebnisse:
1
$ gcc -O2 -o main main.c externfunc.c
2
$ main
3
1
4
0
Ist das in Bug im Compiler? Nein, es ist undefined Behavior.
Der Grund für das unterschiedliche Verhalten:
Der Aufruf von internfunc wird geinlinet und das Ergebnis bereits zur
Compilezeit berechnet. Bei dem Produkt entsteht zwar ein Überlauf
(30*100000000=3000000000 ist nicht als signed int darstellbar), der
Compiler behandelt diesen Überlauf aber nicht explizit, sondern
interpretiert das Ergebnis einfach als signed int, d.h. als
30*100000000-2**32=-1294967296. Diese Zahl wird bei der Zuweisung an ui
implizit nach unsigned int gecastet, was wieder 3000000000 ergibt. Somit
ist der anschließende Vergleich wahr und der Rückgabewert 1.
Bei der Übersetzung externfunci.c weiß der Compiler nicht, mit welchem
Argument die Funktion aufgerufen werden wird, er kann also keine
Berechnung zur Compilezeit durchführen. Er geht aber in den höheren
Optimierungsstufen davon aus, das kein undefined Behavior stattfinden
wird, also kann das Produkt selbst bei Berücksichtigung des impliziten
Casts nach unsigned int niemals 3000000000 werden. Also ist der
Rückgabewert immer 0, und die Multiplikation und der Vergleich können
wegoptimiert werden.
In beiden Fällen optimiert der Compiler die Multiplikation und den
Vergleich weg. Da er dies aber (legalerweise) auf unterschiedliche Weise
tut, sind die Funktionsergebnisse ebenfalls unterschiedlich.
Und wenn jetzt einer meint, dass der gcc vor solchen Dingen per
"-Wunreachable-code" warnt, dann irrt er:
http://gcc.gnu.org/ml/gcc-help/2011-05/msg00360.html
Leider nicht sehr schlau...
Empfehlenswert ist die Nutzung eines statischen Source Code Analyzers.
Gastino G. schrieb:> Und wenn jetzt einer meint, dass der gcc vor solchen Dingen per> "-Wunreachable-code" warnt, dann irrt er:
Du kannst Warnungen aktivieren wie -Wconversion:
1
warning: conversion to 'unsigned int' from 'int' may change the sign of the result [-Wsign-conversion]
Weiters gibt es -fsanitize= :
1
-fsanitize=undefined
2
Enable UndefinedBehaviorSanitizer, a fast undefined behavior
3
detector. Various computations are instrumented to detect
4
undefined behavior at runtime. Current suboptions are:
Wer Signed Overflow wie in Yalus Beispiel verwenden will, setzt -fwrapv
so dass Signed Overflow sich analog verhält wie Unsigned Overflow
(Wrapping, d.h. modulo 2^bits).
Yalu X. schrieb:> Trotzdem liefern sie bei Optimierungsstufe -O2, -O3> und -Os unterschiedliche Ergebnisse:> 1> 0
Die Ergebnisse sind gleich, nämlich beide male Undefined Behavior :-)
Mit gcc 4.7 erhalte ich für internfunc etwa 0 als Ergebnis.
> [...] also kann das Produkt selbst bei Berücksichtigung des> impliziten Casts nach unsigned int niemals 3000000000 werden.
Wie gesagt, wer Signed Overflow mag setzt -fwrapv.
Thread beobachten
Seitenaufteilung abschalten