diff mbox

[Xen-devel,11/34] xen/arm: Introduce __builtin_stack_pointer

Message ID 1395766541-23979-12-git-send-email-julien.grall@linaro.org
State Deferred, archived
Headers show

Commit Message

Julien Grall March 25, 2014, 4:55 p.m. UTC
Clang doesn't support named register. Introduce __builtin_stack_pointer
to be able to use named register with gcc.

In file included from arm32/asm-offsets.c:10:
In file included from xen/xen/include/xen/sched.h:18:
In file included from xen/xen/include/xen/smp.h:4:
In file included from xen/xen/include/asm/smp.h:8:
xen/xen/include/asm/current.h:31:33: error: variable 'sp' is uninitialized when used here [-Werror,-Wuninitialized]
    return (struct cpu_info *)((sp & ~(STACK_SIZE - 1)) + STACK_SIZE - sizeof(struct cpu_info));
                                ^~
xen/xen/include/asm/current.h:30:30: note: initialize the variable 'sp' to silence this warning
    register unsigned long sp asm ("sp");
                             ^
                              = 0

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
Cc: Tim Deegan <tim@xen.org>
---
 xen/include/asm-arm/current.h |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Ian Campbell March 25, 2014, 5:18 p.m. UTC | #1
On Tue, 2014-03-25 at 16:55 +0000, Julien Grall wrote:
> Clang doesn't support named register. Introduce __builtin_stack_pointer
> to be able to use named register with gcc.

I think gcc considers the __builtin namespace to be its own, not sure
about clang.

I think get_stack_pointer() would be a fine name for this macro. It
seems like the clang version should work for both gcc and clang.

Google seems to suggest that __builtin_stack_pointer might become a real
compiler builtin at some point.
Julien Grall March 25, 2014, 6:01 p.m. UTC | #2
Hi Ian,

On 03/25/2014 05:18 PM, Ian Campbell wrote:
> On Tue, 2014-03-25 at 16:55 +0000, Julien Grall wrote:
>> Clang doesn't support named register. Introduce __builtin_stack_pointer
>> to be able to use named register with gcc.
> 
> I think gcc considers the __builtin namespace to be its own, not sure
> about clang.

I'm able to compile on GCC without any issue.

> I think get_stack_pointer() would be a fine name for this macro. It
> seems like the clang version should work for both gcc and clang.
> 
> Google seems to suggest that __builtin_stack_pointer might become a real
> compiler builtin at some point.

I took the idea to the llvmlinux project:
http://git.linuxfoundation.org/?p=llvmlinux.git;a=blob;f=arch/arm/patches/current_stack_pointer_arm.patch;

But I can rename into get_stack_pointer for the next version.

Regards,
Ian Campbell March 26, 2014, 10:31 a.m. UTC | #3
On Tue, 2014-03-25 at 18:01 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 03/25/2014 05:18 PM, Ian Campbell wrote:
> > On Tue, 2014-03-25 at 16:55 +0000, Julien Grall wrote:
> >> Clang doesn't support named register. Introduce __builtin_stack_pointer
> >> to be able to use named register with gcc.
> > 
> > I think gcc considers the __builtin namespace to be its own, not sure
> > about clang.
> 
> I'm able to compile on GCC without any issue.

That's not the point. The point of a namespacing rule is that gcc can
add such a function at any time it likes, and if that breaks your
application then that is tough luck.

> > I think get_stack_pointer() would be a fine name for this macro. It
> > seems like the clang version should work for both gcc and clang.
> > 
> > Google seems to suggest that __builtin_stack_pointer might become a real
> > compiler builtin at some point.
> 
> I took the idea to the llvmlinux project:
> http://git.linuxfoundation.org/?p=llvmlinux.git;a=blob;f=arch/arm/patches/current_stack_pointer_arm.patch;
> 
> But I can rename into get_stack_pointer for the next version.

Sounds good, thanks.

Ian.
Julien Grall March 26, 2014, 10:38 a.m. UTC | #4
On 26/03/14 10:31, Ian Campbell wrote:
> On Tue, 2014-03-25 at 18:01 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 03/25/2014 05:18 PM, Ian Campbell wrote:
>>> On Tue, 2014-03-25 at 16:55 +0000, Julien Grall wrote:
>>>> Clang doesn't support named register. Introduce __builtin_stack_pointer
>>>> to be able to use named register with gcc.
>>>
>>> I think gcc considers the __builtin namespace to be its own, not sure
>>> about clang.
>>
>> I'm able to compile on GCC without any issue.
>
> That's not the point. The point of a namespacing rule is that gcc can
> add such a function at any time it likes, and if that breaks your
> application then that is tough luck.

Thanks, I didn't find anything useful on internet about namespace. It 
seems the common rule is __foo is reserved by the compiler.

>>> I think get_stack_pointer() would be a fine name for this macro. It
>>> seems like the clang version should work for both gcc and clang.
>>>
>>> Google seems to suggest that __builtin_stack_pointer might become a real
>>> compiler builtin at some point.
>>
>> I took the idea to the llvmlinux project:
>> http://git.linuxfoundation.org/?p=llvmlinux.git;a=blob;f=arch/arm/patches/current_stack_pointer_arm.patch;
>>
>> But I can rename into get_stack_pointer for the next version.
>
> Sounds good, thanks.

I will rename it and send the pathc.

Regards,
Ian Campbell March 26, 2014, 10:42 a.m. UTC | #5
On Wed, 2014-03-26 at 10:38 +0000, Julien Grall wrote:
> 
> On 26/03/14 10:31, Ian Campbell wrote:
> > On Tue, 2014-03-25 at 18:01 +0000, Julien Grall wrote:
> >> Hi Ian,
> >>
> >> On 03/25/2014 05:18 PM, Ian Campbell wrote:
> >>> On Tue, 2014-03-25 at 16:55 +0000, Julien Grall wrote:
> >>>> Clang doesn't support named register. Introduce __builtin_stack_pointer
> >>>> to be able to use named register with gcc.
> >>>
> >>> I think gcc considers the __builtin namespace to be its own, not sure
> >>> about clang.
> >>
> >> I'm able to compile on GCC without any issue.
> >
> > That's not the point. The point of a namespacing rule is that gcc can
> > add such a function at any time it likes, and if that breaks your
> > application then that is tough luck.
> 
> Thanks, I didn't find anything useful on internet about namespace. It 
> seems the common rule is __foo is reserved by the compiler.

Yes, I think that comes from the C standard. odd numbers of  _ are for
the libc (by POSIX I think) and even numbers for the compiler (by the C
std).
diff mbox

Patch

diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
index 65c0cdf..0d4a8c0 100644
--- a/xen/include/asm-arm/current.h
+++ b/xen/include/asm-arm/current.h
@@ -25,10 +25,23 @@  struct cpu_info {
     unsigned int pad;
 };
 
+#ifdef __clang__
+#define __builtin_stack_pointer() ({                \
+    unsigned long current_sp;                       \
+    asm ("mov %0, sp" : "=r" (current_sp));         \
+    current_sp;                                     \
+})
+#else
+#define __builtin_stack_pointer() ({                \
+    register unsigned long current_sp asm ("sp");   \
+    current_sp;                                     \
+})
+#endif
+
 static inline struct cpu_info *get_cpu_info(void)
 {
-    register unsigned long sp asm ("sp");
-    return (struct cpu_info *)((sp & ~(STACK_SIZE - 1)) + STACK_SIZE - sizeof(struct cpu_info));
+    return (struct cpu_info *)((__builtin_stack_pointer() & ~(STACK_SIZE - 1)) +
+                               STACK_SIZE - sizeof(struct cpu_info));
 }
 
 #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)