Patchwork [1/5] arm: LLVMLinux: Add current_stack_pointer macro for ARM

login
register
mail settings
Submitter Behan Webster
Date Sept. 6, 2013, 9:28 p.m.
Message ID <1378502899-1241-2-git-send-email-behanw@converseincode.com>
Download mbox | patch
Permalink /patch/273340/
State New
Headers show

Comments

Behan Webster - Sept. 6, 2013, 9:28 p.m.
From: Behan Webster <behanw@converseincode.com>

A macro to get the current stack pointer which allows for a single place in
which to do so with ASM. Before this named registers (a gcc extension) was used
to get the stack pointer. Using ASM is a more portable way of getting the stack
pointer which works with both gcc and clang.  This macro is of the same name
used in the X86 arch.

Author: Behan Webster <behanw@converseincode.com>
Signed-off-by: Behan Webster <behanw@converseincode.com>
Reviewed-by: Jan-Simon Möller <dl9pf@gmx.de>
---
 arch/arm/include/asm/thread_info.h | 9 +++++++++
 1 file changed, 9 insertions(+)
Måns Rullgård - Sept. 6, 2013, 10:12 p.m.
behanw@converseincode.com writes:

> +#define current_stack_pointer ({ \
> +	unsigned long current_sp; \
> +	asm ("mov %0, r13" : "=r" (current_sp)); \
> +	current_sp; \
> +})

Why do you use 'r13' rather than the more common 'sp' alias?
Russell King - ARM Linux - Sept. 6, 2013, 10:20 p.m.
On Fri, Sep 06, 2013 at 05:28:07PM -0400, behanw@converseincode.com wrote:
> From: Behan Webster <behanw@converseincode.com>
> 
> A macro to get the current stack pointer which allows for a single place in
> which to do so with ASM. Before this named registers (a gcc extension) was used
> to get the stack pointer. Using ASM is a more portable way of getting the stack
> pointer which works with both gcc and clang.  This macro is of the same name
> used in the X86 arch.

This will result in less optimal code - rather than the compiler being
able to mask directly with 'sp', it's going to have to use this bit of
assembly to first move it into another register.

Why do we want this change?
Behan Webster - Sept. 6, 2013, 10:50 p.m.
On 09/06/13 18:12, Måns Rullgård wrote:
> behanw@converseincode.com writes:
>
>> +#define current_stack_pointer ({ \
>> +	unsigned long current_sp; \
>> +	asm ("mov %0, r13" : "=r" (current_sp)); \
>> +	current_sp; \
>> +})
> Why do you use 'r13' rather than the more common 'sp' alias?
Originally we were using LLVM's Integrated Assembler (IA), which didn't 
allow for that alias if I remember correctly. However, now we're using 
gas because IA only supports Unified Assembly Language grammar, and not 
the extensions that are common in the kernel code.

I can resubmit using that alias (after the rest of the discussion).

Behan
Behan Webster - Sept. 6, 2013, 10:54 p.m.
On 09/06/13 18:20, Russell King - ARM Linux wrote:
> On Fri, Sep 06, 2013 at 05:28:07PM -0400, behanw@converseincode.com wrote:
>> From: Behan Webster <behanw@converseincode.com>
>>
>> A macro to get the current stack pointer which allows for a single place in
>> which to do so with ASM. Before this named registers (a gcc extension) was used
>> to get the stack pointer. Using ASM is a more portable way of getting the stack
>> pointer which works with both gcc and clang.  This macro is of the same name
>> used in the X86 arch.
> This will result in less optimal code - rather than the compiler being
> able to mask directly with 'sp', it's going to have to use this bit of
> assembly to first move it into another register.
I understand. The issue is that clang doesn't support naming registers 
like this. It's a gcc-ism.

I'm not entirely happy with this solution either, but it was what we 
could get to work for both compilers without the use of ifdefs.

Though also not ideal, how about an #ifdef for clang to do it this way, 
otherwise do it with named registers for gcc? Would that be acceptable?

Thanks,

Behan

Patch

diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
index df5e13d..94283f8 100644
--- a/arch/arm/include/asm/thread_info.h
+++ b/arch/arm/include/asm/thread_info.h
@@ -100,6 +100,15 @@  struct thread_info {
 #define init_stack		(init_thread_union.stack)
 
 /*
+ * how to get the current stack pointer from C
+ */
+#define current_stack_pointer ({ \
+	unsigned long current_sp; \
+	asm ("mov %0, r13" : "=r" (current_sp)); \
+	current_sp; \
+})
+
+/*
  * how to get the thread information struct from C
  */
 static inline struct thread_info *current_thread_info(void) __attribute_const__;