Go Down

Topic: Replacement for AVR libc ATOMIC_BLOCK macros, now for DUE and other platforms. (Read 9 times) previous topic - next topic

pYro_65

[UPDATED V1.2.1b]

Hi all,

I have been working on a project that requires safety from interrupts so I did a bit of research, and found the ATOMIC_BLOCK macros.
They seem to be widely accepted as an easy way of controlling the global interrupt flag. After using it for a while I decided I'm not to keen on the syntax created from using the macros, my main reason is it looks like a function definition nested inside another function, which is not allowed in c++. Also, I have a need for some objects to be able to automatically control their atomic operations, I needed a more user friendly approach.

The idea I have come up with is quite simple. All you have to do is declare a variable of the class of blocking operation you want. The class has no callable methods. It works purely on side effects. The atomic operation lasts for the lifetime of the variable, therefore you can return from a function and still expect the global interrupt flag to be restored. This removes the need to use a temporary to store the value while the interrupt flag is restored before return.

Here is a comparison breakdown.

ATOMIC_BLOCK macros
Blocking type.

  • ATOMIC_BLOCK

  • NONATOMIC_BLOCK


Blocking mode.

  • ATOMIC_RESTORESTATE

  • NONATOMIC_RESTORESTATE

  • ATOMIC_FORCEON

  • NONATOMIC_FORCEOFF



New method. (V1.1)
Blocking type.

  • AtomicBlock

  • NonAtomicBlock


Blocking mode.

  • Atomic_RestoreState

  • Atomic_Force


Extensions for inline atomic blocks.

  • Protect function



V1.2 Now supports:

AVR 8-bit processors

  • Arduino 8-bit family ( Compiled / Tested )

  • Teensy 2.0 ( Not Compiled / Not Tested )

  • Teensy++ 2.0 ( Not Compiled / Not Tested )


ARM Cortex-M3

  • Arduino DUE ( Compiled / Not Tested )

  • Olimex STM32 / Leaflabs Maple. ( Compiled / Not Tested )


ARM Cortex-M4

  • Teensy 3.0 ( Compiled / Not Tested )



I have also extended the functionality with a safe mode ( v1.0) . ATOMIC_BLOCK( ATOMIC_RESTORESTATE ) simply writes an old copy of the status register into SREG. With the safe mode, only the global interrupt bit is touched. This can be used by adding a 'Safe' suffix to the blocking type, or passing  '_Safe' with the blocking mode.

There are 12 blocking variations, 8 of which are unique. A complete breakdown of the library is at the top of the file provided ( AtomicBlock.h ). Without safe mode on ( safe mode has no effect when using blocking mode 'Atomic_Force' ) the library will emulate ATOMIC_BLOCK functionality with no extra overhead.


Here is a short example of how to use the library with variable declarations.

Code: [Select]
#include "AtomicBlock.h"

void setup() { return; }

//New method
void loop()
{
 AtomicBlock< Atomic_RestoreState > a_Block;
 {
   NonAtomicBlock< Atomic_Force > a_NoBlock;

 }
 return;
}

/*
//Old method ( macros )
void loop()
{
 ATOMIC_BLOCK( ATOMIC_RESTORESTATE ){

   NONATOMIC_BLOCK( NONATOMIC_FORCEOFF ){
     
   }
 }
 return;
}
*/


The 'Protect' function usage.
Each blocking type contains a function named 'Protect'. It can be used to
protect any kind of element.

E.g.

   - Typedefs make using 'Protect' easier.
      typedef AtomicBlock< Atomic_Force > MyBlock;
   
      For the sake of these examples 'i' is an 'int', and inc is the funciton below.
      
Code: [Select]
void inc( int &i_Val ) { ++i_Val; }

   - Protected writes.
      
Code: [Select]
MyBlock::Protect( i ) = analogRead( A0 );

   - Protected reads.
      
Code: [Select]
Serial.println( MyBlock::Protect( i ) );

   - Protected non-member function calls.
      
Code: [Select]
MyBlock::Protect( inc )( i );

   - Protected pointers.
Code: [Select]
*( &MyBlock::Protect( i ) ) = 77;
(*MyBlock::Protect( &i ))++;

      
   - No unnessecary instructions.
   
   - This will not produce any code.
         
Code: [Select]
MyBlock::Protect( 1 + 2 + 3 + 4 + 5 );
      
   - These two lines produce exactly the same code.
Code: [Select]
digitalWrite( MyBlock::Protect( 13 ), digitalRead( MyBlock::Protect( 13 ) ) ^ 1 );
digitalWrite( 13, digitalRead( 13 ) ^ 1 );

      
      - This will only turn interrupts on and off once.
Code: [Select]
MyBlock::Protect( MyBlock::Protect( MyBlock::Protect( MyBlock::Protect( i ) ) ) );
      
      
CAUTION: 'Protect' will only protect one element. Statements as arguments are not going to work as you may expect.         
   E.g.
   
      - Wrong use. (argument statement will not be blocked. If you use the result, it will be inside the atomic block.)
         
Code: [Select]
MyBlock::Protect( PORTK |= _BV( 5 ) );
         
      - Correct usage. (just 'Protect' PORTK)
         
Code: [Select]
MyBlock::Protect( PORTK ) |= _BV( 5 );
         
LIMITATIONS:

   * I have chosen not to support any sort of member function protection. Once I have validated the current system
   I can look further into it. The required interface seems to generalise the type system too much and breaks some
   existing functionality as the compiler cannot disambiguate the control paths.

If you aren't logged in you can also download the code from 'google code' here.

I'm after any feedback/ideas for improvment.
Cheers,
Chris.

The files listed in this post are:

  • AtomicBlock.h This file is the original AVR stable version.

  • AtomicBlock_121b.h This file is the new multi-platform (beta) version.


westfw

Quote
[the old syntax] looks like a function definition nested inside another function

I dunno.  It looks like a pretty common macro structure to me; see the recent RunEvery macro, for instance.  ( http://arduino.cc/forum/index.php/topic,124974.0.html )  I've seen a lot of this sort of macro in major C programs (in some ways, it adds object-oriented behavior to C):
Code: [Select]
FOR_ALL_INTERFACES(idb) {
  FOR_ALL_PACKETS_IN_Q(idb->ipQ) {
    // Blah blah blah.
  }
}


Your new syntax isn't valid in C code...
And the old form is avr-libc "standard", extending well beyond Arduino.

pYro_65

#2
Oct 02, 2012, 02:50 am Last Edit: Oct 02, 2012, 02:59 am by pYro_65 Reason: 1

Your new syntax isn't valid in C code...
And the old form is avr-libc "standard", extending well beyond Arduino.


It is C++, not C macros.
And it does not use any gcc specific attributes making it far more portable.

From avr-libc for the ATOMIC_BLOCK macros.
Quote
Note:
The macros in this header file require the ISO/IEC 9899:1999 ("ISO C99") feature of for loop variables that are declared inside the for loop itself. For that reason, this header file can only be used if the standard level of the compiler (option --std=) is set to either c99 or gnu99.

WizenedEE


And it does not use any gcc specific attributes making it far more portable.

From avr-libc for the ATOMIC_BLOCK macros.
Quote
Note:
The macros in this header file require the ISO/IEC 9899:1999 ("ISO C99") feature of for loop variables that are declared inside the for loop itself. For that reason, this header file can only be used if the standard level of the compiler (option --std=) is set to either c99 or gnu99.



Along with this whole thing which is certainly not standard:
Code: [Select]

#define ATOMIC_RESTORESTATE uint8_t sreg_save \
__attribute__((__cleanup__(__iRestore))) = SREG

Coding Badly

Code: [Select]
         {
            NonAtomicBlock< Atomic_Force > a_NoBlock;
            // protected code goes here
            // destructor has to be called here or very bad things happen           
         }


You rely on the fact that the descructor is called precisely at the end of scope.  What happens if the optimizer realizes a_NoBlock is not used and invokes the destructor early?

pYro_65

#5
Oct 02, 2012, 04:22 am Last Edit: Oct 02, 2012, 04:24 am by pYro_65 Reason: 1
The fact that it has side effects cause it to work correctly.

If a class only modifies its internal data, it has no side effects and the compiler can optimise it away.

However this class ( or group of classes ) write to external data during destruction, the status register/SREG. More importantly, it reads from an external source marked as volatile ( SREG ) during construction so the compiler is forced to omit instructions.

Coding Badly


I suspect you misunderstood the question.  But it doesn't matter.  The C++ standard states the destructor has to be called and the end of the block.

pYro_65

Yes I didn't quite answer your question, but I'm hoping that I haven't assumed anything with my design. The ATOMIC_BLOCK macros state they cover all exit paths and hopefully this is the same. Please anyone let me know if you think something is not as it should be.

pico

I'll try out your new approach by replacing some of the ATOMIC_BLOCK macros I'm currently using a number of programs. I also recently ported the ATOMIC_BLOCK macros over to G++ for an ARM Cortex3 architecture (Leaflabs Maple board) for my own use -- I'll try to port your approach as well to test portability.

I also agree it would be nice for portability to get an implementation that doesn't rely on a relatively exotic feature of the Gnu compilers.

Also, since the original ATOMIC_BLOCK macros are actually based on a "for" loop under the hood to specify the code block, it would be theoretically possible to inadvertantly place a "break" or "continue" statement in such a block, and the compiler would not pick it up as out of place. Not a biggie in practice, but a bug yours doesn't suffer from.

WiFi shields/Yun too expensive? Embeddedcoolness.com is now selling the RFXduino nRF24L01+ <-> TCP/IP Linux gateway: Simpler, more affordable, and even more powerful wireless Internet connectivity for *all* your Arduino projects! (nRF24L01+ shield and dev board kits available too.)

pYro_65

Cool, if you replace existing ATOMIC_BLOCK macros with my version and you aren't using the safe mode, the compiled size should be the same.

Let me know how it turns out. :)

WizenedEE


Also, since the original ATOMIC_BLOCK macros are actually based on a "for" loop under the hood to specify the code block, it would be theoretically possible to inadvertantly place a "break" or "continue" statement in such a block, and the compiler would not pick it up as out of place. Not a biggie in practice, but a bug yours doesn't suffer from.


Actually no. The macros don't use the third part of a for loop, which runs at the end of the oop but can be "break"d, but rather a gcc attribute that says to run this piece of code when this variable is destructed.


Also C++ doesn't support longjmp --- destructors aren't called. I'm pretty sure this wouldn't work (please note I have never used setjmp and am not bothering to look up the syntax ;)
Code: [Select]

void setup() {
  pinMode(13, OUTPUT);
  if (setjmp()) {
    digitalWrite(13, HIGH);
  }
  else {
    AtomicBlock<Atomic_Force> a;
    longjmp();
  }
  // Interrupts disabled
}

pYro_65

#11
Oct 03, 2012, 03:56 am Last Edit: Oct 03, 2012, 03:59 am by pYro_65 Reason: 1
Good find, I haven't really read about those functions myself.

But, no that wouldn't work, I don't think ATOMIC_BLOCK would either if you did a 'longjmp' in its scope.

I think this case is fairly well covered as the standard states it completely subverts stack unwinding, and requires 'setjmp' to be set, and is the clients explicit choice. If the user doesn't read documentation on longjmp and mistakes it for an asm jump for example, they have more to worry about than ISR safe code.

It also appears exception handling is the C++ way of "long distance transfer of flow control", which doesn't bypass the C++ stack unwinding semantics.

I'd imagine using it would be quite destructive to a C++ program on a micro controller.

Coding Badly


The only caveat (as far as I can tell) is placement.  Constructors are called in order and destructors in reverse order.  If a different class constructor (or destructor) has to be protected, yours has to be first in the block...

Code: [Select]
{
  NonAtomicBlock< Atomic_Force > a_NoBlock;
  SuperHardwareThingy aThingy;  // Constructor accesses hardware so it must be protected.  It is protected because it comes after the line above.
}


If accidentally reversed, disaster ensues...

Code: [Select]
{
  SuperHardwareThingy aThingy;  // Constructor accesses hardware so it must be protected.  Oops.
  NonAtomicBlock< Atomic_Force > a_NoBlock;
}


The Libc macro does not have the same potential problem.

pico



Also, since the original ATOMIC_BLOCK macros are actually based on a "for" loop under the hood to specify the code block, it would be theoretically possible to inadvertantly place a "break" or "continue" statement in such a block, and the compiler would not pick it up as out of place. Not a biggie in practice, but a bug yours doesn't suffer from.


Actually no.



Actually, yes.

Try compiling the following code. The compiler happily accepts the break and continue statements in the setup() function, but rejects them in the loop() function.


Code: [Select]


#include <util/atomic.h>

void setup()
{
  ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
  {
    int a =0;
    if (a++ > 0) continue;
    else break;
  }
}

void loop()
{
  int a =0;
  if (a++ > 0) continue;
  else break;
}


The compiler messages are:
Code: [Select]

sketch_oct04a.cpp: In function 'void loop()':
sketch_oct04a:16: error: continue statement not within a loop
sketch_oct04a:17: error: break statement not within loop or switch


OTOH, comment out the offending code in the loop() function, and the compiler accepts the break and continue statements in the setup() function happily.

Code: [Select]

Binary sketch size: 456 bytes (of a 32256 byte maximum)


Since it's a "for" loop, I can't see how the compiler could NOT be expected to accept both break and continue statements as legal C statements in that context (because they are).

A bug, but one of perhaps of theoretical interest only. Still, it's a bug Pyro's approach doesn't suffer from, which was my point.

On a practical note, the other advantage of the ATOMIC_BLOCK macros over anything else that is likely to be a contender is that the macros have been very carefully tested for all the subtle problems that can occur with instruction reordering optimisations, etc. The story behind the development by Dean Camera (abcminiuser) was documented in an interesting thread on AVRFreaks.

http://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewtopic&t=45041&postdays=0&postorder=asc&highlight=atomic

It would take a fair degree of analysis and testing before any replacement would reach the same level of acceptance, I think. 
WiFi shields/Yun too expensive? Embeddedcoolness.com is now selling the RFXduino nRF24L01+ <-> TCP/IP Linux gateway: Simpler, more affordable, and even more powerful wireless Internet connectivity for *all* your Arduino projects! (nRF24L01+ shield and dev board kits available too.)

WizenedEE


... 


Whoops, sorry. I misread your post as "The avr-libc macros won't reenable macros if break is used" which I still believe is false. You are correct that break is still a valid statement, and that this would be an ever greater problem:
Code: [Select]

volatile int count; // updated by interrupt

while(1) {
  if (digitalRead(2)) {
    break; // Breaks out of outer while
  }
  ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
    if (count > 5) break; // BUG, outer while is not `broken'.
  }
}

Go Up
 

Quick Reply

With Quick-Reply you can write a post when viewing a topic without loading a new page. You can still use bulletin board code and smileys as you would in a normal post.

Warning: this topic has not been posted in for at least 120 days.
Unless you're sure you want to reply, please consider starting a new topic.

Note: this post will not display until it's been approved by a moderator.
Name:
Email:

shortcuts: alt+s submit/post or alt+p preview