sparse, llvm: Make function declaration accessible to backend

From: Linus Torvalds <[email protected]>

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.

---
 linearize.c |    8 ++++++++
 linearize.h |    1 +
 2 files changed, 9 insertions(+), 0 deletions(-)

index 1899978..7d57474 100644
@@ -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;

index 424ba97..61fbd83 100644
@@ -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

From: Benjamin Herrenschmidt <[email protected]>

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

---
 sparse-llvm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

index a291a0d..9226a21 100644
@@ -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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Jeff

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

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.

Agreed. We already do this for LLVMValueRefs which are stored in ->priv field of pseudo_t.

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

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.

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.

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

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.

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.

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

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

Pekka

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.

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

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

Pekka

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

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

Pekka

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html