<p></p>
<p><b>@maribu</b> commented on this pull request.</p>

<hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/13977#discussion_r427050457">sys/pm_layered/pm.c</a>:</p>
<pre style='color:#555'>> +    unsigned state = irq_disable();
+    pm_blocker_t result = pm_blocker;
+    irq_restore(state);
+    return result;
</pre>
<p>There sadly is no guarantee, that the compiler will generate a single read whenever possible. There were examples on GCC generating two 32 bit stores on x86_64 when writing to a <code>volatile</code>, as two stores with an immediate where faster than preparing the correct value in a register and writing that back to memory. As a result, some linux drivers stopped working when compiled with that GCC version. I think it is safe to say that the Linux kernel guys convinced the compiler guys that their compilers have no host to run on if they break kernels by splitting a single <code>volatile</code> accesses into two accesses. But for non-<code>volatile</code> accesses we should still be careful to do any assumptions on how the generated assembly looks like.</p>
<p>So what we could IMO safely do is something like this:</p>
<div class="highlight highlight-source-c"><pre><span class="pl-c1">pm_blocker_t</span> <span class="pl-en">pm_get_blocker</span>(<span class="pl-k">void</span>)
{
    <span class="pl-c1">pm_blocker_t</span> result;
    <span class="pl-k">if</span> (ARCH_32BIT) {
        <span class="pl-k">volatile</span> <span class="pl-c1">uint32_t</span> *tmp = &pm_blocker.<span class="pl-smi">val_u32</span>;
        result.<span class="pl-smi">val_u32</span> = *tmp;
    }
    <span class="pl-k">else</span> {
        <span class="pl-k">unsigned</span> state = <span class="pl-c1">irq_disable</span>();
        result = pm_blocker;
        <span class="pl-c1">irq_restore</span>(state);
    }
    <span class="pl-k">return</span> result;
}</pre></div>
<p>It would be nice to make the <code>FEATURES</code> like <code>arch32_bit</code> available to C code for this. The compiler should than drop the dead branch.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/RIOT-OS/RIOT/pull/13977#discussion_r427050457">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABE7WYC5KTRPEADCKCQKRB3RSIO3VANCNFSM4MTRVA3A">unsubscribe</a>.<img src="https://github.com/notifications/beacon/ABE7WYEMBQKWAH2JHLW2MZDRSIO3VA5CNFSM4MTRVA3KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGODCXRB5Y.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/RIOT-OS/RIOT/pull/13977#discussion_r427050457",
"url": "https://github.com/RIOT-OS/RIOT/pull/13977#discussion_r427050457",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>