[LLVMdev] A new builtin: __builtin_stack_pointer()

[Behan Webster]

One of the issues the LLVMLinux project is having is with the use of
named registers in the Linux kernel code. The kernel uses something like
this in order to assign a C variable name to a register (one for each
kernel arch).

register unsigned long current_stack_pointer asm("esp");

clang doesn't allow this kind of thing which required a patch which less
efficient:

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

This works for both gcc and clang, but often adds in 3 extra
instructions since you need to copy the stack pointer to another
register, but first that register needs to be saved to the stack and
then restored after the stackpointer has been used; inefficient.

Another way would be to introduce a new builtin in parallel to others
like __builtin_return_address(). Essentially adding a
__builtin_stack_pointer() which does what the above code does. The idea
would be to get something like this added to both clang and gcc in order
to make this work across compilers, and across arches.

It ends up being a trivial patch for clang (see below). We're still
looking for someone to help us on the gcc side.

The goal is to ideally make the kernel code work equally well with both
compilers (clang and gcc).

Thoughts?

Thanks to Mark Charlebois for writing the patch below.

Behan

--
Behan Webster
beh...@converseincode.com


diff --git a/include/clang/Basic/Builtins.def b/include/clang/Basic/Builtins.def
index 0a513ef..ca68f7e 100644
--- a/include/clang/Basic/Builtins.def
+++ b/include/clang/Basic/Builtins.def
@@ -437,6 +437,7 @@ BUILTIN(__builtin_strstr, "c*cC*cC*", "nF")
BUILTIN(__builtin_return_address, "v*IUi", "n")
BUILTIN(__builtin_extract_return_addr, "v*v*", "n")
BUILTIN(__builtin_frame_address, "v*IUi", "n")
+BUILTIN(__builtin_stack_pointer, "v*", "n")
BUILTIN(__builtin_flt_rounds, "i", "nc")
BUILTIN(__builtin_setjmp, "iv**", "j")
BUILTIN(__builtin_longjmp, "vv**i", "r")
diff --git a/lib/CodeGen/CGBuiltin.cpp b/lib/CodeGen/CGBuiltin.cpp
index d187678..f66f506 100644
--- a/lib/CodeGen/CGBuiltin.cpp
+++ b/lib/CodeGen/CGBuiltin.cpp
@@ -736,6 +736,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
Value *F = CGM.getIntrinsic(Intrinsic::frameaddress);
return RValue::get(Builder.CreateCall(F, Depth));
}
+ case Builtin::BI__builtin_stack_pointer: {
+ Value *StackAddr =
+ Builder.CreateCall(CGM.getIntrinsic(Intrinsic::stacksave));
+ return RValue::get(StackAddr);
+ }
case Builtin::BI__builtin_extract_return_addr: {
Value *Address = EmitScalarExpr(E->getArg(0));
Value *Result = getTargetHooks().decodeReturnAddress(*this, Address);

_______________________________________________
LLVM Developers mailing list
LLV...@cs.uiuc.edu http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

[Jakob Stoklund Olesen]

On Oct 10, 2013, at 12:32 PM, Behan Webster <beh...@converseincode.com> wrote:

> One of the issues the LLVMLinux project is having is with the use of
> named registers in the Linux kernel code. The kernel uses something like
> this in order to assign a C variable name to a register (one for each
> kernel arch).
>
> register unsigned long current_stack_pointer asm("esp");
>
> clang doesn't allow this kind of thing which required a patch which less
> efficient:
>
> #define current_stack_pointer ({ \
> unsigned long esp; \
> asm("mov %%esp, %0" : "=r"(esp)); \
> esp; \
> })
>
> This works for both gcc and clang, but often adds in 3 extra
> instructions since you need to copy the stack pointer to another
> register, but first that register needs to be saved to the stack and
> then restored after the stackpointer has been used; inefficient.

#define current_stack_pointer ({ \
register unsigned long esp asm("esp"); \
asm("" : "=r"(esp)); \
esp; \
})

/jakob

[Behan Webster]

On 10/10/13 22:01, Jakob Stoklund Olesen wrote:
> On Oct 10, 2013, at 12:32 PM, Behan Webster <beh...@converseincode.com> wrote:
>
>> One of the issues the LLVMLinux project is having is with the use of
>> named registers in the Linux kernel code. The kernel uses something like
>> this in order to assign a C variable name to a register (one for each
>> kernel arch).
>>
>> register unsigned long current_stack_pointer asm("esp");
>>
>> clang doesn't allow this kind of thing which required a patch which less
>> efficient:
>>
>> #define current_stack_pointer ({ \
>> unsigned long esp; \
>> asm("mov %%esp, %0" : "=r"(esp)); \
>> esp; \
>> })
>>
>> This works for both gcc and clang, but often adds in 3 extra
>> instructions since you need to copy the stack pointer to another
>> register, but first that register needs to be saved to the stack and
>> then restored after the stackpointer has been used; inefficient.
> #define current_stack_pointer ({ \
> register unsigned long esp asm("esp"); \
> asm("" : "=r"(esp)); \
> esp; \
> })
>
> /jakob

That seems to work! Though I'm still testing it in all our situations
for LLVMLinux. It looks like I can get this to work with LLVM 3.3 and
the LLVMLinux kernel patches. Thanks!

However, I'd still like to see __builtin_stack_pointer() added since it
is simple, obvious, and mirrors other builtins like
__builtin_frame_address().

Behan

--
Behan Webster
beh...@converseincode.com

[Alp Toker]

Ping on this?

It seems like a legitimate builtin to have in clang, if only for
compatibility and the nicer IR representation.

Alp.

>
> Behan
>

--
http://www.nuanti.com
the browser experts

[Behan Webster]

Sorry. Been away from my desk for 2 weeks (Linux Con, Embedded Linux
Con, and a week teaching). Just catching up this week.

The suggested work around DOES work for many situations, but is ugly.
However it allows the LLVMLinux patches to work with clang/LLVM 3.3
right now which is important. But I'd love something better for LLVM 3.4

> It seems like a legitimate builtin to have in clang, if only for
> compatibility and the nicer IR representation.

Personally, I'd still like to see __builtin_stack_pointer() added to
clang/LLVM (and gcc too for that matter, but that's still a work in
progress). It would be very useful for our efforts in getting generic
code to work in the Linux kernel.

Thanks,

Behan

--
Behan Webster
beh...@converseincode.com

[Joerg Sonnenberger]

On Tue, Nov 05, 2013 at 08:49:09AM -0800, Behan Webster wrote:
> Personally, I'd still like to see __builtin_stack_pointer() added to
> clang/LLVM (and gcc too for that matter, but that's still a work in
> progress). It would be very useful for our efforts in getting generic
> code to work in the Linux kernel.

What is the desired behavior in the face of inlining?

Joerg

[Konstantin Tokarev]

11.10.2013, 01:39, "Jakob Stoklund Olesen" <stok...@2pi.dk>:

> On Oct 10, 2013, at 12:32 PM, Behan Webster <beh...@converseincode.com> wrote:
>
>>  One of the issues the LLVMLinux project is having is with the use of
>>  named registers in the Linux kernel code. The kernel uses something like
>>  this in order to assign a C variable name to a register (one for each
>>  kernel arch).
>>
>>     register unsigned long current_stack_pointer asm("esp");
>>
>>  clang doesn't allow this kind of thing which required a patch which less
>>  efficient:
>>
>>  #define current_stack_pointer ({ \
>>        unsigned long esp; \
>>        asm("mov %%esp, %0" : "=r"(esp)); \
>>        esp; \
>>  })
>>
>>  This works for both gcc and clang, but often adds in 3 extra
>>  instructions since you need to copy the stack pointer to another
>>  register, but first that register needs to be saved to the stack and
>>  then restored after the stackpointer has been used; inefficient.
>
> #define current_stack_pointer ({ \
>         register unsigned long esp asm("esp"); \
>         asm("" : "=r"(esp)); \
>         esp; \
>     })

And #ifdef it for each arch? Builtin would be much more handy.

--
Regards,
Konstantin

[Behan Webster]

Actually no #ifdef is required with Jakob's suggestion (works the same
in both clang and gcc).

However the builtin is indeed nicer, easier to read and much more in
keeping with the other builtins...

Behan

--
Behan Webster
beh...@converseincode.com

[Jean-Daniel Dupas]

It may works for both GCC and clang, it does not means it will works with every arch linux supports (ARM, PPC, AArch, …)

However the builtin is indeed nicer, easier to read and much more in
keeping with the other builtins...

Behan

- Jean-Daniel

[Behan Webster]

On 11/05/13 11:30, Jean-Daniel Dupas wrote:

Le 5 nov. 2013 � 19:00, Behan Webster <beh...@converseincode.com> a �crit :

On 11/05/13 09:26, Konstantin Tokarev wrote:

11.10.2013, 01:39, "Jakob Stoklund Olesen" <stok...@2pi.dk>:

On Oct 10, 2013, at 12:32 PM, Behan Webster <beh...@converseincode.com> wrote:

One of the issues the LLVMLinux project is having is with the use of
named registers in the Linux kernel code. The kernel uses something like
this in order to assign a C variable name to a register (one for each
kernel arch).

���register unsigned long current_stack_pointer asm("esp");

clang doesn't allow this kind of thing which required a patch which less
efficient:

#define current_stack_pointer ({ \

������unsigned long esp; \
������asm("mov %%esp, %0" : "=r"(esp)); \
������esp; \

})

This works for both gcc and clang, but often adds in 3 extra
instructions since you need to copy the stack pointer to another
register, but first that register needs to be saved to the stack and
then restored after the stackpointer has been used; inefficient.

#define current_stack_pointer ({ \
�������register unsigned long esp asm("esp"); \
�������asm("" : "=r"(esp)); \
�������esp; \
���})

And #ifdef it for each arch? Builtin would be much more handy.

Actually no #ifdef is required with Jakob's suggestion (works the same
in both clang and gcc).

It may works for both GCC and clang, it does not means it will works with every arch linux supports (ARM, PPC, AArch, �)

True enough. Which is indeed another argument for __builtin_stack_pointer().

That point is precisely one of the reasons that we're pushing for this addition.

[Renato Golin]

Hi Behan,

Only saw this thread now. I think having the built-in makes sense, but you sent the description and patch to the LLVM list, not the Clang list. ;)

I'd copy the cfe-dev, but I think now the thread is more complex than cross-posting would allow. So, maybe it'd be better if you posted there again, your original email, the patch and some of the comments on this thread.

cheers,

--renato

On 5 November 2013 16:15, Behan Webster <beh...@converseincode.com> wrote:

On 11/05/13 11:30, Jean-Daniel Dupas wrote:

Le 5 nov. 2013 à 19:00, Behan Webster <beh...@converseincode.com> a écrit :

On 11/05/13 09:26, Konstantin Tokarev wrote:

11.10.2013, 01:39, "Jakob Stoklund Olesen" <stok...@2pi.dk>:

On Oct 10, 2013, at 12:32 PM, Behan Webster <beh...@converseincode.com> wrote:

One of the issues the LLVMLinux project is having is with the use of
named registers in the Linux kernel code. The kernel uses something like
this in order to assign a C variable name to a register (one for each
kernel arch).

   register unsigned long current_stack_pointer asm("esp");

clang doesn't allow this kind of thing which required a patch which less
efficient:

#define current_stack_pointer ({ \

      unsigned long esp; \

      asm("mov %%esp, %0" : "=r"(esp)); \

      esp; \
})

This works for both gcc and clang, but often adds in 3 extra
instructions since you need to copy the stack pointer to another
register, but first that register needs to be saved to the stack and
then restored after the stackpointer has been used; inefficient.

#define current_stack_pointer ({ \

       register unsigned long esp asm("esp"); \

       asm("" : "=r"(esp)); \
       esp; \
   })

And #ifdef it for each arch? Builtin would be much more handy.

Actually no #ifdef is required with Jakob's suggestion (works the same
in both clang and gcc).

It may works for both GCC and clang, it does not means it will works with every arch linux supports (ARM, PPC, AArch, …)

True enough. Which is indeed another argument for __builtin_stack_pointer().

That point is precisely one of the reasons that we're pushing for this addition.

Behan

-- 
Behan Webster
beh...@converseincode.com