<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 11/16/13 07:17, Renato Golin wrote:<br>
</div>
<blockquote
cite="mid:CAMSE1kfgezYHbKqPYrryDLTtC4S7DAU+Jtc2UqXWy6gB9AD3aA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">On 16 November 2013 13:38, Tinti <span
dir="ltr"><<a moz-do-not-send="true"
href="mailto:viniciustinti@gmail.com" target="_blank">viniciustinti@gmail.com</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 dir="ltr">Hi,
<div><br>
</div>
<div>I think we should change one of our patches to not
use 'static register' as comment in this link <a
moz-do-not-send="true"
href="http://www.lemoda.net/c/keywords/register.html"
target="_blank">http://www.lemoda.net/c/keywords/register.html</a></div>
</div>
</blockquote>
<div><br>
</div>
<div>Hi Tinti,</div>
<div><br>
</div>
<div>The C standard (6.7.1p5) says:</div>
<div><br>
</div>
<div>"A declaration of an identifier for an object with
storage-class specifier register</div>
<div>suggests that access to the object be as fast as
possible. The extent to which such</div>
<div>suggestions are effective is implementation-defined."</div>
<div><br>
</div>
<div>Which basically means, "register" is just a hint.
Compilers can, and do, ignore such hints over
optimizations, register pressure, vectorization, etc.</div>
</div>
</div>
</div>
</blockquote>
Understood. However, in this case it only works if it is guaranteed
to be a register. The point is accessing the stack pointer as a C
variable.<br>
<br>
We'd actually like to replace it's use with
__builtin_stack_pointer() (something we're trying to get added to
both clang and gcc).<br>
<br>
<blockquote
cite="mid:CAMSE1kfgezYHbKqPYrryDLTtC4S7DAU+Jtc2UqXWy6gB9AD3aA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>While the standard doesn't allow you to take the
address of a register variable (6.5.3.2p1), all the other
constraints are undefined behaviour.</div>
</div>
</div>
</div>
</blockquote>
Well, in this case the kernel is using a gcc extension, so the
current codes sadly uses the "undefined behaviour" part. :(<br>
<br>
<blockquote
cite="mid:CAMSE1kfgezYHbKqPYrryDLTtC4S7DAU+Jtc2UqXWy6gB9AD3aA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>The problem with "static register" is not that both
cannot coexist, but that "At most, one storage-class
specifier may be given in the declaration specifiers in a</div>
<div>declaration" (6.7.1p1).</div>
</div>
</div>
</div>
</blockquote>
Yup. The static part should be removed. My bad.<br>
<br>
<blockquote
cite="mid:CAMSE1kfgezYHbKqPYrryDLTtC4S7DAU+Jtc2UqXWy6gB9AD3aA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<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 dir="ltr">
<div>#elif defined(__clang__)
<br>
</div>
<div>
<div>#define __builtin_stack_pointer() ({ \
</div>
<div> unsigned long current_sp; \
</div>
<div> asm ("mov %0, sp" : "=r" (current_sp)); \
</div>
<div> current_sp; \
</div>
<div>})</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>I thought you guys were using the modified version,
without the mov...</div>
</div>
</div>
</div>
</blockquote>
Still in the current patch. I haven't had a chance to sub in the new
version yet. At the moment, the version above works for the testing
we're doing, but it's not the final one.<br>
<br>
<blockquote
cite="mid:CAMSE1kfgezYHbKqPYrryDLTtC4S7DAU+Jtc2UqXWy6gB9AD3aA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<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 dir="ltr">
<div>
<div>#else /* gcc */</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>This is horrendous! Is this in the kernel, or is it one
of our local patches?</div>
</div>
</div>
</div>
</blockquote>
That is the way it is currently done in the kernel with gcc. I've
merely moved it to one place from the various places it is copied to
in the ARM portion of the kernel.<br>
<br>
The x86 kernel does exactly this (define a global named register
variable to access "esp").<br>
<br>
<blockquote
cite="mid:CAMSE1kfgezYHbKqPYrryDLTtC4S7DAU+Jtc2UqXWy6gB9AD3aA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>We should never assume anything on the #else, but
specifically require what you want.</div>
<div><br>
</div>
<div>#else should *only* have one of:</div>
<div> * A behaviour that is commonly known as global,
generally as an alternative to a very specific behaviour
in the</div>
</div>
</div>
</div>
</blockquote>
Which is exactly what this patch does.<br>
<br>
<blockquote
cite="mid:CAMSE1kfgezYHbKqPYrryDLTtC4S7DAU+Jtc2UqXWy6gB9AD3aA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> #if</div>
<div> * An error message</div>
</div>
</div>
</div>
</blockquote>
Which would break the kernel. gcc IS the default case.<br>
<br>
As written right now it tries to use __builtin_stack_pointer() if
available (which allows us to test), or if clang is defined uses the
clang patch, else does it the gcc way.<br>
<br>
<blockquote
cite="mid:CAMSE1kfgezYHbKqPYrryDLTtC4S7DAU+Jtc2UqXWy6gB9AD3aA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<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 dir="ltr">
<div>
<div>static register unsigned long
current_stack_pointer asm ("sp");
<==================</div>
<div><br>
</div>
</div>
<div>If we do so, we get kernel-gcc back to work on
vexpress. What do you think?</div>
</div>
</blockquote>
</div>
<br>
</div>
<div class="gmail_extra">Global register variables doesn't make
sense. But this is clearly a special case, which means all
bets are off.</div>
</div>
</blockquote>
Making it local register variables just spreads the pain out. At
least at a global place you can apply the other versions of the
code.<br>
<br>
Currently in the ARM kernel they are all spread out as local
register variables. In the x86 kernel they are done as a global
(like my patch is trying to do now for ARM).<br>
<br>
<blockquote
cite="mid:CAMSE1kfgezYHbKqPYrryDLTtC4S7DAU+Jtc2UqXWy6gB9AD3aA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">I'd be inclined to say that just
"static" would be enough there, but that depends on how GCC
interpret asm variables. What is the original code for that
line in the vanilla kernel?</div>
</div>
</blockquote>
The version in the #else that you don't like. It's that very line of
code we are trying to get to work with clang.<br>
<br>
I know. It isn't ideal. That's why we want __builtin_stack_pointer()<br>
<br>
Behan<br>
<pre class="moz-signature" cols="72">--
Behan Webster
<a class="moz-txt-link-abbreviated" href="mailto:behanw@converseincode.com">behanw@converseincode.com</a></pre>
</body>
</html>