Follow-up Comment #4, bug #22163 (project avr-libc):
I've just tried this code section, and there is a good reason for the "unused
variable 'sreg_save'" warning - looking at the generated assembly, the
variable *is* unused. In other words, the SREG is not restored after the
block, as it is in the C version.
Could it be that the __attribute__((__cleanup__)) attribute is not working
correctly for avr c++, at least when used in this way? If that's the case,
then this is definitely a bug.
I've noticed another issue with the atomic.h functions - I don't think the
memory blocks (asm("":::"memory")) are in the right places. It's important to
ensure that any changes to memory are written out before interrupts are
enabled - thus the block should appear *before* the sei() instructions, and
before restoring SREG. The block is unnecessary before disabling interrupts.
I could be wrong, but I *think* that's the correct ordering.
In C++ it is normally considered better to use a class and RAII for something
like this. You should have a class such as:
class CriticalSection {
public:
CriticalSection() : _sreg( SREG ) { cli(); }
~CriticalSection() { asm("" ::: "memory"); SREG = _sreg; }
private:
uint8_t _sreg;
};
Then you simply declare a "CriticalSection" variable in the block you want to
protect. Similar classes can be defined for non-critical sections, and for
when you want to restore the old interrupt enable state, or to force the flag
on or off.
As a workaround for the bug blocking cleanup functions, it's possible to use
a RAII class within the same framework as the C-style atomic blocks. It's a
little messy, but gives identical optimal assembly code (in my brief testing,
anyway). I haven't gone through the details of patching this into atomic.h
(I'm not sure what the policy of including c++ specific stuff is), but it can
be added on at the end of atomic.h as it re-defines some macros:
#ifdef __cplusplus
// Override these macros:
#undef ATOMIC_RESTORESTATE
#undef ATOMIC_FORCEON
#undef NONATOMIC_RESTORESTATE
#undef NONATOMIC_FORCEOFF
class RestoreState {
public:
RestoreState() : _x( SREG ), _restore( true ) {}
// For keeping sreg
RestoreState(uint8_t x) : _x( x ), _restore( false ) {} // For
__ToDo loop
hack
~RestoreState() { if (_restore) SREG = _x; }
operator bool() const { return _x; };
// For testing __ToDo
private:
uint8_t _x;
bool _restore;
};
class ForceOn {
public:
ForceOn() : _x( 0 ), _restore( true ) {}
// For ignoring sreg
ForceOn(uint8_t x) : _x( x ), _restore( false ) {} // For
__ToDo loop hack
~ForceOn() { if (_restore) sei(); }
operator bool() const { return _x; };
// For testing __ToDo
private:
uint8_t _x;
bool _restore;
};
class ForceOff {
public:
ForceOff() : _x( 0 ), _restore( true ) {}
// For ignoring sreg
ForceOff(uint8_t x) : _x( x ), _restore( false ) {} // For
__ToDo loop
hack
~ForceOff() { if (_restore) cli(); }
operator bool() const { return _x; };
// For testing __ToDo
private:
uint8_t _x;
bool _restore;
};
#define ATOMIC_RESTORESTATE RestoreState sreg_save
#define ATOMIC_FORCEON ForceOn sreg_save
#define NONATOMIC_RESTORESTATE RestoreState sreg_save
#define NONATOMIC_FORCEOFF ForceOff sreg_save
#endif
_______________________________________________________
Reply to this item at:
<http://savannah.nongnu.org/bugs/?22163>
_______________________________________________
Message sent via/by Savannah
http://savannah.nongnu.org/
_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@nongnu.org
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev
