<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Nov 21, 2013 at 2:32 PM, Linus Torvalds <span dir="ltr"><<a href="mailto:torvalds@linux-foundation.org" target="_blank">torvalds@linux-foundation.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">On Thu, Nov 21, 2013 at 8:02 AM, Alexander Holler <<a href="mailto:holler@ahsoftware.de">holler@ahsoftware.de</a>> wrote:<br>
><br>
</div><div class="im">> Luis Lozano just noted (see <a href="https://lkml.org/lkml/2013/11/20/625" target="_blank">https://lkml.org/lkml/2013/11/20/625</a>) that<br>
> current_thread_info() has the prototype<br>
><br>
> static inline struct thread_info *current_thread_info(void)<br>
> __attribute_const__;<br>
><br>
> on arm (and arm64 and unicore32, something the paste from Mathieu missed so<br>
> most people here might have missed that detail too). It's a very good<br>
> finding from Luis.<br></div></blockquote><div><br></div><div>Can someone provide a test case? We are unable to reproduce the problem in ChromeOS land. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">
<br>
</div>No, because it is immaterial.<br>
<br>
We *want* gcc to optimize away multiple accesses to "sp". Because it<br>
doesn't *matter* whether "sp" changes or not, the *result* is always<br>
the same. That's what the "const" means.<br>
<br>
The "& ~(THREAD_SIZE - 1)" part will remove all the bits that can<br>
change. Really. So the result *is* constant (within one thread).<br>
Marking it constant and telling gcc that it can combine these things<br>
is correct.<br></blockquote><div><br></div><div>Its unfortunate that the attribute '__const__ ' is very misleading - the semantic used by gcc is that this is an aliasing attribute that implies that the function does not depend in any way on global state. Meaning, if called with the same parameters, it expected to return the same value. It should have been called 'pure', but gcc has a different 'pure' attribute that imples something different :-(</div>
<div><br></div><div>That said, gcc should be smart enough to figure out that this function is not doing anything complicated. Again, a test case would be much appreciated.</div><div><br></div><div>Bhaskar</div><div><br></div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Guys, read my email again.<br>
<br>
The bug is not that gcc can re-order or combine the accesses to "sp".<br>
WE WANT THAT TO HAPPEN.<br>
<br>
The bug is *outside* that "current_thread_info()" macro/inline<br>
function. It's the *dereference* of the pointer that gcc re-orders.<br>
AND THAT IS WRONG.<br>
<br>
Gcc seems to mess up the alias analysis, and decide that the<br>
deferences cannot alias. Which is wrong. They clearly *can* alias,<br>
exactly because the value of "sp & ~(THREAD_SIZE - 1)" ends up having<br>
the same value all the time.<br>
<span class=""><font color="#888888"><br></font></span></blockquote><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><font color="#888888">
Linus<br>
</font></span></blockquote></div><br></div></div>