Pekka Enberg | 1 Feb 10:55
Favicon
Gravatar

[RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

From: Linus Torvalds <torvalds <at> linux-foundation.org>

On Tue, Aug 30, 2011 at 10:43 AM, Jeff Garzik <jeff <at> garzik.org> wrote:
> * if someone knows how to access a function declaration, I can solve the
> varargs problem

Hmm. Right now we do not have access to the function declaration at
linearize time. We've checked that the arguments match, and we've cast
the arguments to the right types (evaluate.c), so the thinking was
that you just use the arguments as-is.

But if llvm needs the declaration of a function, we'd need to squirrel it away.

Cc: Benjamin Herrenschmidt <benh <at> kernel.crashing.org>
Cc: Christopher Li <sparse <at> chrisli.org>
Cc: Jeff Garzik <jgarzik <at> redhat.com>
Cc: Linus Torvalds <torvalds <at> linux-foundation.org>
[ penberg <at> kernel.org: Fix validation/context.c breakage. ]
Signed-off-by: Pekka Enberg <penberg <at> kernel.org>
---
 linearize.c |    8 ++++++++
 linearize.h |    1 +
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/linearize.c b/linearize.c
index 1899978..7d57474 100644
--- a/linearize.c
+++ b/linearize.c
@@ -1195,6 +1195,7 @@ static pseudo_t linearize_call_expression(struct entrypoint *ep, struct expressi
 	struct instruction *insn = alloc_typed_instruction(OP_CALL, expr->ctype);
(Continue reading)

Pekka Enberg | 1 Feb 10:55
Favicon
Gravatar

[RFC/PATCH 2/2] sparse, llvm: Fix varargs functions

From: Benjamin Herrenschmidt <benh <at> kernel.crashing.org>

We need to tell llvm about it or it won't generate the proper
stack frame & argument list on some architectures.

Cc: Christopher Li <sparse <at> chrisli.org>
Cc: Jeff Garzik <jgarzik <at> redhat.com>
Cc: Linus Torvalds <torvalds <at> linux-foundation.org>
Signed-off-by: Benjamin Herrenschmidt <benh <at> kernel.crashing.org>
[ penberg <at> kernel.org: Fix function pointer calls ]
Signed-off-by: Pekka Enberg <penberg <at> kernel.org>
---
 sparse-llvm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sparse-llvm.c b/sparse-llvm.c
index a291a0d..9226a21 100644
--- a/sparse-llvm.c
+++ b/sparse-llvm.c
@@ -79,7 +79,7 @@ static LLVMTypeRef sym_func_type(LLVMModuleRef module, struct symbol *sym)
 		arg_type[idx++] = symbol_type(module, arg_sym);
 	} END_FOR_EACH_PTR(arg);
 	func_type = LLVMFunctionType(ret_type, arg_type, n_arg,
-				     /* varargs? */ 0);
+				     sym->ctype.base_type->variadic);

 	return func_type;
 }
@@ -744,7 +744,7 @@ static LLVMTypeRef get_func_type(struct function *fn, struct instruction *insn)
 	} END_FOR_EACH_PTR(arg);
(Continue reading)

Re: [RFC/PATCH 2/2] sparse, llvm: Fix varargs functions

On Wed, 2012-02-01 at 11:55 +0200, Pekka Enberg wrote:
> From: Benjamin Herrenschmidt <benh <at> kernel.crashing.org>
> 
> We need to tell llvm about it or it won't generate the proper
> stack frame & argument list on some architectures.
> 
> Cc: Christopher Li <sparse <at> chrisli.org>
> Cc: Jeff Garzik <jgarzik <at> redhat.com>
> Cc: Linus Torvalds <torvalds <at> linux-foundation.org>
> Signed-off-by: Benjamin Herrenschmidt <benh <at> kernel.crashing.org>
> [ penberg <at> kernel.org: Fix function pointer calls ]
> Signed-off-by: Pekka Enberg <penberg <at> kernel.org>
> ---
>  sparse-llvm.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sparse-llvm.c b/sparse-llvm.c
> index a291a0d..9226a21 100644
> --- a/sparse-llvm.c
> +++ b/sparse-llvm.c
> @@ -79,7 +79,7 @@ static LLVMTypeRef sym_func_type(LLVMModuleRef module, struct symbol *sym)
>  		arg_type[idx++] = symbol_type(module, arg_sym);
>  	} END_FOR_EACH_PTR(arg);
>  	func_type = LLVMFunctionType(ret_type, arg_type, n_arg,
> -				     /* varargs? */ 0);
> +				     sym->ctype.base_type->variadic);
>  
>  	return func_type;
>  }

(Continue reading)

Jeff Garzik | 1 Feb 14:58
Favicon

Re: [RFC/PATCH 2/2] sparse, llvm: Fix varargs functions

On 02/01/2012 05:47 AM, Benjamin Herrenschmidt wrote:
> On Wed, 2012-02-01 at 11:55 +0200, Pekka Enberg wrote:
>> From: Benjamin Herrenschmidt<benh <at> kernel.crashing.org>
>>
>> We need to tell llvm about it or it won't generate the proper
>> stack frame&  argument list on some architectures.
>>
>> Cc: Christopher Li<sparse <at> chrisli.org>
>> Cc: Jeff Garzik<jgarzik <at> redhat.com>
>> Cc: Linus Torvalds<torvalds <at> linux-foundation.org>
>> Signed-off-by: Benjamin Herrenschmidt<benh <at> kernel.crashing.org>
>> [ penberg <at> kernel.org: Fix function pointer calls ]
>> Signed-off-by: Pekka Enberg<penberg <at> kernel.org>
>> ---
>>   sparse-llvm.c |    4 ++--
>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/sparse-llvm.c b/sparse-llvm.c
>> index a291a0d..9226a21 100644
>> --- a/sparse-llvm.c
>> +++ b/sparse-llvm.c
>> @@ -79,7 +79,7 @@ static LLVMTypeRef sym_func_type(LLVMModuleRef module, struct symbol *sym)
>>   		arg_type[idx++] = symbol_type(module, arg_sym);
>>   	} END_FOR_EACH_PTR(arg);
>>   	func_type = LLVMFunctionType(ret_type, arg_type, n_arg,
>> -				     /* varargs? */ 0);
>> +				     sym->ctype.base_type->variadic);
>>
>>   	return func_type;
>>   }
(Continue reading)

Christopher Li | 2 Feb 01:09

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Wed, Feb 1, 2012 at 1:55 AM, Pekka Enberg <penberg <at> kernel.org> wrote:
> From: Linus Torvalds <torvalds <at> linux-foundation.org>
>
> On Tue, Aug 30, 2011 at 10:43 AM, Jeff Garzik <jeff <at> garzik.org> wrote:
>> * if someone knows how to access a function declaration, I can solve the
>> varargs problem
>
> Hmm. Right now we do not have access to the function declaration at
> linearize time. We've checked that the arguments match, and we've cast
> the arguments to the right types (evaluate.c), so the thinking was
> that you just use the arguments as-is.

Ok, this patch definitely works. However, I think there is ways to get the
type without this patch. The more general question is, how to get type of
any given pseudo register. Currently it is a bit twisted but that information
should be accessible. I believe that has been discuss on the mail list
before. That is why we have insn->type. LLVM want to know every type
of every value in the back end any way. If we are going to do proper
llvm style "get element pointer" in the back end, we need to access type
of every pesudo register.

The current twisted way to get type from the pseudo register is some thing like
this (totally untested code):

swtich (pseudo->type) {
     case PSEDUO_REG:
          return pseudo->def->type;
     case PSEUDO_ARG:
     case PSEUDO_SYMBOL:
          return psuedo->symbol;
(Continue reading)

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Wed, 2012-02-01 at 16:09 -0800, Christopher Li wrote:
> 
> Ok, this patch definitely works. However, I think there is ways to get the
> type without this patch. The more general question is, how to get type of
> any given pseudo register. Currently it is a bit twisted but that information
> should be accessible. I believe that has been discuss on the mail list
> before. That is why we have insn->type. LLVM want to know every type
> of every value in the back end any way. If we are going to do proper
> llvm style "get element pointer" in the back end, we need to access type
> of every pesudo register.

We also should probably store the LLVM Ref of the type once "converted"
into the original struct symbol so that we don't have to re-create it or
all the time (thinking especially of typedef'ed function pointers) or
search for it by name (functions).

Cheers,
Ben.

--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Pekka Enberg | 2 Feb 07:28
Favicon
Gravatar

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Thu, 2 Feb 2012, Benjamin Herrenschmidt wrote:
> We also should probably store the LLVM Ref of the type once "converted"
> into the original struct symbol so that we don't have to re-create it or
> all the time (thinking especially of typedef'ed function pointers) or
> search for it by name (functions).

Agreed. We already do this for LLVMValueRefs which are stored in ->priv 
field of pseudo_t.
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jeff Garzik | 2 Feb 02:03
Favicon

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On 02/01/2012 07:09 PM, Christopher Li wrote:
> Ok, this patch definitely works. However, I think there is ways to get the
> type without this patch. The more general question is, how to get type of
> any given pseudo register.

That is useful, yes.  But it does not address this specific problem.

We need the function declaration remembered, rather than what we have 
now -- a list of arguments with full type information, specific to its 
callsite.

You cannot deduce that a function call is/not varargs presently, even 
with a working pseudo->type setup.

varargs is just one of those annoying areas where the compiler needs to 
have rather specific knowledge, in order to properly construct a call

	Jeff

--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Wed, 2012-02-01 at 20:03 -0500, Jeff Garzik wrote:
> On 02/01/2012 07:09 PM, Christopher Li wrote:
> > Ok, this patch definitely works. However, I think there is ways to get the
> > type without this patch. The more general question is, how to get type of
> > any given pseudo register.
> 
> 
> That is useful, yes.  But it does not address this specific problem.
> 
> We need the function declaration remembered, rather than what we have 
> now -- a list of arguments with full type information, specific to its 
> callsite.
> 
> You cannot deduce that a function call is/not varargs presently, even 
> with a working pseudo->type setup.
> 
> varargs is just one of those annoying areas where the compiler needs to 
> have rather specific knowledge, in order to properly construct a call

And there could be more. For example specific attributes on the
declaration may affect the ABI for the call (think asmlinkage or good
old pascal calling conventions on old macos :-)

Not something we absolutely need to sort out right now but another
reason why we really need to base the LLVM side definition based on the
declaration.

I'll try to toy a bit more this week-end see if the patches work for all
cases I can think of. We really have two different things (represented
by the two different hunks) which we might try to better factor:
(Continue reading)

Christopher Li | 2 Feb 02:33

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Wed, Feb 1, 2012 at 5:22 PM, Benjamin Herrenschmidt
<benh <at> kernel.crashing.org> wrote:
> And there could be more. For example specific attributes on the
> declaration may affect the ABI for the call (think asmlinkage or good
> old pascal calling conventions on old macos :-)
>
> Not something we absolutely need to sort out right now but another
> reason why we really need to base the LLVM side definition based on the
> declaration.

Yes, that is what I am talking about too. Notice that insn->func is a pseudo
not a symbol. Pseudo can be result from another expression. That is my
suggestion in the counter RFC, put the full symbol type into pseudo->type.
You should have access of that information.

>
> I'll try to toy a bit more this week-end see if the patches work for all
> cases I can think of. We really have two different things (represented
> by the two different hunks) which we might try to better factor:
>
> The case of a function call where we are after the declaration, and the
> case of creating an llvm type containing a function pointer (which can
> happen as part of the first one if an argument is a function pointer)
> for which we are looking at the base_type.
>
> We should at least make it a single piece of code that takes a symbol
> and shoots out a llvm ref.

Do you mean take a pseudo then shoot out a llvm ref? The pseudo is
the more general form, a pseudo can come from symbol node or
(Continue reading)

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Wed, 2012-02-01 at 17:33 -0800, Christopher Li wrote:
> 
> Do you mean take a pseudo then shoot out a llvm ref? The pseudo is
> the more general form, a pseudo can come from symbol node or
> expressions (function pointers). 

But in both cases the type itself is a struct symbol isn't it ? Either a
function def as Linus patch provides or base_type which is a struct
symbol too....

Sure if we have a consistent pseudo->type that covers both cases then we
can just pass that along, I don't mind.

Cheers,
Ben.

--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Christopher Li | 2 Feb 03:10

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Wed, Feb 1, 2012 at 5:50 PM, Benjamin Herrenschmidt
<benh <at> kernel.crashing.org> wrote:
> But in both cases the type itself is a struct symbol isn't it ? Either a
> function def as Linus patch provides or base_type which is a struct
> symbol too....

Yes, sparse type itself is "struct symbol" pointer.
I see. You mean construct a LLVM value from a give sparse type.
Sure, that is value point. I actually did that once before, in my llvm
hack attempt. I lost that code base due to a hard drive failure.

> Sure if we have a consistent pseudo->type that covers both cases then we
> can just pass that along, I don't mind.

Yes, that is the point my counter RFC. When you look at it, the
insn->fntype is really type of the insn->func pseudo. That is a one off
thing for call instruction. Store type inside pseudo provide the same
functionality
and unify how to get type from pseudo.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Pekka Enberg | 3 Feb 10:09
Favicon
Gravatar

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Thu, Feb 2, 2012 at 4:10 AM, Christopher Li <sparse <at> chrisli.org> wrote:
> Yes, that is the point my counter RFC. When you look at it, the
> insn->fntype is really type of the insn->func pseudo. That is a one off
> thing for call instruction. Store type inside pseudo provide the same
> functionality and unify how to get type from pseudo.

Ping? I'd really like to have this bug fixed because it affects basic
"hello, world" on x86-64 and PPC.

                        Pekka
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Fri, 2012-02-03 at 11:09 +0200, Pekka Enberg wrote:
> On Thu, Feb 2, 2012 at 4:10 AM, Christopher Li <sparse <at> chrisli.org> wrote:
> > Yes, that is the point my counter RFC. When you look at it, the
> > insn->fntype is really type of the insn->func pseudo. That is a one off
> > thing for call instruction. Store type inside pseudo provide the same
> > functionality and unify how to get type from pseudo.
> 
> Ping? I'd really like to have this bug fixed because it affects basic
> "hello, world" on x86-64 and PPC.

Yeah I don't know sparse well enough to have an informed preference of
one way vs. the other, so just pick one that works :-)

From there we can cache the llvm ref etc... which should fix a while
pile of problems and make things faster.

Cheers,
Ben.

--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Christopher Li | 4 Feb 13:20

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Fri, Feb 3, 2012 at 1:09 AM, Pekka Enberg <penberg <at> kernel.org> wrote:
> On Thu, Feb 2, 2012 at 4:10 AM, Christopher Li <sparse <at> chrisli.org> wrote:
>> Yes, that is the point my counter RFC. When you look at it, the
>> insn->fntype is really type of the insn->func pseudo. That is a one off
>> thing for call instruction. Store type inside pseudo provide the same
>> functionality and unify how to get type from pseudo.
>
> Ping? I'd really like to have this bug fixed because it affects basic
> "hello, world" on x86-64 and PPC.

Hi Pekka,

Sorry I get really spaced. Can you continue apply that into your sparse-llvm
repository? The insn->type to pseudo->ctype change is actually impact a
lot of code. I haven't able to complete it yet. On the other hand, I
don't want you
to block on it. The more I look at it, the more I believe this is the
right thing
to do. Pseudo come from expressions, the pseudo->ctype is just the expr->ctype.

We will merge the sparse-llvm again when I get this sort out. One good thing
about git is that branch and merge is really easy. I you to continue
the sparse-llvm
repository. I can submit some llvm related patch for you to review as well.
sparse-llvm.c is the biggest user of insn->type right now. I want to simplify
the usage a little bit.

Thanks

Chris
(Continue reading)

Pekka Enberg | 4 Feb 13:56
Favicon
Gravatar

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Sat, 4 Feb 2012, Christopher Li wrote:
> Sorry I get really spaced. Can you continue apply that into your sparse-llvm
> repository?

Of course. Linus are you OK with the changes I did to your patch? Can I 
have your signoff before I merge it?

 			Pekka
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Linus Torvalds | 4 Feb 16:27
Gravatar

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Sat, Feb 4, 2012 at 4:56 AM, Pekka Enberg <penberg <at> kernel.org> wrote:
>
> Of course. Linus are you OK with the changes I did to your patch? Can I have
> your signoff before I merge it?

Sure, just add my signed-off, that patch is fine (nor do I mind the
other approach that makes it a type of a pseudo)

                Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Pekka Enberg | 4 Feb 16:34
Favicon
Gravatar

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Sat, Feb 4, 2012 at 4:56 AM, Pekka Enberg <penberg <at> kernel.org> wrote:
>> Of course. Linus are you OK with the changes I did to your patch? Can I have
>> your signoff before I merge it?

On Sat, 4 Feb 2012, Linus Torvalds wrote:
> Sure, just add my signed-off, that patch is fine (nor do I mind the
> other approach that makes it a type of a pseudo)

Applied and pushed. Ben, "hello world" should work for you in master now.

 			Pekka
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Christopher Li | 2 Feb 02:22

Re: [RFC/PATCH 1/2] sparse, llvm: Make function declaration accessible to backend

On Wed, Feb 1, 2012 at 5:03 PM, Jeff Garzik <jeff <at> garzik.org> wrote:
> That is useful, yes.  But it does not address this specific problem.
>
> We need the function declaration remembered, rather than what we have now --
> a list of arguments with full type information, specific to its callsite.
>
> You cannot deduce that a function call is/not varargs presently, even with a
> working pseudo->type setup.

I am not trying do deduce function call/varargs from the call. I am
trying to get
it from call_insn->func->type directly. Similar to Linus' patch,
instead of store fntype
at call_insn->fntype, store it at call_insn->func->type directly. It should work
the similar way as Linus's patch, no?

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Gmane