diff mbox

[06/13] llvm: fix type of literal integer passed as arguments

Message ID CANeU7Qnoqg-rM2MS+3BuKEvsLUBvnv+rvfYmE=XkhMicdejN=A@mail.gmail.com (mailing list archive)
State Rejected, archived
Headers show

Commit Message

Christopher Li March 7, 2017, 3:33 p.m. UTC
On Sun, Mar 5, 2017 at 7:20 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Like for all others instructions, LLVM needs the type
> of each operands. However this information is not always
> available via the pseudo, like here when passing a integer
> constant as argument since for sparse constants are typeless.
>
> Fix this by getting the type via the function prototype.
>
> +               LLVMValueRef value;
> +               if (arg->type == PSEUDO_VAL) {
> +                       /* Value pseudos do not have type information. */
> +                       /* Use the function prototype to get the type. */
> +                       struct symbol *ctype = get_nth1_arg(insn->func->sym, i + 1);

I try to come up with an example to use the PREPARE_PTR_LIST() in this patch.
I hit a bug "insn->func->sym"  assume "insn->func" is a function symbol node.
If "insn->func" is a function pointer then access "insn->func->sym" is wrong.

Any way, my modify patch attached. It should work similar to this patch
without using the nth argument help function. My limited test hit this
function pointer bug.


Chris

Comments

Luc Van Oostenryck March 7, 2017, 4:21 p.m. UTC | #1
On Tue, Mar 07, 2017 at 11:33:19PM +0800, Christopher Li wrote:
> On Sun, Mar 5, 2017 at 7:20 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > Like for all others instructions, LLVM needs the type
> > of each operands. However this information is not always
> > available via the pseudo, like here when passing a integer
> > constant as argument since for sparse constants are typeless.
> >
> > Fix this by getting the type via the function prototype.
> >
> > +               LLVMValueRef value;
> > +               if (arg->type == PSEUDO_VAL) {
> > +                       /* Value pseudos do not have type information. */
> > +                       /* Use the function prototype to get the type. */
> > +                       struct symbol *ctype = get_nth1_arg(insn->func->sym, i + 1);
> 
> I try to come up with an example to use the PREPARE_PTR_LIST() in this patch.
> I hit a bug "insn->func->sym"  assume "insn->func" is a function symbol node.
> If "insn->func" is a function pointer then access "insn->func->sym" is wrong.

Mmmm yes, indeed.
 
> Any way, my modify patch attached. It should work similar to this patch
> without using the nth argument help function. My limited test hit this
> function pointer bug.

OK. 

-- Luc
--
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
Dibyendu Majumdar March 7, 2017, 7:41 p.m. UTC | #2
On 7 March 2017 at 15:33, Christopher Li <sparse@chrisli.org> wrote:
> On Sun, Mar 5, 2017 at 7:20 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> Like for all others instructions, LLVM needs the type
>> of each operands. However this information is not always
>> available via the pseudo, like here when passing a integer
>> constant as argument since for sparse constants are typeless.
>>
>> Fix this by getting the type via the function prototype.
>>
>> +               LLVMValueRef value;
>> +               if (arg->type == PSEUDO_VAL) {
>> +                       /* Value pseudos do not have type information. */
>> +                       /* Use the function prototype to get the type. */
>> +                       struct symbol *ctype = get_nth1_arg(insn->func->sym, i + 1);
>
> I try to come up with an example to use the PREPARE_PTR_LIST() in this patch.

I suppose that the function prototype may not have the same number of
declared parameters as the actual call arguments - e.g. if the
function is variadic. In that case there may not be corresponding
parameter definition available. I think that in this case default
argument promotion rules will need to be applied for trailing
arguments.

Regards
Dibyendu
--
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
Dibyendu Majumdar March 10, 2017, 4:08 p.m. UTC | #3
On 7 March 2017 at 19:41, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> On 7 March 2017 at 15:33, Christopher Li <sparse@chrisli.org> wrote:
>> On Sun, Mar 5, 2017 at 7:20 PM, Luc Van Oostenryck
>> <luc.vanoostenryck@gmail.com> wrote:
>>> Like for all others instructions, LLVM needs the type
>>> of each operands. However this information is not always
>>> available via the pseudo, like here when passing a integer
>>> constant as argument since for sparse constants are typeless.
>>>
>>> Fix this by getting the type via the function prototype.
>>>
>>> +               LLVMValueRef value;
>>> +               if (arg->type == PSEUDO_VAL) {
>>> +                       /* Value pseudos do not have type information. */
>>> +                       /* Use the function prototype to get the type. */
>>> +                       struct symbol *ctype = get_nth1_arg(insn->func->sym, i + 1);
>>
>> I try to come up with an example to use the PREPARE_PTR_LIST() in this patch.
>
> I suppose that the function prototype may not have the same number of
> declared parameters as the actual call arguments - e.g. if the
> function is variadic. In that case there may not be corresponding
> parameter definition available. I think that in this case default
> argument promotion rules will need to be applied for trailing
> arguments.
>

I have also hit a problem when calling a function via function
pointer. In this case the instruction->func appears to be PSEUDO_REG
rather than PSEUDO_SYM, so trying to get the function prototype
doesn't work.

If the call is a via a function pointer then where should one look for
what the argument type should be when a constant is encountered?

Regards
Dibyendu
--
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
Luc Van Oostenryck March 10, 2017, 5:47 p.m. UTC | #4
On Fri, Mar 10, 2017 at 5:08 PM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
> I have also hit a problem when calling a function via function
> pointer. In this case the instruction->func appears to be PSEUDO_REG
> rather than PSEUDO_SYM, so trying to get the function prototype
> doesn't work.
>
> If the call is a via a function pointer then where should one look for
> what the argument type should be when a constant is encountered?

I think I have all such cases fixed.
I'll send an updated patch serie later today or tomorrow.

Luc
--
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
diff mbox

Patch

From patchwork Sun Mar  5 11:20:40 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [06/13] llvm: fix type of literal integer passed as arguments
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
X-Patchwork-Id: 9604557
Message-Id: <20170305112047.3411-7-luc.vanoostenryck@gmail.com>
To: linux-sparse@vger.kernel.org
Cc: Dibyendu Majumdar <mobile@majumdar.org.uk>,
 Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Date: Sun,  5 Mar 2017 12:20:40 +0100

Like for all others instructions, LLVM needs the type
of each operands. However this information is not always
available via the pseudo, like here when passing a integer
constant as argument since for sparse constants are typeless.

Fix this by getting the type via the function prototype.

Reported-by: Dibyendu Majumdar <mobile@majumdar.org.uk>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 sparse-llvm.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)



--- sparse.chrisl.orig/sparse-llvm.c
+++ sparse.chrisl/sparse-llvm.c
@@ -729,6 +729,8 @@  static void output_op_call(struct functi
 {
 	LLVMValueRef target, func;
 	int n_arg = 0, i;
+	struct symbol_list *argument_types = insn->func->sym->ctype.base_type->arguments;
+	struct symbol *argtype;
 	struct pseudo *arg;
 	LLVMValueRef *args;
 
@@ -739,9 +741,20 @@  static void output_op_call(struct functi
 	args = calloc(n_arg, sizeof(LLVMValueRef));
 
 	i = 0;
+	PREPARE_PTR_LIST(argument_types, argtype);
 	FOR_EACH_PTR(insn->arguments, arg) {
-		args[i++] = pseudo_to_value(fn, insn, arg);
+		LLVMValueRef value;
+		if (arg->type == PSEUDO_VAL) {
+			/* Value pseudos do not have type information. */
+			/* Use the function prototype to get the type. */
+			value = val_to_value(fn, arg->value, argtype);
+		} else {
+			value = pseudo_to_value(fn, insn, arg);
+		}
+		args[i++] = value;
+		NEXT_PTR_LIST(argtype);
 	} END_FOR_EACH_PTR(arg);
+	FINISH_PTR_LIST(argtype);
 
 	func = pseudo_to_value(fn, insn, insn->func);
 	target = LLVMBuildCall(fn->builder, func, args, n_arg, "");