avr-gcc-list
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [avr-gcc-list] Avr-gcc Produces Incorrect Code with -Os


From: Andy H
Subject: Re: [avr-gcc-list] Avr-gcc Produces Incorrect Code with -Os
Date: Thu, 22 May 2008 22:15:41 -0400
User-agent: Thunderbird 2.0.0.14 (Windows/20080421)

Don't worry about being shot down. Nobody hear can shoot straight, unless it's their foot.
(myself included)

This is very similar to what we do. It can also be targeted to specific interrupts (or tasks).
We have to do it like this - or latency will loose us data.
Like your it CAPITALISED with red flashing lights for review!

Andy


Scott and Roxanne Munns wrote:
Hi everyone,

A lurker pops his head out of his hole for a comment - please don't shoot
me!  :)

Normally, critical sections are supposed to be used to protect *data*, not
*code*.  The atomic operations listed in the URL are doing very specific
atomic operations on data.  In other words, they also follow the theme of
protecting data, not code.  It seems to me that applying the "critical"
attribute to generic functions can lead a programmer to start thinking in
terms of protecting code instead of data.  Eventually that can lead to a
disastrous result, especially if you ever work with multi-threaded code.

All of the examples I have seen in the thread so far were used for very
simple data-modifying operations that are suitable for inlining.  I think
that is acceptable use of "critical".  However, not everyone will follow
that principle - they may treat "critical" as a cure-all.

In the past, I have solved the critical section problem by using some
#defines.  Unfortunately, I had to use THAT_OTHER_COMPILER(tm).  I'd give
you the exact definitions, but I'm at home and don't have access to my work
code.

DECLARE_CRITICAL_SECTION - at the top of each function with a critical
section - creates a variable to store SREG
BEGIN_CRITICAL_SECTION - records the value of SREG and clears global
interrupt flag
END_CRITICAL_SECTION - restores SREG

Technically it's no better than doing the code inline, but it is more
self-documenting.

Hope this generates some more ideas/discussion,
Scott


-----Original Message-----
From: address@hidden
[mailto:address@hidden On Behalf Of
Andy H
Sent: Thursday, May 22, 2008 5:36 PM
To: David Brown
Cc: address@hidden
Subject: Re: [avr-gcc-list] Avr-gcc Produces Incorrect Code with -Os

http://gcc.gnu.org/onlinedocs/gcc-4.1.0/gcc/Atomic-Builtins.html



David Brown wrote:
address@hidden wrote:
Eric,

The problem with the function is typical housekeeping overhead gets wrapped up.

To take a silly example:

Say you want to send some message to ISR (via memory) using critical function

critical void function(int *buffer, int *cValue) { *cValue++ = *buffer++; }

Getting *buffer++ does not need ISR disabled. Perhaps neither does the cValue++;

It is far more important to be correct than to be fast - thus it is better to err on the side of disabling interrupts for too long. Of course, in time-critical code disabling the interrupts for too long could also make the code fail.

The point of the "critical" attribute is that it makes it very easy to write correct code. If you want code to be as fast as possible, your example should be written thus:

static inline critical void functionHelper(int v, int *cValue) {
   *cValue = v;
}

void function(int *buffer, int *cValue) {
   functionHelper(*buffer++, cValue++); }

With small static inline functions, you have exactly the granularity you want. And of course, you are still free to write out the cli()/sreg code if you find that makes sense for critical code.

There are also ways to get atomic access which don't block interrupts. (I expect you are already familiar with several methods, but this could be of interest to others on this list too.) For example:


#define volatileAccess(v) *((volatile typeof((v)) *) &(v)) #define nonblock_atomic_read(v) ({ \
   typeof(v) a, b; \
   a = volatileAccess(v); \
   b = volatileAccess(v); \
   while (a != b) { \
       a = b; \
       b = volatileAccess(v); \
   }; \
   b; \
   })

It's also possible to use a 1-byte flag for access control, since it can always be written and read atomically. But these sorts of solutions require more care and thought - a "critical" attribute is makes it easier for programmers to write working code.

So this method, although not wrong, is something I would normally reject for time critical systems, where latency is key. Inline coding has less overhead but even then its not perfect - though perhaps more palatable than assembler.

I do not know much about the gcc atomic operations. But it might be worth reviewing. There may be some hooks that make it work well with optimisations without adding a bunch of code hacks.
Also it makes code more portable (Atmel ARM or course!)

I believe the atomic operations are target-specific, and use target-specific assembly. Many larger processors have atomic operations of some sort - disabling interrupts won't work if you have an SMP system.

It would be *very* nice if gcc had a set of atomic access buildins that were consistent across a range of targets. It would also be *very* nice if function attributes were consistent across ports. It seems that every gcc port has its own function attribute for specifying interrupt functions, for example. Sometimes they have the same name, but different functions - the msp430 port has "interrupt" and "signal" with slightly different syntax from the avr port, and opposite effects. There are also attributes that only some ports have, that could be easily* and usefully provided in all ports. Examples are "critical", "naked", "interrupt", "near", "far", and "saveall".


* At least, it's easy to think out what the generated assembly would look like - I don't know if that makes it easy to implement in practice!


mvh.,

David



_______________________________________________
AVR-GCC-list mailing list
address@hidden
http://lists.nongnu.org/mailman/listinfo/avr-gcc-list


_______________________________________________
AVR-GCC-list mailing list
address@hidden
http://lists.nongnu.org/mailman/listinfo/avr-gcc-list

No virus found in this incoming message.
Checked by AVG. Version: 7.5.524 / Virus Database: 269.24.0/1460 - Release Date: 5/22/2008
7:06 AM
No virus found in this outgoing message.
Checked by AVG. Version: 7.5.524 / Virus Database: 269.24.0/1460 - Release Date: 5/22/2008
7:06 AM




reply via email to

[Prev in Thread] Current Thread [Next in Thread]