[riot-notifications] [RIOT-OS/RIOT] cpu/atmega_common: Bugfixes in irq_arch.c (#11452)

Marian Buschsieweke notifications at github.com
Fri Apr 26 10:52:21 CEST 2019


### Contribution description

1. Fixed the return code of `irq_enable()`:
    - Previously the status register after enabling the interrupts was returned, not prior to enabling it
        - The doc states the return value is the "Previous value of the status register", not the new value
    - The first commit changes this to return the new value
2. Added required memory barriers to `irq_*()` family of functions
    - Added barriers to prevent the compiler from reordering code across calls to `irq_disable()`, `irq_restore()` and `irq_enable()`
    - Added barrier to `irq_is_in()`


### Testing procedure
- Regarding the fixed return value:
    - I personally think a code review is sufficient here. If the reviewer insists, I will come up with a test
- Regarding the barriers:
    - The optimization potential on the AVR platform by reordering or combining memory accesses is practically zero. Most instructions are executed in one cycle and access to memory can only be performed in 8 bit chunks
        - Thus: Reordering and combining memory accesses is unlikely to yield any benefit.
        - Thus: The compiler will not reorder or combine memory accesses most of the time even without memory barriers
    - I compiled `examples/default` for the Arduino Mega2560 using GCC 8.3.0 before (master + fix of return value of `irq_enable()`) and after (this PR):
        - With LTO: https://www.mari-bu.de/avr_irq_lto.html
        - Without LTO: https://www.mari-bu.de/avr_irq_no_lto.html
    - As seen this does not change the binaries, so there cannot be a regression introduced.

### Issues/PRs references
Same as https://github.com/RIOT-OS/RIOT/pull/11440, but for AVR. Related to https://github.com/RIOT-OS/RIOT/pull/11438
You can view, comment on, or merge this pull request online at:

  https://github.com/RIOT-OS/RIOT/pull/11452

-- Commit Summary --

  * cpu/atmega_common: Fix return value of irq_enable
  * cpu/atmega_common: Add barriers to irq_*()

-- File Changes --

    M cpu/atmega_common/irq_arch.c (13)

-- Patch Links --

https://github.com/RIOT-OS/RIOT/pull/11452.patch
https://github.com/RIOT-OS/RIOT/pull/11452.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11452
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190426/635c0577/attachment-0001.html>


More information about the notifications mailing list