[riot-notifications] [RIOT-OS/RIOT] cpu/arm7_common: Make irq_*() compiler barriers (#11440)
notifications at github.com
Thu Apr 25 11:08:03 CEST 2019
maribu commented on this pull request.
> return retval;
- __asm__ volatile(" mrs %0, cpsr" : "=r"(retval) : /* no inputs */);
+ __asm__ volatile(" mrs %0, cpsr" : "=r"(retval) : /* no inputs */ : "memory");
> Uff, is the compiler allowed to do this?
If a transformation does not change visible behavior from the compilers point of view, it is legal. The compiler is unaware of concurrent memory accesses. So speculative stores to `x` in the example above would be totally valid from the compilers viewpoint (even if `x` is a global), as the compiler does not take into account that other threads could read `x` at that very instance when the speculative store was done and not fixed subsequently.
> Do you have a reference?
This article. https://lwn.net/Articles/586838/ (If you want to fast-forward to relevant part search for speculative stores
In more detail: If the inline assembly in `irq_is_in()` is only marked as `volatile`, but no (compiler) barrier is added. Thus, non-`volatile` accesses can be reordered with regard to `irq_is_in()`. The store `x = 3` is neither `volatile` nor guarded by a memory barrier, so it could be moved up. If that store was done incorrectly, the subsequent `x = 7` will fix it. So in a single-threaded context this transformation will work just fine - which is all the compiler needs to prove.
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the notifications