C++ version to avr/interrupt.h and util/atomic.h

Go To Last Post
12 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I wrote a C++ version to the the C headers avr/interrupt.h and util/atomic.h from AVR Libc. Nothing especial here, I'm using RAII to implement an atomic scope to execute a block of code instead the clever macro ATOMIC_BLOCK that uses a for loop and the GCC attribute cleanup. The macro functions cli/sei() were replaced by always_inline functions defined inside a namespace:

using namespace avr::interrupt;

on(); //enable global interrupts
off(); //disable global interrupts

{ //scope with code executed without being disturbed by interrupts
  atomic sa;
  //code
} //global interrupts are always enabled at the end

{
  atomic sa{restore};
  //code
} //global interrupts are enabled at the end if they were enabled before

There is also a support to help the programmer to avoid reordering code when timing is a critical factor. Let's consider the code below:

using namespace avr::interrupt;
unsigned int ivar;
void f(unsigned int val) {
  val = 65535U / val;
  atomic sa;
  ivar = val;
}

Using avr-gcc 10.2 with -Os we have the following:

movw	r22, r24
cli
ldi	r24, 0xFF	; 255
ldi	r25, 0xFF	; 255
rcall	.+18		; 0x4e <__udivmodhi4>
sts	0x0061, r23	; 0x800061 <ivar+0x1>
sts	0x0060, r22	; 0x800060 <ivar>
sei
ret

The division call is inside the critical region. We can achieve a better result helping the compiler:

using namespace avr::interrupt;
unsigned int ivar;
void f(unsigned int val) {
  val = 65535U / val;
  atomic sa(on_at_the_end, val);
  ivar = val;
}

This generates:

movw	r22, r24
ldi	r24, 0xFF	; 255
ldi	r25, 0xFF	; 255
rcall	.+20		; 0x4e <__udivmodhi4>
cli
sts	0x0061, r23	; 0x800061 <ivar+0x1>
sts	0x0060, r22	; 0x800060 <ivar>
sei
ret

Note that the call to __udivmodhi4 is now outside the region delimited by the instructions cli and sei.

https://github.com/ricardocosme/avrINT

github.com/ricardocosme

avrIO: Operation of I/O port pins and I/O registers.

Last Edited: Wed. Feb 24, 2021 - 02:29 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

> The division call is inside the critical region. We can achieve a better result helping the compiler:

 

The ivar will most likely be volatile, otherwise it would need no protection, so when guarding a volatile the need for anything special then may also disappear-

 

https://godbolt.org/z/xvEjao

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

rcosme wrote:
I'm using RAII to implement
Yeah but, what if the user writes:

void f(unsigned int val) {
  val = 65535U / val;
  atomic sa;
  ivar = val;
  complex_function();
  float f = sin(PINB);
   // and other complex stuff
}

The atomic lock applies when "sa" is constructed but does not go out of context, be destructed, and have the "off switch" for the protection until the end of the function. So all the complex stuff is being done INSIDE the I reg protection? Surely not what you want? The disable of I has to be for the shortest time possible.

 

I suppose the solution is:

void f(unsigned int val) {
  val = 65535U / val;
  {
    atomic sa;
    ivar = val;
  }
  complex_function();
  float f = sin(PINB);
  // and other complex stuff
}

but then (apart from the quantity of typing) is this really any better than?:

void f(unsigned int val) {
  val = 65535U / val;
  ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
    ivar = val;
  }
  complex_function();
  float f = sin(PINB);
  // and other complex stuff
}

(Oh and remember AVR-LibC has to cater for both C and C++).

Last Edited: Wed. Feb 24, 2021 - 09:20 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

I suppose the solution is:

//...
{
  atomic sa;
  ivar = val;
}
//...


Yes, this is what is done in C++ to use RAII with a narrow scope.

clawson wrote:

but then (apart from the quantity of typing) is this really any better than?:

void f(unsigned int val) {
  //...
  ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
    ivar = val;
  }
  //...
}


Actually, you need to do the following to restore the state at the end of the scope:

void f(unsigned int val) {
  //...
  {
    atomic sa{restore}
    ivar = val;
  }
  //...
}

[Thinking: maybe it's a better idea to use the 'restore' policy as default instead of 'on_at_the_end'(this is the 'ATOMIC_FORCEON' in AVR Libc) because the last seems to me more uncommon]

FYI: https://github.com/ricardocosme/...

IMO the usage of RAII using ctor and dtor is more natural in C++ to solve this type of problem.

github.com/ricardocosme

avrIO: Operation of I/O port pins and I/O registers.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yeah, I would suggest "Restorestate" is the one used 99% of the time. (that's kind of the point you want I left exactly as you started before entering the protected region).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

Yeah, I would suggest "Restorestate" is the one used 99% of the time.

Right, thanks for the feedback.

github.com/ricardocosme

avrIO: Operation of I/O port pins and I/O registers.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

>Yeah, I would suggest "Restorestate" is the one used 99% of the time

 

I would even skip the ability to do anything else. The saving of 1 instruction if you know irq's are on is not worth the addition of more options. You get one little class that does one simple job, and everyone can see what it does and how it does it.

 

~5 lines of code handles most needs. 

 

struct InterruptLock {
    InterruptLock   () : sreg_(SREG) { asm("cli"); }
    ~InterruptLock  () { SREG = sreg_; }
    private:
    u8 sreg_;
};

 

volatile time_t systemSeconds;

 

static auto  time        ()             { InterruptLock lock; return systemSeconds; }

static auto  time        (time_t t)     { InterruptLock lock; systemSeconds = t; }
static auto  localtime   (tm& tmptr)    { time_t t = time(); localtime_r( &t, &tmptr ); }

 

int main(){

    tm now;
    localtime( now );

 

 

Another good reason to keep it simple is you can create this class on the fly if needed, from memory as its so simple, or stick it in some common header, etc. You can then use it anywhere, such as posting examples here or on compiler explorer, and will not be tied down to a 3-4 header file version which contains variations for whatever std is in use. When simple, it also means you can use it as an example for other uses-

 

struct ScopeBackup { //backup a register, restore when exit scope
    ScopeBackup   (volatile u8& reg) : reg_(reg), backup_(reg) {}
    ~ScopeBackup  () { reg_ = backup_; }
    private:
    volatile u8& reg_;
    u8 backup_;
};

 

or template so you do not need to create versions for every type that can be used-

 

template<typename T>
struct ScopeBackup {
    ScopeBackup   (T& reg) : reg_(reg), backup_(reg) {}
    ~ScopeBackup  () { reg_ = backup_; }
    private:
    T& reg_;
    u8 backup_;
};

 

ISR(TCA0_CMP0_vect){

    //main code may be doing something that uses TEMP, so backup/restore

    ScopeBackup tempBak{ TCA0.SINGLE.TEMP };

    //do a read/write on a register that affects temp

    //TEMP is restored, and main code does not need to protect a 16bit write to a tca0 register

}

 

Last Edited: Wed. Feb 24, 2021 - 09:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

The saving of 1 instruction if you know irq's are on is not worth the addition of more options.

zero-overhead principle

github.com/ricardocosme

avrIO: Operation of I/O port pins and I/O registers.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well, you then end up with a truckload of parts and you only use a handful.

 

Or rather, your attic collects all these things that you think you will use someday, but never do.

 

Keep the common parts on hand, when you need the unusual part not in stock just order it.

 

Storage of extra bits is free on a pc, but you still have to stumble over all the extra bits when looking for the needed bits.

 

</metaphors>

 

You also get to maintain all that extra code. New std, new version. More features, new version. Plus in this case you allow the potential to introduce a problem if used wrong- sei gets used when its not supposed to if you thought irq's were on but they were not (and were off for a good reason that may not be apparent in the function). If you only allow one simple version, it cannot be used wrong.

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Well, you then end up with a truckload of parts and you only use a handful.
Or rather, your attic collects all these things that you think you will use someday, but never do.

I'm using the two variants.

 

Quote:

(...) Plus in this case you allow the potential to introduce a problem if used wrong- sei gets used when its not supposed to if you thought irq's were on but they were not (and were off for a good reason that may not be apparent in the function). If you only allow one simple version, it cannot be used wrong.

Design by contract, which means {Pre,post}conditions and invariants, it's a good approach to be adopted when designing solutions. It's reasonable a postcondition that the interrupts are enabled. This isn't only more efficient when the postcondition is possible, but it can also be simpler to reason about the code.

github.com/ricardocosme

avrIO: Operation of I/O port pins and I/O registers.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

IIRC ATOMIC_BLOCK exists because of the possibility of code motion enlarging the critical section when relying on just sei and cli.

When new, I'm not sure it was actually guaranteed to work, but it did in more cases.

Once the memory block was added to sei() and cli(), I think it was guaranteed to work,

but no longer necessary.

Iluvatar is the better part of Valar.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

IIRC ATOMIC_BLOCK exists because of the possibility of code motion
enlarging the critical section when relying on just sei and cli.

It exists because it's error prone and sometimes a tedious task to use the pair cli/sei(or restore). When you have multiple exit points in the function you need to call sei or restore the SREG state at each point. The problem would be worse if we were using C++ exceptions(I think that this can be possible in the future with zero-overhead deterministic exceptions[1]).

The motion of the code(reordering code) is orthogonal to the RAII usage.

 

Quote:

When new, I'm not sure it was actually guaranteed to work, but it did in
more cases.

Once the memory block was added to sei() and cli(), I think it was
guaranteed to work, but no longer necessary.


AFAIK the "first" version of cli/sei didn't use a memory fence, so some code motion could be done at specific usages.[2]

In the end, it's a bad choice use the pair cli/sei(or restore) without RAII.

 

[1] http://www.open-std.org/jtc1/sc2...
[2] https://lists.nongnu.org/archive...

 

github.com/ricardocosme

avrIO: Operation of I/O port pins and I/O registers.