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

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

[Pekka Enberg]

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

On Tue, Aug 30, 2011 at 10:43 AM, Jeff Garzik <jeff@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@kernel.crashing.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Jeff Garzik <jgarzik@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
[ penberg@kernel.org: Fix validation/context.c breakage. ]
Signed-off-by: Pekka Enberg <penberg@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);
 	pseudo_t retval, call;
 	struct ctype *ctype = NULL;
+	struct symbol *fntype;
 	struct context *context;
 
 	if (!expr->ctype) {
@@ -1212,6 +1213,13 @@ static pseudo_t linearize_call_expression(struct entrypoint *ep, struct expressi
 	if (fn->ctype)
 		ctype = &fn->ctype->ctype;
 
+	fntype = fn->ctype;
+	if (fntype) {
+		if (fntype->type == SYM_NODE)
+			fntype = fntype->ctype.base_type;
+	}
+	insn->fntype = fntype;
+
 	if (fn->type == EXPR_PREOP) {
 		if (fn->unop->type == EXPR_SYMBOL) {
 			struct symbol *sym = fn->unop->symbol;
diff --git a/linearize.h b/linearize.h
index 424ba97..61fbd83 100644
--- a/linearize.h
+++ b/linearize.h
@@ -116,6 +116,7 @@ struct instruction {
 		struct /* call */ {
 			pseudo_t func;
 			struct pseudo_list *arguments;
+			struct symbol *fntype;
 		};
 		struct /* context */ {
 			int increment;
-- 
1.7.6.4

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

[Pekka Enberg]

From: Benjamin Herrenschmidt <benh@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@chrisli.org>
Cc: Jeff Garzik <jgarzik@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[ penberg@kernel.org: Fix function pointer calls ]
Signed-off-by: Pekka Enberg <penberg@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);
 
 	func_type = LLVMFunctionType(ret_type, arg_type, n_arg,
-				     /* varargs? */ 0);
+				     insn->fntype->variadic);
 
 	return func_type;
 }
-- 
1.7.6.4

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

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

[Benjamin Herrenschmidt]

On Wed, 2012-02-01 at 11:55 +0200, Pekka Enberg wrote:
> From: Benjamin Herrenschmidt <benh@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@chrisli.org>
> Cc: Jeff Garzik <jgarzik@redhat.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> [ penberg@kernel.org: Fix function pointer calls ]
> Signed-off-by: Pekka Enberg <penberg@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;
>  }

Is the above hunk correct ? It was really just a guess, I haven't tested
that code path :-)

From my understanding (please correct me if I'm wrong) this is used to
generate a type for a function pointer inside a structure definition or
an argument list... so I -assume- base_type will work but still only
have a very blurry version of the big picture in mind...

(And no time to do more than answer email on that subject today)

> @@ -744,7 +744,7 @@ static LLVMTypeRef get_func_type(struct function *fn, struct instruction *insn)
>  	} END_FOR_EACH_PTR(arg);
>  
>  	func_type = LLVMFunctionType(ret_type, arg_type, n_arg,
> -				     /* varargs? */ 0);
> +				     insn->fntype->variadic);
>  
>  	return func_type;
>  }

Cheers,
Ben.

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

[Jeff Garzik]

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@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@chrisli.org>
>> Cc: Jeff Garzik<jgarzik@redhat.com>
>> Cc: Linus Torvalds<torvalds@linux-foundation.org>
>> Signed-off-by: Benjamin Herrenschmidt<benh@kernel.crashing.org>
>> [ penberg@kernel.org: Fix function pointer calls ]
>> Signed-off-by: Pekka Enberg<penberg@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;
>>   }
>
> Is the above hunk correct ? It was really just a guess, I haven't tested
> that code path :-)

I was wondering that, myself :)  Need coffee, then testing, I suppose :)

	Jeff

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

[Christopher Li]

On Wed, Feb 1, 2012 at 1:55 AM, Pekka Enberg <penberg@kernel.org> wrote:
> From: Linus Torvalds <torvalds@linux-foundation.org>
>
> On Tue, Aug 30, 2011 at 10:43 AM, Jeff Garzik <jeff@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;
     ....

}

Now think about it for a little bit, instead of this patch, how about
I make a counter RFC:

Move the insn->type into pseudo->type. Then you can use use
pseudo->type to access
the type of any pseudo. I estimate the memory usage should be similar,
if not smaller.
It make the pesudo struct larger, however, it make the insn struct
smaller. There is more
instruction than pesudo register in general, because not all
instruction resulting an output
pesudo register. Even consider some pseudo does not generate from
instructions, it is
likely a saving in memory foot print overall.

The plus side, you can have easy access of type for all pseudo. We
might able to get
rid of insn->size as well. Over all I feel it is cleaner.

Suggestions and feed back please?

Chris

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

[Benjamin Herrenschmidt]

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.

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

[Pekka Enberg]

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.

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

[Jeff Garzik]

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

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

[Benjamin Herrenschmidt]

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:

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.

Cheers,
Ben.

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

[Christopher Li]

On Wed, Feb 1, 2012 at 5:22 PM, Benjamin Herrenschmidt
<benh@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
expressions (function pointers).

Chris

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

[Benjamin Herrenschmidt]

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.

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

[Christopher Li]

On Wed, Feb 1, 2012 at 5:50 PM, Benjamin Herrenschmidt
<benh@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

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

[Pekka Enberg]

On Thu, Feb 2, 2012 at 4:10 AM, Christopher Li <sparse@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

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

[Benjamin Herrenschmidt]

On Fri, 2012-02-03 at 11:09 +0200, Pekka Enberg wrote:
> On Thu, Feb 2, 2012 at 4:10 AM, Christopher Li <sparse@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.
 

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

[Christopher Li]

On Fri, Feb 3, 2012 at 1:09 AM, Pekka Enberg <penberg@kernel.org> wrote:
> On Thu, Feb 2, 2012 at 4:10 AM, Christopher Li <sparse@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

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

[Pekka Enberg]

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

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

[Linus Torvalds]

On Sat, Feb 4, 2012 at 4:56 AM, Pekka Enberg <penberg@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

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

[Pekka Enberg]

On Sat, Feb 4, 2012 at 4:56 AM, Pekka Enberg <penberg@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

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

[Christopher Li]

On Wed, Feb 1, 2012 at 5:03 PM, Jeff Garzik <jeff@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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html