[llvmlinux] Multiple storage class error

Behan Webster behanw at converseincode.com
Sat Nov 16 21:54:52 UTC 2013


On 11/16/13 07:17, Renato Golin wrote:
> On 16 November 2013 13:38, Tinti <viniciustinti at gmail.com
> <mailto:viniciustinti at gmail.com>> wrote:
>
>     Hi,
>
>     I think we should change one of our patches to not use 'static
>     register' as comment in this
>     link http://www.lemoda.net/c/keywords/register.html
>
>
> Hi Tinti,
>
> The C standard (6.7.1p5) says:
>
> "A declaration of an identifier for an object with storage-class
> specifier register
> suggests that access to the object be as fast as possible. The extent
> to which such
> suggestions are effective is implementation-defined."
>
> Which basically means, "register" is just a hint. Compilers can, and
> do, ignore such hints over optimizations, register pressure,
> vectorization, etc.
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.

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).

> 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.
Well, in this case the kernel is using a gcc extension, so the current
codes sadly uses the "undefined behaviour" part. :(

> 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
> declaration" (6.7.1p1).
Yup. The static part should be removed. My bad.

>     #elif defined(__clang__)                                          
>                  
>     #define __builtin_stack_pointer() ({ \                            
>                  
>             unsigned long current_sp; \                              
>                   
>             asm ("mov %0, sp" : "=r" (current_sp)); \                
>                   
>             current_sp; \                                            
>                   
>     })
>
>
> I thought you guys were using the modified version, without the mov...
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.

>     #else /* gcc */
>
>
> This is horrendous! Is this in the kernel, or is it one of our local
> patches?
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.

The x86 kernel does exactly this (define a global named register
variable to access "esp").

> We should never assume anything on the #else, but specifically require
> what you want.
>
> #else should *only* have one of:
>  * A behaviour that is commonly known as global, generally as an
> alternative to a very specific behaviour in the
Which is exactly what this patch does.

> #if
>  * An error message
Which would break the kernel. gcc IS the default case.

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.
 
>
>     static register unsigned long current_stack_pointer asm ("sp");  
>                <==================
>
>     If we do so, we get kernel-gcc back to work on vexpress. What do
>     you think?
>
>
> Global register variables doesn't make sense. But this is clearly a
> special case, which means all bets are off.
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.

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).

> 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?
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.

I know. It isn't ideal. That's why we want __builtin_stack_pointer()

Behan

-- 
Behan Webster
behanw at converseincode.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxfoundation.org/pipermail/llvmlinux/attachments/20131116/38d80db0/attachment-0001.html>


More information about the LLVMLinux mailing list