Sparse-LLVM issue compiling NULL pointers

Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

I am trying to debug a failure in sparse-llvm when compiling following:

struct mytype {
 int *foo;
};
extern void init_mytype(struct mytype *mt);
void init_mytype(struct mytype *mt) {
 mt->foo = (void *)0;
}

I am new to sparse so do not fully understand how it works, hence my
explanation below could be wrong.

As far as I understand, an integer constant 0 is converted to a value
pseudo in linearize_expression(). As a value pseudo only has a value
and no type the LLVM IR generator does not have enough information to
ensure that the correct type is used when it encounters the value
pseudo.

While trying to work out how to resolve this issue, I also found
following potential additional issues.

When handling (void*) 0, in the function evaluate_cast() in
evaluate.c, the expression type is changed to NULL type. However this
changed type is not returned.

 if (!(t1->ctype.modifiers & MOD_PTRINHERIT) && class1 == TYPE_PTR &&
     !as1 && (target->flags & Int_const_expr)) {
  if (t1->ctype.base_type == &C->S->void_ctype) {
   if (is_zero_constant(C, target)) {
    /* NULL */
    expr->type = EXPR_VALUE;
    expr->ctype = &C->S->null_ctype;
    expr->value = 0;
    return ctype;
   }
  }
 }

Should this be instead:

 if (!(t1->ctype.modifiers & MOD_PTRINHERIT) && class1 == TYPE_PTR &&
     !as1 && (target->flags & Int_const_expr)) {
  if (t1->ctype.base_type == &C->S->void_ctype) {
   if (is_zero_constant(C, target)) {
    /* NULL */
    expr->type = EXPR_VALUE;
    expr->ctype = &C->S->null_ctype;
    expr->value = 0;
    return expr->ctype;
   }
  }
 }

A related question is around the expansion of cast expressions in
cast_value() function in expand.c. The code snippet I was looking at
is this:

 if (old_size == new_size) {
  expr->value = old->value;
  return;
 }

Should this be changed to:

 if (old_size == new_size) {
  expr->value = old->value;
  expr->ctype = oldtype;
  return;
 }

The two changes above appear to help ensure that a VALUE expression's
type shows correctly that the expression is a NULL pointer. Assuming
this is correct then in value_pseudo() function in lineariez.c, it
would be possible to distinguish between integer constants and a NULL
pointer.

My question is this: should a value pseudo have type information also?
This seems like a necessity for LLVM backend.

Thanks and Regards
Dibyendu

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Tue, Feb 28, 2017 at 06:20:43AM +0000, Dibyendu Majumdar wrote:
> I am trying to debug a failure in sparse-llvm when compiling following:
> 
> struct mytype {
>  int *foo;
> };
> extern void init_mytype(struct mytype *mt);
> void init_mytype(struct mytype *mt) {
>  mt->foo = (void *)0;
> }
> 
> I am new to sparse so do not fully understand how it works, hence my
> explanation below could be wrong.
> 
> As far as I understand, an integer constant 0 is converted to a value
> pseudo in linearize_expression(). As a value pseudo only has a value
> and no type the LLVM IR generator does not have enough information to
> ensure that the correct type is used when it encounters the value
> pseudo.

In some cases, yes.
It would be useful to list and somehow detail those cases.
Even better would be to have test cases.

> While trying to work out how to resolve this issue, I also found
> following potential additional issues.
> 
> When handling (void*) 0, in the function evaluate_cast() in
> evaluate.c, the expression type is changed to NULL type. However this
> changed type is not returned.
> 
>  if (!(t1->ctype.modifiers & MOD_PTRINHERIT) && class1 == TYPE_PTR &&
>      !as1 && (target->flags & Int_const_expr)) {
>   if (t1->ctype.base_type == &C->S->void_ctype) {
>    if (is_zero_constant(C, target)) {
>     /* NULL */
>     expr->type = EXPR_VALUE;
>     expr->ctype = &C->S->null_ctype;
>     expr->value = 0;
>     return ctype;
>    }
>   }
>  }
> 
> Should this be instead:
> 
>  if (!(t1->ctype.modifiers & MOD_PTRINHERIT) && class1 == TYPE_PTR &&
>      !as1 && (target->flags & Int_const_expr)) {
>   if (t1->ctype.base_type == &C->S->void_ctype) {
>    if (is_zero_constant(C, target)) {
>     /* NULL */
>     expr->type = EXPR_VALUE;
>     expr->ctype = &C->S->null_ctype;
>     expr->value = 0;
>     return expr->ctype;
>    }
>   }
>  }

Most probably, yes.
Do you have something that can show it make a difference?

> A related question is around the expansion of cast expressions in
> cast_value() function in expand.c. The code snippet I was looking at
> is this:
> 
>  if (old_size == new_size) {
>   expr->value = old->value;
>   return;
>  }
> 
> Should this be changed to:
> 
>  if (old_size == new_size) {
>   expr->value = old->value;
>   expr->ctype = oldtype;
>   return;
>  }

I don't think so.
At first sight, it look as if doing so is equivalent to dropping the cast
completly (for what concerns the types).

> The two changes above appear to help ensure that a VALUE expression's
> type shows correctly that the expression is a NULL pointer.

I don't think so. There is a difference between 'a null pointer' and
'the NULL pointer'. 'NULL' (at least how we want it here) is of type
'void *' while a 'null pointer' can be any pointer type.

> Assuming
> this is correct then in value_pseudo() function in lineariez.c, it
> would be possible to distinguish between integer constants and a NULL
> pointer.

This shouldn't be needed. The linearization should produce a cast of
the typeless value '0' to a pointer type/a void pointer.

But it's also very possible that this cast is later optimized away ...

> My question is this: should a value pseudo have type information also?

I don't think so.

> This seems like a necessity for LLVM backend.

There is indeed some problems regarding this, we looked a bit at this
some weeks ago. However I firmly believe that the information about
the type belong to the operations and not the values. But this,
indeed, leave unanswered the question "what is the type of a pseudo
which is not the result of an operation, the constant values?".

There is also several issues regarding type in casts. One of them is
solved and is in master now: (9cc8f6624 "fix cast's target type info").
I'm not sure it will help you but it's related to losing some type
info in casts.

I think that the best you can do for us to be able to help you is to create
test case showing concretely the issues you have.


Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

Hi Luc,

On 28 February 2017 at 15:09, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Tue, Feb 28, 2017 at 06:20:43AM +0000, Dibyendu Majumdar wrote:
>> I am trying to debug a failure in sparse-llvm when compiling following:
>>
>> struct mytype {
>>  int *foo;
>> };
>> extern void init_mytype(struct mytype *mt);
>> void init_mytype(struct mytype *mt) {
>>  mt->foo = (void *)0;
>> }
>>
>
> I think that the best you can do for us to be able to help you is to create
> test case showing concretely the issues you have.
>

Above is a simple test case. If you use an LLVM build with assertions
enabled then the code generator aborts:

Assertion failed: getOperand(0)->getType() ==
cast<PointerType>(getOperand(1)->getType())->getElementType() && "Ptr
must be a pointer to Val type!", file
C:\d\llvm-3.9.0.src\lib\IR\Instructions.cpp, line 1436

In the details I posted I was trying to say that :

a) The Expression Value does not appear to pass down the fact that it
has found a NULL pointer - i.e. (void *)0.
b) And even if this is conveyed (by making changes I suggested) then
it is lost at the time of converting to value pseudo. As far as I can
tell the cast is not being optimized away, it is just lost in
translation.

I could be wrong in my analysis as I mentioned before. I will try to
generate some debug info to further show what seems to be happening.

Regards
Dibyendu

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Tue, Feb 28, 2017 at 5:04 PM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
> Hi Luc,
>
> On 28 February 2017 at 15:09, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> On Tue, Feb 28, 2017 at 06:20:43AM +0000, Dibyendu Majumdar wrote:
>>> I am trying to debug a failure in sparse-llvm when compiling following:
>>>
>>> struct mytype {
>>>  int *foo;
>>> };
>>> extern void init_mytype(struct mytype *mt);
>>> void init_mytype(struct mytype *mt) {
>>>  mt->foo = (void *)0;
>>> }
>>>
>>
>> I think that the best you can do for us to be able to help you is to create
>> test case showing concretely the issues you have.
>>
>
> Above is a simple test case. If you use an LLVM build with assertions
> enabled then the code generator aborts:
>
> Assertion failed: getOperand(0)->getType() ==
> cast<PointerType>(getOperand(1)->getType())->getElementType() && "Ptr
> must be a pointer to Val type!", file
> C:\d\llvm-3.9.0.src\lib\IR\Instructions.cpp, line 1436

I had hoped that "sparse-llvm file.c | llvm-dis" would give a bit more info
but well ...

> In the details I posted I was trying to say that :
>
> a) The Expression Value does not appear to pass down the fact that it
> has found a NULL pointer - i.e. (void *)0.
> b) And even if this is conveyed (by making changes I suggested) then
> it is lost at the time of converting to value pseudo. As far as I can
> tell the cast is not being optimized away, it is just lost in
> translation.

OK. It would certainly be a good thing to show/look at the output of
test-linearize together with the C code and any info coming from
sparse-llvm (and llvm-dis) since this output is the readable form
of sparse-llvm's input.


Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 28 February 2017 at 16:04, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> On 28 February 2017 at 15:09, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> On Tue, Feb 28, 2017 at 06:20:43AM +0000, Dibyendu Majumdar wrote:
>>> I am trying to debug a failure in sparse-llvm when compiling following:
>>>
>>> struct mytype {
>>>  int *foo;
>>> };
>>> extern void init_mytype(struct mytype *mt);
>>> void init_mytype(struct mytype *mt) {
>>>  mt->foo = (void *)0;
>>> }
>>>
>>
>> I think that the best you can do for us to be able to help you is to create
>> test case showing concretely the issues you have.
>>
>
> Above is a simple test case. If you use an LLVM build with assertions
> enabled then the code generator aborts:
>
> Assertion failed: getOperand(0)->getType() ==
> cast<PointerType>(getOperand(1)->getType())->getElementType() && "Ptr
> must be a pointer to Val type!", file
> C:\d\llvm-3.9.0.src\lib\IR\Instructions.cpp, line 1436
>
> In the details I posted I was trying to say that :
>
> a) The Expression Value does not appear to pass down the fact that it
> has found a NULL pointer - i.e. (void *)0.
> b) And even if this is conveyed (by making changes I suggested) then
> it is lost at the time of converting to value pseudo. As far as I can
> tell the cast is not being optimized away, it is just lost in
> translation.
>
> I could be wrong in my analysis as I mentioned before. I will try to
> generate some debug info to further show what seems to be happening.
>

I should add that if (int *) is used instead of (void *) in the above
example, it still aborts, although this time the changes I suggested
are not relevant.

Regards

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Tue, Feb 28, 2017 at 4:09 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> There is indeed some problems regarding this, we looked a bit at this
> some weeks ago. However I firmly believe that the information about
> the type belong to the operations and not the values.

I've taken a very quick look at this "mt->foo = (void *)0"
The type info is perfectly present.
If in sparse-llvm.c:output_op_store() you add somewhere something like:
    fprintf(stderr, "-> %s\n", show_typename(insn->type));
You will see that it display the expected type: "int *".
This is all the type info needed: it's the type of insn->target (the
value to be stored)
and the type of the dereferencing of insn->src (the (base) address).

The problem is that output_op_store() doesn't use this info, it tries to deduce
this type via pseudo_to_value() but pseudo_to_value() wrongly assumes that all
PSEUDO_VALUE-pseudo are integer.

Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Tue, Feb 28, 2017 at 06:03:05PM +0100, Luc Van Oostenryck wrote:
> On Tue, Feb 28, 2017 at 4:09 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > There is indeed some problems regarding this, we looked a bit at this
> > some weeks ago. However I firmly believe that the information about
> > the type belong to the operations and not the values.
> 
> I've taken a very quick look at this "mt->foo = (void *)0"
> The type info is perfectly present.
> If in sparse-llvm.c:output_op_store() you add somewhere something like:
>     fprintf(stderr, "-> %s\n", show_typename(insn->type));
> You will see that it display the expected type: "int *".
> This is all the type info needed: it's the type of insn->target (the
> value to be stored)
> and the type of the dereferencing of insn->src (the (base) address).
> 
> The problem is that output_op_store() doesn't use this info, it tries to deduce
> this type via pseudo_to_value() but pseudo_to_value() wrongly assumes that all
> PSEUDO_VALUE-pseudo are integer.


Not very pretty and incomplete but the following patch allow sparse-llvm
to compile this:
	struct mytype {
		int *foo;
	};
	
	extern void init_mytype(struct mytype *mt);
	void init_mytype(struct mytype *mt)
	{
		mt->foo = (int *)mt;
		mt->foo = (void *)mt;
		mt->foo = (int *)0;
		mt->foo = (void *)0;
		mt->foo = (void *)(long)0;
	}

It fail at " ... = (... *)1;" though.


diff --git a/sparse-llvm.c b/sparse-llvm.c
index 9f362b3ed..9e0450ae7 100644
--- a/sparse-llvm.c
+++ b/sparse-llvm.c
@@ -306,6 +306,7 @@ static void pseudo_name(pseudo_t pseudo, char *buf)
 static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *insn, pseudo_t pseudo)
 {
 	LLVMValueRef result = NULL;
+	LLVMTypeRef type;
 
 	switch (pseudo->type) {
 	case PSEUDO_REG:
@@ -360,7 +361,21 @@ static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *ins
 		break;
 	}
 	case PSEUDO_VAL:
-		result = LLVMConstInt(insn_symbol_type(fn->module, insn), pseudo->value, 1);
+		type = insn_symbol_type(fn->module, insn);
+		switch (LLVMGetTypeKind(type)) {
+		case LLVMPointerTypeKind:
+			assert(!pseudo->value);
+			result = LLVMConstPointerNull(type);
+			break;
+		case LLVMIntegerTypeKind:
+			result = LLVMConstInt(type, pseudo->value, 1);
+			break;
+		default:
+			assert(0);
+		}
 		break;
 	case PSEUDO_ARG: {
 		result = LLVMGetParam(fn->fn, pseudo->nr - 1);
@@ -626,6 +641,7 @@ static void output_op_store(struct function *fn, struct instruction *insn)
 
 	addr = calc_memop_addr(fn, insn);
 
 	target_in = pseudo_to_value(fn, insn, insn->target);
 
 	/* perform store */

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

Hi Luc,

Thanks for taking the time to look into this and the fix!

Regards
Dibyendu

On 28 February 2017 at 17:35, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Tue, Feb 28, 2017 at 06:03:05PM +0100, Luc Van Oostenryck wrote:
>> On Tue, Feb 28, 2017 at 4:09 PM, Luc Van Oostenryck
>> <luc.vanoostenryck@gmail.com> wrote:
>> > There is indeed some problems regarding this, we looked a bit at this
>> > some weeks ago. However I firmly believe that the information about
>> > the type belong to the operations and not the values.
>>
>> I've taken a very quick look at this "mt->foo = (void *)0"
>> The type info is perfectly present.
>> If in sparse-llvm.c:output_op_store() you add somewhere something like:
>>     fprintf(stderr, "-> %s\n", show_typename(insn->type));
>> You will see that it display the expected type: "int *".
>> This is all the type info needed: it's the type of insn->target (the
>> value to be stored)
>> and the type of the dereferencing of insn->src (the (base) address).
>>
>> The problem is that output_op_store() doesn't use this info, it tries to deduce
>> this type via pseudo_to_value() but pseudo_to_value() wrongly assumes that all
>> PSEUDO_VALUE-pseudo are integer.
>
>
> Not very pretty and incomplete but the following patch allow sparse-llvm
> to compile this:
>         struct mytype {
>                 int *foo;
>         };
>
>         extern void init_mytype(struct mytype *mt);
>         void init_mytype(struct mytype *mt)
>         {
>                 mt->foo = (int *)mt;
>                 mt->foo = (void *)mt;
>                 mt->foo = (int *)0;
>                 mt->foo = (void *)0;
>                 mt->foo = (void *)(long)0;
>         }
>
> It fail at " ... = (... *)1;" though.
>
>
> diff --git a/sparse-llvm.c b/sparse-llvm.c
> index 9f362b3ed..9e0450ae7 100644
> --- a/sparse-llvm.c
> +++ b/sparse-llvm.c
> @@ -306,6 +306,7 @@ static void pseudo_name(pseudo_t pseudo, char *buf)
>  static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *insn, pseudo_t pseudo)
>  {
>         LLVMValueRef result = NULL;
> +       LLVMTypeRef type;
>
>         switch (pseudo->type) {
>         case PSEUDO_REG:
> @@ -360,7 +361,21 @@ static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *ins
>                 break;
>         }
>         case PSEUDO_VAL:
> -               result = LLVMConstInt(insn_symbol_type(fn->module, insn), pseudo->value, 1);
> +               type = insn_symbol_type(fn->module, insn);
> +               switch (LLVMGetTypeKind(type)) {
> +               case LLVMPointerTypeKind:
> +                       assert(!pseudo->value);
> +                       result = LLVMConstPointerNull(type);
> +                       break;
> +               case LLVMIntegerTypeKind:
> +                       result = LLVMConstInt(type, pseudo->value, 1);
> +                       break;
> +               default:
> +                       assert(0);
> +               }
>                 break;
>         case PSEUDO_ARG: {
>                 result = LLVMGetParam(fn->fn, pseudo->nr - 1);
> @@ -626,6 +641,7 @@ static void output_op_store(struct function *fn, struct instruction *insn)
>
>         addr = calc_memop_addr(fn, insn);
>
>         target_in = pseudo_to_value(fn, insn, insn->target);
>
>         /* perform store */

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 28 February 2017 at 17:35, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Not very pretty and incomplete but the following patch allow sparse-llvm
> to compile this:
>         struct mytype {
>                 int *foo;
>         };
>
>         extern void init_mytype(struct mytype *mt);
>         void init_mytype(struct mytype *mt)
>         {
>                 mt->foo = (int *)mt;
>                 mt->foo = (void *)mt;
>                 mt->foo = (int *)0;
>                 mt->foo = (void *)0;
>                 mt->foo = (void *)(long)0;
>         }
>
> It fail at " ... = (... *)1;" though.
>
>
> +               type = insn_symbol_type(fn->module, insn);
> +               switch (LLVMGetTypeKind(type)) {
> +               case LLVMPointerTypeKind:
> +                       assert(!pseudo->value);
> +                       result = LLVMConstPointerNull(type);
> +                       break;
> +               case LLVMIntegerTypeKind:
> +                       result = LLVMConstInt(type, pseudo->value, 1);
> +                       break;
> +               default:
> +                       assert(0);
> +               }

Following modified version should handle values than 0.

  LLVMTypeRef type = insn_symbol_type(fn->module, insn);
  switch (LLVMGetTypeKind(type)) {
  case LLVMPointerTypeKind:
   if (pseudo->value == 0)
    result = LLVMConstPointerNull(type);
   else
    result = LLVMConstIntToPtr(LLVMConstInt(LLVMIntType(bits_in_pointer),
pseudo->value, 1), type);
   break;
  case LLVMIntegerTypeKind:
   result = LLVMConstInt(type, pseudo->value, 1);
   break;
  default:
   assert(0);
  }

Regards
Dibyendu

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Tue, Feb 28, 2017 at 06:08:16PM +0000, Dibyendu Majumdar wrote:
> 
> Following modified version should handle values than 0.


Good, I'll cook a patch based on this.

But I still have a fundamental problem with pseudo_to_value().
It's working with your example because we're using an OP_STORE
and we're interesting in the type of the "target" and for this
instruction (admittedly, like most instructions) insn->type is
the type of insn->target. With insn->src or OP_SETNE, the result
would have been incorrect.

So while this patch is a little improvement and doesn't hurt,
it's still not a good solution.
What I last wrote on the subject the 29th Jan still holds
(the "type" info should be given as argument to pseudo_to_value()
instead of passing the pointer to the instruction) or at least
something along this line.

Luc Van Oostenryck

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

Hi Luc,

On 28 February 2017 at 17:03, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Tue, Feb 28, 2017 at 4:09 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> There is indeed some problems regarding this, we looked a bit at this
>> some weeks ago. However I firmly believe that the information about
>> the type belong to the operations and not the values.
>

I am trying to work out how a value pseudo correct type can be
determined when the pseudo is a function call argument. Would
appreciate any pointers on this. The current implementation of
pseudo_to_value() uses the function call instruction which is
incorrect.

Thanks and Regards
Dibyendu

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 1 March 2017 at 10:58, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> On 28 February 2017 at 17:03, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> On Tue, Feb 28, 2017 at 4:09 PM, Luc Van Oostenryck
>> <luc.vanoostenryck@gmail.com> wrote:
>>> There is indeed some problems regarding this, we looked a bit at this
>>> some weeks ago. However I firmly believe that the information about
>>> the type belong to the operations and not the values.
>>
>
> I am trying to work out how a value pseudo correct type can be
> determined when the pseudo is a function call argument. Would
> appreciate any pointers on this. The current implementation of
> pseudo_to_value() uses the function call instruction which is
> incorrect.
>

I have implemented a solution that get the type information from the
function prototype for pseudo values when processing function
arguments, but not sure this is correct.

Anyway have hit a bunch of other issues with sparse-llvm ... :-(

Regards
Dibyendu

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Wed, Mar 01, 2017 at 02:45:03PM +0000, Dibyendu Majumdar wrote:
> On 1 March 2017 at 10:58, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> > On 28 February 2017 at 17:03, Luc Van Oostenryck
> > <luc.vanoostenryck@gmail.com> wrote:
> >> On Tue, Feb 28, 2017 at 4:09 PM, Luc Van Oostenryck
> >> <luc.vanoostenryck@gmail.com> wrote:
> >>> There is indeed some problems regarding this, we looked a bit at this
> >>> some weeks ago. However I firmly believe that the information about
> >>> the type belong to the operations and not the values.
> >>
> >
> > I am trying to work out how a value pseudo correct type can be
> > determined when the pseudo is a function call argument. Would
> > appreciate any pointers on this. The current implementation of
> > pseudo_to_value() uses the function call instruction which is
> > incorrect.
> >
> 
> I have implemented a solution that get the type information from the
> function prototype for pseudo values when processing function
> arguments, but not sure this is correct.

You need something like:
+struct symbol *argument_type(pseudo_t src)
+{
+	struct entrypoint *ep = src->def->bb->ep;
+	struct symbol_list *args = ep->name->ctype.base_type->arguments;
+	struct symbol *arg;
+	int i = 0;
+	FOR_EACH_PTR(args, arg) {
+		if (++i == src->nr)
+			return arg;
+	} END_FOR_EACH_PTR(arg);
+
+	assert(0);
+}
 
> Anyway have hit a bunch of other issues with sparse-llvm ... :-(

Each day its problem (and happily its solution too!).


Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 2 March 2017 at 05:21, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>> Anyway have hit a bunch of other issues with sparse-llvm ... :-(
>
> Each day its problem (and happily its solution too!).
>

I will submit test cases for the new problems when I get some time.
But I am beginning to think that there is quite a bit of work needed
to fix the issues, and in any case it might be better to create an
LLVM backend from the parse tree rather than the linearized version.
The current approach is very low level - for example struct member
access bypasses the natural LLVM way of doing it. This approach will
have the consequence that LLVM will generate poor quality code as it
will not have enough information to optimise properly. The current
approach is more suited to backends that directly emit machine code I
think.

Regards
Dibyendu

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Tue, Feb 28, 2017 at 5:49 PM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
>
> I should add that if (int *) is used instead of (void *) in the above
> example, it still aborts, although this time the changes I suggested
> are not relevant.

Strange. It seems to work correctly for me.

Luc

[PATCH] llvm: fix getting type of values

[Luc Van Oostenryck]

In sparse-llvm: there was the assumption that the type
of a PSEUDO_VAL was always one of the integer type.
But this is not always the case: constant pointers,
like NULL, are also of the PSEUDO_VAL kind.

Fix this by using the type associated with the concerned
instruction to retrieve and use the correct type.

Note: while this patch improve the situation, like for example
the test cases added here, it's still not correct since now
we're make the assumption that insn->type is the type we need
for the pseudo. This is maybe often true, but certainly not
always. For example this is not true for:
- OP_STORE/OP_LOAD's insn->src
- OP_SET{EQ,...}'s   insn->src[12]
- in general for any instructions the target have a different type
  than the operands (when we're insterested in the operands).
- probably some  others ones

CC: Dibyendu Majumdar <mobile@majumdar.org.uk>
Reported-by: Dibyendu Majumdar <mobile@majumdar.org.uk>
Some-parts-also-by: Dibyendu Majumdar <mobile@majumdar.org.uk>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 sparse-llvm.c             | 16 +++++++++++++++-
 validation/backend/null.c | 24 ++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 validation/backend/null.c

diff --git a/sparse-llvm.c b/sparse-llvm.c
index 9f362b3ed..ff66a96a7 100644
--- a/sparse-llvm.c
+++ b/sparse-llvm.c
@@ -305,6 +305,7 @@ static void pseudo_name(pseudo_t pseudo, char *buf)
 
 static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *insn, pseudo_t pseudo)
 {
+	LLVMTypeRef type, iptr_type;
 	LLVMValueRef result = NULL;
 
 	switch (pseudo->type) {
@@ -360,7 +361,20 @@ static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *ins
 		break;
 	}
 	case PSEUDO_VAL:
-		result = LLVMConstInt(insn_symbol_type(fn->module, insn), pseudo->value, 1);
+		type = insn_symbol_type(fn->module, insn);
+		switch (LLVMGetTypeKind(type)) {
+		case LLVMPointerTypeKind:
+			iptr_type = LLVMIntType(bits_in_pointer);
+			result = LLVMConstInt(iptr_type, pseudo->value, 1);
+			result = LLVMConstIntToPtr(result, type);
+			break;
+		case LLVMIntegerTypeKind:
+			result = LLVMConstInt(type, pseudo->value, 1);
+			break;
+		default:
+			assert(0);
+		}
+
 		break;
 	case PSEUDO_ARG: {
 		result = LLVMGetParam(fn->fn, pseudo->nr - 1);
diff --git a/validation/backend/null.c b/validation/backend/null.c
new file mode 100644
index 000000000..5c595c70b
--- /dev/null
+++ b/validation/backend/null.c
@@ -0,0 +1,24 @@
+extern int *ip[];
+
+void foo(void);
+void foo(void)
+{
+	ip[0] = (void *)0L;
+	ip[1] = (int *)0L;
+	ip[2] = (void *)0;
+	ip[3] = (int *)0;
+	ip[4] = (void *)(long)0;
+	ip[5] = (int *)(long)0;
+	ip[6] = (void *)123;
+	ip[7] = (int *)123;
+	ip[8] = (void *)123L;
+	ip[9] = (int *)123L;
+	ip[10] = (void *)(long)123;
+	ip[11] = (int *)(long)123;
+}
+
+/*
+ * check-name: store constants to pointer
+ * check-command: sparse-llvm $file
+ * check-output-ignore
+ */
-- 
2.11.1

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 02, 2017 at 05:41:59AM +0000, Dibyendu Majumdar wrote:
> On 2 March 2017 at 05:21, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> >> Anyway have hit a bunch of other issues with sparse-llvm ... :-(
> >
> > Each day its problem (and happily its solution too!).
> >
> 
> I will submit test cases for the new problems when I get some time.

OK. Thanks.

> But I am beginning to think that there is quite a bit of work needed
> to fix the issues, and in any case it might be better to create an
> LLVM backend from the parse tree rather than the linearized version.

Possible, but this will certainly need some work too.

> The current approach is very low level - for example struct member
> access bypasses the natural LLVM way of doing it.

Yes, that's true.

> This approach will
> have the consequence that LLVM will generate poor quality code as it
> will not have enough information to optimise properly. The current
> approach is more suited to backends that directly emit machine code I
> think.

As far as I understood, the idea behind the linearized code and the
optimization made on it was to have it's own backend. Sparse-llvm
only came much later.

Regards,
Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 2 March 2017 at 13:56, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Thu, Mar 02, 2017 at 05:41:59AM +0000, Dibyendu Majumdar wrote:
>> But I am beginning to think that there is quite a bit of work needed
>> to fix the issues, and in any case it might be better to create an
>> LLVM backend from the parse tree rather than the linearized version.
>
> Possible, but this will certainly need some work too.
>

True, which is why I was hoping that I could at least get the current
version working. Unfortunately even the simplest programs I through at
it fail. Looks like it hasn't really been tested as there is no
defined subset of C that works.

Maybe if we keep fixing the issues one by one we will get to a point
where it works.

Regards

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

Hi Luc,

On 2 March 2017 at 13:56, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Thu, Mar 02, 2017 at 05:41:59AM +0000, Dibyendu Majumdar wrote:
>> On 2 March 2017 at 05:21, Luc Van Oostenryck
>> <luc.vanoostenryck@gmail.com> wrote:
>> >> Anyway have hit a bunch of other issues with sparse-llvm ... :-(
>> >
>> > Each day its problem (and happily its solution too!).
>> >
>>
>> I will submit test cases for the new problems when I get some time.
>

Below is an example that fails for me. I am running a modified version
of Sparse although the modifications are to do with a) removing global
state and b) compiling with Visual C++ on Windows 10 - and all sparse
tests pass so I don't think they have anything to do with the failure
here. I do have Linux build of sparse but without LLVM so I will need
to sort that out to test. Anyway, I would be interested to know if you
get a failure (you will need to have LLVM assertions enabled).

typedef unsigned long long size_t;
struct buffer_type_st {
 struct buffer_type_st    *next_buffer;
 char           *buffer;
};
typedef struct buffer_type_st buffer_type_t;
struct link_st {
 struct link_st           *next;
};
typedef struct link_st link_t;
struct allocator_st {
 buffer_type_t    *buffer_list;
 link_t           *free_list;
 char           *next_avail;
 char           *last;
 size_t          size;
 size_t          n;
};
typedef struct allocator_st allocator;
extern void
grow_allocator(allocator * a);
extern void *
malloc(size_t size);
void
grow_allocator(allocator * a)
{
 buffer_type_t    *tmp;
 tmp = (buffer_type_t *) malloc(sizeof(buffer_type_t));
 tmp->buffer = (char *) malloc(a->size * a->n);
 tmp->next_buffer = a->buffer_list;
 a->buffer_list = tmp;
 a->next_avail = a->buffer_list->buffer;
 a->last = a->next_avail + (a->size * a->n);
}


The failure occurs in the line:

 a->next_avail = a->buffer_list->buffer;

I am guessing it has to do with the nested member access but I have
not yet been able to work out what the code is doing.

LLVM gives me following error:

Assertion failed: S->getType()->isPtrOrPtrVectorTy() && "Invalid
cast", file C:\d\llvm-3.9.0.src\lib\IR\Instructions.cpp, line 2759

This occurs in calc_memop_addr() function when it tries to do following:

 as = LLVMGetPointerAddressSpace(LLVMTypeOf(src));

If I dump insn, insn->src, and the LLVM value and type before this line, I get:

1) insn load.64     %r18 <- 8[%r2]
2) pseudo %r2
3) store %struct.buffer_type_st* %R2, %struct.buffer_type_st** %26
4) void

The type of the LLVM store instruction is 'void'.

My guess is that something is going horribly wrong in the way member
access is handled.

If you are able to reproduce this and have any suggestions then please
let me know.

Thanks and Regards
Dibyendu

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 02, 2017 at 02:33:09PM +0000, Dibyendu Majumdar wrote:
> This occurs in calc_memop_addr() function when it tries to do following:
> 
>  as = LLVMGetPointerAddressSpace(LLVMTypeOf(src));
> 
> If I dump insn, insn->src, and the LLVM value and type before this line, I get:
> 
> 1) insn load.64     %r18 <- 8[%r2]
> 2) pseudo %r2
> 3) store %struct.buffer_type_st* %R2, %struct.buffer_type_st** %26
> 4) void
> 
> The type of the LLVM store instruction is 'void'.
> 
> My guess is that something is going horribly wrong in the way member
> access is handled.
> 
> If you are able to reproduce this and have any suggestions then please
> let me know.

Why I try your code (without LLVM assertions), I got indeed several
problems:
	Call parameter type does not match function signature!
	i0 16
	 i64  %1 = call i8* @malloc(i0 16)
	Invalid bitcast
	  %27 = bitcast void <badref> to i8**
	Both operands to a binary operator are not of the same type!
	  %R31 = add void <badref>, i64 %R30
	Stored value type does not match pointer operand type!
	  store void %R31, i8** %46

The first one is really weird but I think you don't see it.
The next two I have no idea.
The fourth have something obviously wrong with the type of its 'target'.
None of these seems to correspond to the problem you're seeing.

However, while running sparse-llvm on some code sample I use to test
the linearization, I see that most errors are type errors and are
related to pointer arithmetic, exactly where LLVM's getelemptr is used.
Most offending instructions are OP_ADD (but since I have tests for 
bitfields I see also errors for OP_AND, OP_OR & OP_LSR).
I guess that if you test OP_ADD instruction with pointer on one side
and integer on tne other side and issue an appropriate LLVMBuildGEP(),
things will already be much better.

Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 02, 2017 at 02:05:34PM +0000, Dibyendu Majumdar wrote:
> On 2 March 2017 at 13:56, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > On Thu, Mar 02, 2017 at 05:41:59AM +0000, Dibyendu Majumdar wrote:
> >> But I am beginning to think that there is quite a bit of work needed
> >> to fix the issues, and in any case it might be better to create an
> >> LLVM backend from the parse tree rather than the linearized version.
> >
> > Possible, but this will certainly need some work too.
> >
> 
> True, which is why I was hoping that I could at least get the current
> version working. Unfortunately even the simplest programs I through at
> it fail. Looks like it hasn't really been tested as there is no
> defined subset of C that works.
> 
> Maybe if we keep fixing the issues one by one we will get to a point
> where it works.

I have no doubts about it.


Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

Hi Luc,

On 2 March 2017 at 16:04, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Thu, Mar 02, 2017 at 02:33:09PM +0000, Dibyendu Majumdar wrote:
>> This occurs in calc_memop_addr() function when it tries to do following:
>>
>>  as = LLVMGetPointerAddressSpace(LLVMTypeOf(src));
>>
>> If I dump insn, insn->src, and the LLVM value and type before this line, I get:
>>
>> 1) insn load.64     %r18 <- 8[%r2]
>> 2) pseudo %r2
>> 3) store %struct.buffer_type_st* %R2, %struct.buffer_type_st** %26
>> 4) void
>>
>> The type of the LLVM store instruction is 'void'.
>>
>> My guess is that something is going horribly wrong in the way member
>> access is handled.
>>
>> If you are able to reproduce this and have any suggestions then please
>> let me know.
>
> Why I try your code (without LLVM assertions), I got indeed several
> problems:
>         Call parameter type does not match function signature!
>         i0 16
>          i64  %1 = call i8* @malloc(i0 16)

I have the function argument fix so I don't get the malloc error.

>         Invalid bitcast
>           %27 = bitcast void <badref> to i8**
>         Both operands to a binary operator are not of the same type!

I think this corresponds to the code that fails - the type of the LLVM
instruction is 'void' and the code is trying to cast to it or
something.

>           %R31 = add void <badref>, i64 %R30
>         Stored value type does not match pointer operand type!
>           store void %R31, i8** %46
>

The code aborts at previous step

> The first one is really weird but I think you don't see it.

It is the function call issue, for which I have a fix I described.

> The next two I have no idea.
> The fourth have something obviously wrong with the type of its 'target'.

> However, while running sparse-llvm on some code sample I use to test
> the linearization, I see that most errors are type errors and are
> related to pointer arithmetic, exactly where LLVM's getelemptr is used.
> Most offending instructions are OP_ADD (but since I have tests for
> bitfields I see also errors for OP_AND, OP_OR & OP_LSR).
> I guess that if you test OP_ADD instruction with pointer on one side
> and integer on tne other side and issue an appropriate LLVMBuildGEP(),
> things will already be much better.
>

Here is the output from linearize. I think the way stores and loads
are handled is broken. It appears that the last store / load
instruction is stored in insn->target->priv, and then used later on
... I do not understand what the code is trying to do. Is it trying to
optimize away stores and loads?

grow_allocator:
.L0000022DAFA94A88:
        <entry-point>
        call.64     %r1 <- malloc, $16
        ptrcast.64  %r2 <- (64) %r1
        load.64     %r4 <- 32[%arg1]
        load.64     %r6 <- 40[%arg1]
        mulu.64     %r7 <- %r4, %r6
        call.64     %r8 <- malloc, %r7
        ptrcast.64  %r9 <- (64) %r8
        store.64    %r9 -> 8[%r2]
        load.64     %r12 <- 0[%arg1]
        store.64    %r12 -> 0[%r2]
        store.64    %r2 -> 0[%arg1]
        load.64     %r18 <- 8[%r2]
        store.64    %r18 -> 16[%arg1]
        load.64     %r23 <- 32[%arg1]
        load.64     %r25 <- 40[%arg1]
        mulu.64     %r26 <- %r23, %r25
        add.64      %r27 <- %r18, %r26
        store.64    %r27 -> 24[%arg1]
        ret

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 2 March 2017 at 16:29, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> Hi Luc,
>
> On 2 March 2017 at 16:04, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> On Thu, Mar 02, 2017 at 02:33:09PM +0000, Dibyendu Majumdar wrote:
>>> This occurs in calc_memop_addr() function when it tries to do following:
>>>
>>>  as = LLVMGetPointerAddressSpace(LLVMTypeOf(src));
>>>
>>> If I dump insn, insn->src, and the LLVM value and type before this line, I get:
>>>
>>> 1) insn load.64     %r18 <- 8[%r2]
>>> 2) pseudo %r2
>>> 3) store %struct.buffer_type_st* %R2, %struct.buffer_type_st** %26
>>> 4) void
>>>
>>> The type of the LLVM store instruction is 'void'.
>>>
>>> My guess is that something is going horribly wrong in the way member
>>> access is handled.
>>>
>>> If you are able to reproduce this and have any suggestions then please
>>> let me know.
>>
>> Why I try your code (without LLVM assertions), I got indeed several
>> problems:
>>         Call parameter type does not match function signature!
>>         i0 16
>>          i64  %1 = call i8* @malloc(i0 16)
>
> I have the function argument fix so I don't get the malloc error.
>
>>         Invalid bitcast
>>           %27 = bitcast void <badref> to i8**
>>         Both operands to a binary operator are not of the same type!
>
> I think this corresponds to the code that fails - the type of the LLVM
> instruction is 'void' and the code is trying to cast to it or
> something.
>
>>           %R31 = add void <badref>, i64 %R30
>>         Stored value type does not match pointer operand type!
>>           store void %R31, i8** %46
>>
>
> The code aborts at previous step
>
>> The first one is really weird but I think you don't see it.
>
> It is the function call issue, for which I have a fix I described.
>
>> The next two I have no idea.
>> The fourth have something obviously wrong with the type of its 'target'.
>
>> However, while running sparse-llvm on some code sample I use to test
>> the linearization, I see that most errors are type errors and are
>> related to pointer arithmetic, exactly where LLVM's getelemptr is used.
>> Most offending instructions are OP_ADD (but since I have tests for
>> bitfields I see also errors for OP_AND, OP_OR & OP_LSR).
>> I guess that if you test OP_ADD instruction with pointer on one side
>> and integer on tne other side and issue an appropriate LLVMBuildGEP(),
>> things will already be much better.
>>
>
> Here is the output from linearize. I think the way stores and loads
> are handled is broken. It appears that the last store / load
> instruction is stored in insn->target->priv, and then used later on
> ... I do not understand what the code is trying to do. Is it trying to
> optimize away stores and loads?
>
> grow_allocator:
> .L0000022DAFA94A88:
>         <entry-point>
>         call.64     %r1 <- malloc, $16
>         ptrcast.64  %r2 <- (64) %r1
>         load.64     %r4 <- 32[%arg1]
>         load.64     %r6 <- 40[%arg1]
>         mulu.64     %r7 <- %r4, %r6
>         call.64     %r8 <- malloc, %r7
>         ptrcast.64  %r9 <- (64) %r8
>         store.64    %r9 -> 8[%r2]
>         load.64     %r12 <- 0[%arg1]
>         store.64    %r12 -> 0[%r2]
>         store.64    %r2 -> 0[%arg1]
>         load.64     %r18 <- 8[%r2]
>         store.64    %r18 -> 16[%arg1]
>         load.64     %r23 <- 32[%arg1]
>         load.64     %r25 <- 40[%arg1]
>         mulu.64     %r26 <- %r23, %r25
>         add.64      %r27 <- %r18, %r26
>         store.64    %r27 -> 24[%arg1]
>         ret

Just to clarify I mean above that the way Sparse-LLVM handles it is broken.

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

Hi Luc,

On 2 March 2017 at 05:21, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Wed, Mar 01, 2017 at 02:45:03PM +0000, Dibyendu Majumdar wrote:
>> On 1 March 2017 at 10:58, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
>> > On 28 February 2017 at 17:03, Luc Van Oostenryck
>> > <luc.vanoostenryck@gmail.com> wrote:
>> >> On Tue, Feb 28, 2017 at 4:09 PM, Luc Van Oostenryck
>> >> <luc.vanoostenryck@gmail.com> wrote:
>> >>> There is indeed some problems regarding this, we looked a bit at this
>> >>> some weeks ago. However I firmly believe that the information about
>> >>> the type belong to the operations and not the values.
>> >>
>> >
>> > I am trying to work out how a value pseudo correct type can be
>> > determined when the pseudo is a function call argument. Would
>> > appreciate any pointers on this. The current implementation of
>> > pseudo_to_value() uses the function call instruction which is
>> > incorrect.
>> >
>>
>> I have implemented a solution that get the type information from the
>> function prototype for pseudo values when processing function
>> arguments, but not sure this is correct.
>
> You need something like:
> +struct symbol *argument_type(pseudo_t src)
> +{
> +       struct entrypoint *ep = src->def->bb->ep;
> +       struct symbol_list *args = ep->name->ctype.base_type->arguments;
> +       struct symbol *arg;
> +       int i = 0;
> +       FOR_EACH_PTR(args, arg) {
> +               if (++i == src->nr)
> +                       return arg;
> +       } END_FOR_EACH_PTR(arg);
> +
> +       assert(0);
> +}
>
>> Anyway have hit a bunch of other issues with sparse-llvm ... :-(
>
> Each day its problem (and happily its solution too!).
>

I am using the following fix. There is some noise below as this code
is from modified version.


static LLVMValueRef pseudo_val_to_value(struct dmr_C *C, struct
function *fn, LLVMTypeRef type, pseudo_t pseudo)
{
 assert(pseudo->type == PSEUDO_VAL);
 LLVMValueRef result = NULL;
 switch (LLVMGetTypeKind(type)) {
 case LLVMPointerTypeKind:
  if (pseudo->value == 0)
   result = LLVMConstPointerNull(type);
  else
   result = LLVMConstIntToPtr(
    LLVMConstInt(
     LLVMIntType(C->target->bits_in_pointer),
     pseudo->value, 1),
    type);
  break;
 case LLVMIntegerTypeKind:
  result = LLVMConstInt(type, pseudo->value, 1);
  break;
 default:
  assert(0);
 }
 // ORIGINAL result = LLVMConstInt(type, pseudo->value, 1);
 // PREVIOUS FIX result = LLVMConstInt(LLVMInt64Type(), pseudo->value, 1);
 return result;
}

static LLVMValueRef pseudo_to_value(struct dmr_C *C, struct function
*fn, struct instruction *insn, pseudo_t pseudo)
{
 ...
 case PSEUDO_VAL: {
  LLVMTypeRef type = insn_symbol_type(C, fn->module, insn);
  result = pseudo_val_to_value(C, fn, type, pseudo);
  break;
 }
 ...
}

static void output_op_call(struct dmr_C *C, struct function *fn,
struct instruction *insn)
{
 LLVMValueRef target, func;
 int n_arg = 0, i;
 struct pseudo *arg;
 LLVMValueRef *args;
 pseudo_t function_proto = insn->func;
 int n_proto_args = 0;
 struct symbol *proto_symbol = function_proto->sym->ctype.base_type;
 struct symbol *proto_arg;
 struct symbol **proto_args;
 /* count function prototype args, get prototype argument symbols */
 FOR_EACH_PTR(proto_symbol->arguments, proto_arg) {
  n_proto_args++;
 } END_FOR_EACH_PTR(proto_arg);
 proto_args = alloca(n_proto_args * sizeof(struct symbol *));
 int idx = 0;
 FOR_EACH_PTR(proto_symbol->arguments, proto_arg) {
  proto_args[idx++] = proto_arg->ctype.base_type;
 } END_FOR_EACH_PTR(proto_arg);
 n_arg = 0;
 FOR_EACH_PTR(insn->arguments, arg) {
  n_arg++;
 } END_FOR_EACH_PTR(arg);
 if (n_arg != n_proto_args) {
  fprintf(stderr, "Mismatch in function arguments\n");
  abort();
 }
 args = alloca(n_arg * sizeof(LLVMValueRef));
 i = 0;
 FOR_EACH_PTR(insn->arguments, arg) {
  if (arg->type == PSEUDO_VAL) {
   /* as value pseudo do not have type information we use the
      function prototype to decide types */
   LLVMTypeRef type = symbol_type(C, fn->module, proto_args[i]);
   args[i] = pseudo_val_to_value(C, fn, type, arg);
  }
  else {
   args[i] = pseudo_to_value(C, fn, insn, arg);
  }
  i++;
 } END_FOR_EACH_PTR(arg);
 func = pseudo_to_value(C, fn, insn, insn->func);
 target = LLVMBuildCall(fn->builder, func, args, n_arg, "");
 insn->target->priv = target;
}

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 2 March 2017 at 16:29, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> Here is the output from linearize. I think the way stores and loads
> are handled is broken. It appears that the last store / load
> instruction is stored in insn->target->priv, and then used later on
> ... I do not understand what the code is trying to do. Is it trying to
> optimize away stores and loads?
>
> grow_allocator:
> .L0000022DAFA94A88:
>         <entry-point>
>         call.64     %r1 <- malloc, $16
>         ptrcast.64  %r2 <- (64) %r1
>         load.64     %r4 <- 32[%arg1]
>         load.64     %r6 <- 40[%arg1]
>         mulu.64     %r7 <- %r4, %r6
>         call.64     %r8 <- malloc, %r7
>         ptrcast.64  %r9 <- (64) %r8
>         store.64    %r9 -> 8[%r2]
>         load.64     %r12 <- 0[%arg1]
>         store.64    %r12 -> 0[%r2]

It appears that the sparse-llvm code is storing the LLVM instruction
for '%r2' in pseudo->priv.

>         store.64    %r2 -> 0[%arg1]

And then it using the value of the store instruction whenever it sees '%r2'?

>         load.64     %r18 <- 8[%r2]

But here it fails because it needs to cast the LLVM store instruction
to be a pointer to access 8[]?

Regards

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

Hi Luc,

I was looking at whether I can use the standard LLVM GEP api to access
struct and array members in sparse-llvm. I noticed that the output
from sparse has byte offsets of each member - but not the member's
ordinal position. To use the native LLVM methods we would have to
allow LLVM to work out byte offsets, and just use the struct or array
definition. This would I think make the code more robust and also lead
to better optimisation.

Unfortunately LLVM does not support unions so for unions a bit of what
is being done now is probably needed, but even here it is better to
cast a union to its member struct type and then use LLVM GEP api to
access its members.

What do you think?

I think adding the ordinal position to sparse symbols is a trivial
enhancement - do you agree?

Regards
Dibyendu

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 02, 2017 at 04:30:53PM +0000, Dibyendu Majumdar wrote:
> >> Why I try your code (without LLVM assertions), I got indeed several
> >> problems:
> >>         Call parameter type does not match function signature!
> >>         i0 16
> >>          i64  %1 = call i8* @malloc(i0 16)
> >
> > I have the function argument fix so I don't get the malloc error.

Good.

> >>         Invalid bitcast
> >>           %27 = bitcast void <badref> to i8**
> >>         Both operands to a binary operator are not of the same type!
> >
> > I think this corresponds to the code that fails - the type of the LLVM
> > instruction is 'void' and the code is trying to cast to it or
> > something.

That's obviously wrong.

> >>           %R31 = add void <badref>, i64 %R30
> >>         Stored value type does not match pointer operand type!
> >>           store void %R31, i8** %46
> >>
> >
> > The code aborts at previous step
> >
> >> The first one is really weird but I think you don't see it.
> >
> > It is the function call issue, for which I have a fix I described.

OK.

> >> The next two I have no idea.
> >> The fourth have something obviously wrong with the type of its 'target'.
> >
> >> However, while running sparse-llvm on some code sample I use to test
> >> the linearization, I see that most errors are type errors and are
> >> related to pointer arithmetic, exactly where LLVM's getelemptr is used.
> >> Most offending instructions are OP_ADD (but since I have tests for
> >> bitfields I see also errors for OP_AND, OP_OR & OP_LSR).
> >> I guess that if you test OP_ADD instruction with pointer on one side
> >> and integer on tne other side and issue an appropriate LLVMBuildGEP(),
> >> things will already be much better.
> >>
> >
> > Here is the output from linearize. I think the way stores and loads
> > are handled is broken. It appears that the last store / load
> > instruction is stored in insn->target->priv, and then used later on
> > ... I do not understand what the code is trying to do. Is it trying to
> > optimize away stores and loads?

No, no.
What is stored in ->priv is the target's  LLVMValueRef (and for a store
the 'target' is what need to be stored).
And indeed there is a bug there: it's target_in that should be stored in
->priv (in fact, for a store, there is no need to put anything at all
in this field; at least I don't see any reason why it should).
Nice catch.
I don't know how it's related to your problem though.

Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 02, 2017 at 04:39:38PM +0000, Dibyendu Majumdar wrote:
> I am using the following fix. There is some noise below as this code
> is from modified version.

OK, thanks.
I'll look at this a bit later, possibly tomorrow.

Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 02, 2017 at 05:04:04PM +0100, Luc Van Oostenryck wrote:
> However, while running sparse-llvm on some code sample I use to test
> the linearization, I see that most errors are type errors and are
> related to pointer arithmetic, exactly where LLVM's getelemptr is used.
> Most offending instructions are OP_ADD (but since I have tests for 
> bitfields I see also errors for OP_AND, OP_OR & OP_LSR).
> I guess that if you test OP_ADD instruction with pointer on one side
> and integer on tne other side and issue an appropriate LLVMBuildGEP(),
> things will already be much better.

Another place, that clearly need some more love and attention is
output_op_ptrcast() (and possibly outpout_op_cast() too):
Not all OP_PTRCAST can be mapped to an LLVM bitcast, only the ones
wich doesn't change the size can (and even, I'm not suer all can).

Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 2 March 2017 at 17:18, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Thu, Mar 02, 2017 at 04:30:53PM +0000, Dibyendu Majumdar wrote:
>> > Here is the output from linearize. I think the way stores and loads
>> > are handled is broken. It appears that the last store / load
>> > instruction is stored in insn->target->priv, and then used later on
>> > ... I do not understand what the code is trying to do. Is it trying to
>> > optimize away stores and loads?
>
> No, no.
> What is stored in ->priv is the target's  LLVMValueRef (and for a store
> the 'target' is what need to be stored).
> And indeed there is a bug there: it's target_in that should be stored in
> ->priv (in fact, for a store, there is no need to put anything at all
> in this field; at least I don't see any reason why it should).
> Nice catch.
> I don't know how it's related to your problem though.
>

My question was why is something being stored in 'priv' and used
later? That seems to be an optimisation? Why not recompute every time?

I think this caching in 'priv' and using it later is the cause of the
wrong code - unless as you say the value stored in 'priv' is incorrect
and needs fixing.

Regards

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 02, 2017 at 05:18:16PM +0000, Dibyendu Majumdar wrote:
> Hi Luc,
> 
> I was looking at whether I can use the standard LLVM GEP api to access
> struct and array members in sparse-llvm. I noticed that the output
> from sparse has byte offsets of each member - but not the member's
> ordinal position.

Indeed, when the offset is a compile constant otherwise the offset
is set to zero and there is a pair OP_MUL/OP_ADD before that does
the needed pointer arithmetic.

> To use the native LLVM methods we would have to
> allow LLVM to work out byte offsets, and just use the struct or array
> definition. This would I think make the code more robust and also lead
> to better optimisation.

I'm not sure to understand here: which byte offsets need to be worked out?

Also, for the optimization, I wouldn't worry for now because:
- the linearized code is already optimized (nothing sophisticated but the
  essential is already there)
- you can always later call LLVM's optimization passes
- the code selector will also do some optimization.

> Unfortunately LLVM does not support unions so for unions a bit of what
> is being done now is probably needed, but even here it is better to
> cast a union to its member struct type and then use LLVM GEP api to
> access its members.
> 
> What do you think?

Honestly, I think nothing because I don't understand enough the issues
with sparse-llvm and what LLVM needs.

> I think adding the ordinal position to sparse symbols is a trivial
> enhancement - do you agree?

Unless I'm misunderstand what you mean, these postion are already there
but not explicitly: you can lookup the lists to see what position a field
in a structure is.
If you need the byte offsets, those are already available (or trivialy
calculable).

Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 02, 2017 at 05:03:06PM +0000, Dibyendu Majumdar wrote:
> On 2 March 2017 at 16:29, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> > Here is the output from linearize. I think the way stores and loads
> > are handled is broken. It appears that the last store / load
> > instruction is stored in insn->target->priv, and then used later on
> > ... I do not understand what the code is trying to do. Is it trying to
> > optimize away stores and loads?
> >
> > grow_allocator:
> > .L0000022DAFA94A88:
> >         <entry-point>
> >         call.64     %r1 <- malloc, $16
> >         ptrcast.64  %r2 <- (64) %r1
> >         load.64     %r4 <- 32[%arg1]
> >         load.64     %r6 <- 40[%arg1]
> >         mulu.64     %r7 <- %r4, %r6
> >         call.64     %r8 <- malloc, %r7
> >         ptrcast.64  %r9 <- (64) %r8
> >         store.64    %r9 -> 8[%r2]
> >         load.64     %r12 <- 0[%arg1]
> >         store.64    %r12 -> 0[%r2]
> 
> It appears that the sparse-llvm code is storing the LLVM instruction
> for '%r2' in pseudo->priv.
> 
> >         store.64    %r2 -> 0[%arg1]

Ah yes, it overwrite the correct value previously stored there (the
LLVMValueRef corresponding to %r2) with the return value of LLVMBuildStore().

> And then it using the value of the store instruction whenever it sees '%r2'?
> 
> >         load.64     %r18 <- 8[%r2]
> 
> But here it fails because it needs to cast the LLVM store instruction
> to be a pointer to access 8[]?

Yes, now anything using %r2 will go wrong.

Removing the last line of output_op_store() (insn->target->priv = target;)
should fix this.


Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 2, 2017 at 6:49 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>>
>> It appears that the sparse-llvm code is storing the LLVM instruction
>> for '%r2' in pseudo->priv.
>>
>> >         store.64    %r2 -> 0[%arg1]
>
> Ah yes, it overwrite the correct value previously stored there (the
> LLVMValueRef corresponding to %r2) with the return value of LLVMBuildStore().
>
>> And then it using the value of the store instruction whenever it sees '%r2'?
>>
>> >         load.64     %r18 <- 8[%r2]
>>
>> But here it fails because it needs to cast the LLVM store instruction
>> to be a pointer to access 8[]?
>
> Yes, now anything using %r2 will go wrong.
>
> Removing the last line of output_op_store() (insn->target->priv = target;)
> should fix this.


In fact LLVMBuildStore() return a LLVMValueRef.
Could this be the 'void' we seen?

Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 2 March 2017 at 17:57, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>> Removing the last line of output_op_store() (insn->target->priv = target;)
>> should fix this.
>
> In fact LLVMBuildStore() return a LLVMValueRef.
> Could this be the 'void' we seen?
>

It returns a Value that is a store instruction, or at least that's the
way I interpret this and so yes, I think the type of this was void.

BTW removing the line makes progress, but it fails at the add
instruction later. Are there other issues like this you can spot? I am
a bit lost here I am afraid.

Regards

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

Hi Luc,

On 2 March 2017 at 16:04, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> However, while running sparse-llvm on some code sample I use to test
> the linearization, I see that most errors are type errors and are
> related to pointer arithmetic, exactly where LLVM's getelemptr is used.
> Most offending instructions are OP_ADD (but since I have tests for
> bitfields I see also errors for OP_AND, OP_OR & OP_LSR).
> I guess that if you test OP_ADD instruction with pointer on one side
> and integer on tne other side and issue an appropriate LLVMBuildGEP(),
> things will already be much better.
>

This seems spot on as by making change as below, the final assertion
failure went away. I have not yet checked the generated code but that
is next.

 case OP_ADD:
  if (symbol_is_fp_type(C, insn->type))
   target = LLVMBuildFAdd(fn->builder, lhs, rhs, target_name);
  else {
   if (LLVMGetTypeKind(LLVMTypeOf(lhs)) == LLVMPointerTypeKind) {
    target = LLVMBuildGEP(fn->builder, lhs, &rhs, 1, "");
   }
   else if (LLVMGetTypeKind(LLVMTypeOf(lhs)) == LLVMPointerTypeKind) {
    target = LLVMBuildGEP(fn->builder, rhs, &lhs, 1, "");
   }
   else {
    target = LLVMBuildAdd(fn->builder, lhs, rhs, target_name);
   }
  }
  break;

Thanks and Regards

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 2 March 2017 at 17:43, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Thu, Mar 02, 2017 at 05:18:16PM +0000, Dibyendu Majumdar wrote:
>> Hi Luc,
>>
>> I was looking at whether I can use the standard LLVM GEP api to access
>> struct and array members in sparse-llvm. I noticed that the output
>> from sparse has byte offsets of each member - but not the member's
>> ordinal position.
>
> Indeed, when the offset is a compile constant otherwise the offset
> is set to zero and there is a pair OP_MUL/OP_ADD before that does
> the needed pointer arithmetic.
>
>> To use the native LLVM methods we would have to
>> allow LLVM to work out byte offsets, and just use the struct or array
>> definition. This would I think make the code more robust and also lead
>> to better optimisation.
>
> I'm not sure to understand here: which byte offsets need to be worked out?

You use the GEP API which allows you say that you want to access field
5 from a struct, and so on. Here 5 is the fifth field. LLVM works out
the rest. This is better than casting the struct to char* and then
doing pointer arithmetic.

>
> Also, for the optimization, I wouldn't worry for now because:
> - the linearized code is already optimized (nothing sophisticated but the
>   essential is already there)
> - you can always later call LLVM's optimization passes
> - the code selector will also do some optimization.
>

Sure but my experience with LLVM is that it needs to have type
metadata for each store and load instruction to properly do type based
alias analysis. Otherwise it will miss optimisation opportunities.
Maybe we can still provide type metadata as we do have the symbol
definition etc.

But I agree that if sparse-llvm works correctly for all inputs then
that is a big step forward (for me) !

Regards

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 2, 2017 at 7:58 PM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
> On 2 March 2017 at 17:43, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> On Thu, Mar 02, 2017 at 05:18:16PM +0000, Dibyendu Majumdar wrote:
>>> Hi Luc,
>>>
>>> I was looking at whether I can use the standard LLVM GEP api to access
>>> struct and array members in sparse-llvm. I noticed that the output
>>> from sparse has byte offsets of each member - but not the member's
>>> ordinal position.
>>
>> Indeed, when the offset is a compile constant otherwise the offset
>> is set to zero and there is a pair OP_MUL/OP_ADD before that does
>> the needed pointer arithmetic.
>>
>>> To use the native LLVM methods we would have to
>>> allow LLVM to work out byte offsets, and just use the struct or array
>>> definition. This would I think make the code more robust and also lead
>>> to better optimisation.
>>
>> I'm not sure to understand here: which byte offsets need to be worked out?
>
> You use the GEP API which allows you say that you want to access field
> 5 from a struct, and so on. Here 5 is the fifth field. LLVM works out
> the rest. This is better than casting the struct to char* and then
> doing pointer arithmetic.

OK. Yes.

>> Also, for the optimization, I wouldn't worry for now because:
>> - the linearized code is already optimized (nothing sophisticated but the
>>   essential is already there)
>> - you can always later call LLVM's optimization passes
>> - the code selector will also do some optimization.
>>
>
> Sure but my experience with LLVM is that it needs to have type
> metadata for each store and load instruction to properly do type based
> alias analysis. Otherwise it will miss optimisation opportunities.
> Maybe we can still provide type metadata as we do have the symbol
> definition etc.

There is certainly no reasons to not give easily available information.

> But I agree that if sparse-llvm works correctly for all inputs then
> that is a big step forward (for me) !

Indeed :)


Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 02, 2017 at 06:18:43PM +0100, Luc Van Oostenryck wrote:
> > > Here is the output from linearize. I think the way stores and loads
> > > are handled is broken. It appears that the last store / load
> > > instruction is stored in insn->target->priv, and then used later on
> > > ... I do not understand what the code is trying to do. Is it trying to
> > > optimize away stores and loads?
> 
> No, no.
> What is stored in ->priv is the target's  LLVMValueRef (and for a store
> the 'target' is what need to be stored).
> And indeed there is a bug there: it's target_in that should be stored in
> ->priv (in fact, for a store, there is no need to put anything at all
> in this field; at least I don't see any reason why it should).
> Nice catch.
> I don't know how it's related to your problem though.

OK, I've just checked and indeed removing this assignment to ->priv
in output_op_store() was wrong and is most probably very related to
your problem. I used something as simple as:
	void foo(int *p, int a, int b)
	{
		int c = a + b;
	
		p[0] = c;
		p[1] = c;
	}

Which returned:
	Stored value type does not match pointer operand type!
	  store void <badref>, i32* %8

And with this assignment removed this error is no more and the
generated LLVM IR is:
	define void @foo(i32*, i32, i32) {
	  %R3 = add i32 %1, %2
	  %3 = bitcast i32* %0 to i8*
	  %4 = getelementptr inbounds i8, i8* %3, i64 0
	  %5 = bitcast i8* %4 to i32*
	  store i32 %R3, i32* %5
	  %6 = bitcast i32* %0 to i8*
	  %7 = getelementptr inbounds i8, i8* %6, i64 4
	  %8 = bitcast i8* %7 to i32*
	  store i32 %R3, i32* %8
	  ret void
	}

And the generated x86 code is:
	addl	%edx, %esi
	movl	%esi, (%rdi)
	movl	%esi, 0x4(%rdi)
	retq


Luc Van Oostenryck

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

Hi Luc,

Next problem:

The following fails.

typedef unsigned long long size_t;
struct buffer_type_st {
 struct buffer_type_st    *next_buffer;
 char           *buffer;
};
typedef struct buffer_type_st buffer_type_t;
struct link_st {
 struct link_st           *next;
};
typedef struct link_st link_t;
struct allocator_st {
 buffer_type_t    *buffer_list;
 link_t           *free_list;
 char           *next_avail;
 char           *last;
 size_t          size;
 size_t          n;
};
typedef struct allocator_st allocator;
extern void *
alloc_node(allocator * a);
extern void
grow_allocator(allocator * a);
void *
alloc_node(allocator * a)
{
 link_t           *tmp;
 tmp = a->free_list;
 return (void *) tmp;
}


I get LLVM assertion failure for following instruction:

insn cast.64     %r4 <- (64) %r2

Assertion failed: isa<X>(Val) && "cast<Ty>() argument of incompatible
type!", file C:\d\llvm-3.9.0.src\include\llvm/Support/Casting.h, line
237

I think this is because the linearize is outputting an integer cast
rather than pointer cast so LLVM is not happy.

Regards
Dibyendu

p.s. should I start a new thread for each separate issue?

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 3 March 2017 at 02:52, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> Hi Luc,
>
> Next problem:
>
> The following fails.
>
> typedef unsigned long long size_t;
> struct buffer_type_st {
>  struct buffer_type_st    *next_buffer;
>  char           *buffer;
> };
> typedef struct buffer_type_st buffer_type_t;
> struct link_st {
>  struct link_st           *next;
> };
> typedef struct link_st link_t;
> struct allocator_st {
>  buffer_type_t    *buffer_list;
>  link_t           *free_list;
>  char           *next_avail;
>  char           *last;
>  size_t          size;
>  size_t          n;
> };
> typedef struct allocator_st allocator;
> extern void *
> alloc_node(allocator * a);
> extern void
> grow_allocator(allocator * a);
> void *
> alloc_node(allocator * a)
> {
>  link_t           *tmp;
>  tmp = a->free_list;
>  return (void *) tmp;
> }
>
>
> I get LLVM assertion failure for following instruction:
>
> insn cast.64     %r4 <- (64) %r2
>
> Assertion failed: isa<X>(Val) && "cast<Ty>() argument of incompatible
> type!", file C:\d\llvm-3.9.0.src\include\llvm/Support/Casting.h, line
> 237
>
> I think this is because the linearize is outputting an integer cast
> rather than pointer cast so LLVM is not happy.
>

Simpler example:

void *
alloc_node(void)
{
 char *tmp;
 tmp = (void *)0;
 return tmp;
}

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 3 March 2017 at 03:01, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
>> Next problem:
>>
>> The following fails.
>>
>> typedef unsigned long long size_t;
>> struct buffer_type_st {
>>  struct buffer_type_st    *next_buffer;
>>  char           *buffer;
>> };
>> typedef struct buffer_type_st buffer_type_t;
>> struct link_st {
>>  struct link_st           *next;
>> };
>> typedef struct link_st link_t;
>> struct allocator_st {
>>  buffer_type_t    *buffer_list;
>>  link_t           *free_list;
>>  char           *next_avail;
>>  char           *last;
>>  size_t          size;
>>  size_t          n;
>> };
>> typedef struct allocator_st allocator;
>> extern void *
>> alloc_node(allocator * a);
>> extern void
>> grow_allocator(allocator * a);
>> void *
>> alloc_node(allocator * a)
>> {
>>  link_t           *tmp;
>>  tmp = a->free_list;
>>  return (void *) tmp;
>> }
>>
>>
>> I get LLVM assertion failure for following instruction:
>>
>> insn cast.64     %r4 <- (64) %r2
>>
>> Assertion failed: isa<X>(Val) && "cast<Ty>() argument of incompatible
>> type!", file C:\d\llvm-3.9.0.src\include\llvm/Support/Casting.h, line
>> 237
>>
>> I think this is because the linearize is outputting an integer cast
>> rather than pointer cast so LLVM is not happy.
>>
>
> Simpler example:
>
> void *
> alloc_node(void)
> {
>  char *tmp;
>  tmp = (void *)0;
>  return tmp;
> }

I tried the following fix in sparse-llvm.c. It handles pointer casts
in output_op_cast. While this fixes the simpler example I get a
different error now on the larger sample.

static void output_op_cast(struct dmr_C *C, struct function *fn,
struct instruction *insn, LLVMOpcode op)
{
 LLVMValueRef src, target;
 char target_name[64];
 src = insn->src->priv;
 if (!src)
  src = pseudo_to_value(C, fn, insn, insn->src);
 pseudo_name(C, insn->target, target_name);
 assert(!symbol_is_fp_type(C, insn->type));
 if (LLVMGetTypeKind(LLVMTypeOf(src)) == LLVMPointerTypeKind) {
  target = LLVMBuildBitCast(fn->builder, src, insn_symbol_type(C,
fn->module, insn), target_name);
 }
 else {
  if (insn->size < LLVMGetIntTypeWidth(LLVMTypeOf(src)))
   target = LLVMBuildTrunc(fn->builder, src, insn_symbol_type(C,
fn->module, insn), target_name);
  else
   target = LLVMBuildCast(fn->builder, op, src, insn_symbol_type(C,
fn->module, insn), target_name);
 }
 insn->target->priv = target;
}

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Fri, Mar 03, 2017 at 02:52:00AM +0000, Dibyendu Majumdar wrote:
> I think this is because the linearize is outputting an integer cast
> rather than pointer cast so LLVM is not happy.

Yes, it's very possible.
As I said, in another mail yesterday, output_op_ptrcast()
need some changes. It makes the assumption that the cast
is always between type of the same size (and so generate
a LLVM's bitcast). But in sparse, OP_PTRCAST is used for
all cast to a non-void pointer type.

Luc

> p.s. should I start a new thread for each separate issue?

Yes, please.

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 2, 2017 at 7:02 PM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
>> In fact LLVMBuildStore() return a LLVMValueRef.
>> Could this be the 'void' we seen?
>>
>
> It returns a Value that is a store instruction, or at least that's the
> way I interpret this and so yes, I think the type of this was void.

I failed to find doc about it. But anwyay, overwritting the target->priv
can't be correct.

> BTW removing the line makes progress, but it fails at the add
> instruction later. Are there other issues like this you can spot? I am
> a bit lost here I am afraid.
No, not like this. What I saw was just the result of looking at your
example and trying to understand what's happening.

Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Fri, Mar 03, 2017 at 02:52:00AM +0000, Dibyendu Majumdar wrote:
> I get LLVM assertion failure for following instruction:
> 
> insn cast.64     %r4 <- (64) %r2
> 
> Assertion failed: isa<X>(Val) && "cast<Ty>() argument of incompatible
> type!", file C:\d\llvm-3.9.0.src\include\llvm/Support/Casting.h, line
> 237
> 
> I think this is because the linearize is outputting an integer cast
> rather than pointer cast so LLVM is not happy.

Yes.
Here I have something that talk more to me:
	ZExt only operates on integer
	  %R4 = zext %struct.link_st* %load_target to i8*
 

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 3 March 2017 at 04:27, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Fri, Mar 03, 2017 at 02:52:00AM +0000, Dibyendu Majumdar wrote:
>> I get LLVM assertion failure for following instruction:
>>
>> insn cast.64     %r4 <- (64) %r2
>>
>> Assertion failed: isa<X>(Val) && "cast<Ty>() argument of incompatible
>> type!", file C:\d\llvm-3.9.0.src\include\llvm/Support/Casting.h, line
>> 237
>>
>> I think this is because the linearize is outputting an integer cast
>> rather than pointer cast so LLVM is not happy.
>
> Yes.
> Here I have something that talk more to me:
>         ZExt only operates on integer
>           %R4 = zext %struct.link_st* %load_target to i8*
>

Also by casting to an integer subsequent operations fail. Any idea why
cast is being output rather than ptrcast?

Regards

[PATCH] llvm: fix output_op_[ptr]cast()

[Luc Van Oostenryck]

This should fox the problems of mixed types in casts.
Without much testing but should be essentialy OK.

CC: Dibyendu Majumdar <mobile@majumdar.org.uk>
---
 sparse-llvm.c | 40 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/sparse-llvm.c b/sparse-llvm.c
index edd0615ec..92ad26845 100644
--- a/sparse-llvm.c
+++ b/sparse-llvm.c
@@ -763,6 +763,8 @@ static void output_op_phi(struct function *fn, struct instruction *insn)
 static void output_op_ptrcast(struct function *fn, struct instruction *insn)
 {
 	LLVMValueRef src, target;
+	LLVMTypeRef dtype;
+	LLVMOpcode op;
 	char target_name[64];
 
 	src = insn->src->priv;
@@ -773,15 +775,31 @@ static void output_op_ptrcast(struct function *fn, struct instruction *insn)
 
 	assert(!symbol_is_fp_type(insn->type));
 
-	target = LLVMBuildBitCast(fn->builder, src, insn_symbol_type(fn->module, insn), target_name);
+	dtype = insn_symbol_type(fn->module, insn);
+	switch (LLVMGetTypeKind(LLVMTypeOf(src))) {
+	case LLVMPointerTypeKind:
+		op = LLVMBitCast;
+		break;
+	case LLVMIntegerTypeKind:
+		op = LLVMIntToPtr;
+		break;
+	default:
+		assert(0);
+	}
 
+	target = LLVMBuildCast(fn->builder, op, src, dtype, target_name);
 	insn->target->priv = target;
 }
 
 static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOpcode op)
 {
 	LLVMValueRef src, target;
+	LLVMTypeRef dtype;
 	char target_name[64];
+	unsigned int width;
+
+	if (is_ptr_type(insn->type))
+		return output_op_ptrcast(fn, insn);
 
 	src = insn->src->priv;
 	if (!src)
@@ -791,11 +809,23 @@ static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOp
 
 	assert(!symbol_is_fp_type(insn->type));
 
-	if (insn->size < LLVMGetIntTypeWidth(LLVMTypeOf(src)))
-		target = LLVMBuildTrunc(fn->builder, src, insn_symbol_type(fn->module, insn), target_name);
-	else
-		target = LLVMBuildCast(fn->builder, op, src, insn_symbol_type(fn->module, insn), target_name);
+	dtype = insn_symbol_type(fn->module, insn);
+	switch (LLVMGetTypeKind(LLVMTypeOf(src))) {
+	case LLVMPointerTypeKind:
+		op = LLVMPtrToInt;
+		break;
+	case LLVMIntegerTypeKind:
+		width = LLVMGetIntTypeWidth(LLVMTypeOf(src));
+		if (insn->size < width)
+			op = LLVMTrunc;
+		else if (insn->size == width)
+			op = LLVMBitCast;
+		break;
+	default:
+		assert(0);
+	}
 
+	target = LLVMBuildCast(fn->builder, op, src, dtype, target_name);
 	insn->target->priv = target;
 }
 
-- 
2.11.1

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Thu, Mar 02, 2017 at 06:41:35PM +0000, Dibyendu Majumdar wrote:
> This seems spot on as by making change as below, the final assertion
> failure went away. I have not yet checked the generated code but that
> is next.
> 
>  case OP_ADD:
>   if (symbol_is_fp_type(C, insn->type))
>    target = LLVMBuildFAdd(fn->builder, lhs, rhs, target_name);
>   else {
>    if (LLVMGetTypeKind(LLVMTypeOf(lhs)) == LLVMPointerTypeKind) {
>     target = LLVMBuildGEP(fn->builder, lhs, &rhs, 1, "");
>    }
>    else if (LLVMGetTypeKind(LLVMTypeOf(lhs)) == LLVMPointerTypeKind) {

Yes, this is what I meant but here you're testing twice the type of 'lhs'.
One of the test should be on 'rhs'.

Can you resend it in another thread once it's fixed?

Thanks,
Luc

Re: [PATCH] llvm: fix output_op_[ptr]cast()

[Dibyendu Majumdar]

Hi Luc,

On 3 March 2017 at 05:24, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> This should fox the problems of mixed types in casts.
> Without much testing but should be essentialy OK.
>
> CC: Dibyendu Majumdar <mobile@majumdar.org.uk>
> ---
>  sparse-llvm.c | 40 +++++++++++++++++++++++++++++++++++-----
>  1 file changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/sparse-llvm.c b/sparse-llvm.c
> index edd0615ec..92ad26845 100644
> --- a/sparse-llvm.c
> +++ b/sparse-llvm.c
> @@ -763,6 +763,8 @@ static void output_op_phi(struct function *fn, struct instruction *insn)
>  static void output_op_ptrcast(struct function *fn, struct instruction *insn)
>  {
>         LLVMValueRef src, target;
> +       LLVMTypeRef dtype;
> +       LLVMOpcode op;
>         char target_name[64];
>
>         src = insn->src->priv;
> @@ -773,15 +775,31 @@ static void output_op_ptrcast(struct function *fn, struct instruction *insn)
>
>         assert(!symbol_is_fp_type(insn->type));
>
> -       target = LLVMBuildBitCast(fn->builder, src, insn_symbol_type(fn->module, insn), target_name);
> +       dtype = insn_symbol_type(fn->module, insn);
> +       switch (LLVMGetTypeKind(LLVMTypeOf(src))) {
> +       case LLVMPointerTypeKind:
> +               op = LLVMBitCast;
> +               break;
> +       case LLVMIntegerTypeKind:
> +               op = LLVMIntToPtr;
> +               break;
> +       default:
> +               assert(0);
> +       }
>
> +       target = LLVMBuildCast(fn->builder, op, src, dtype, target_name);
>         insn->target->priv = target;
>  }
>
>  static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOpcode op)
>  {
>         LLVMValueRef src, target;
> +       LLVMTypeRef dtype;
>         char target_name[64];
> +       unsigned int width;
> +
> +       if (is_ptr_type(insn->type))
> +               return output_op_ptrcast(fn, insn);
>
>         src = insn->src->priv;
>         if (!src)
> @@ -791,11 +809,23 @@ static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOp
>
>         assert(!symbol_is_fp_type(insn->type));
>
> -       if (insn->size < LLVMGetIntTypeWidth(LLVMTypeOf(src)))
> -               target = LLVMBuildTrunc(fn->builder, src, insn_symbol_type(fn->module, insn), target_name);
> -       else
> -               target = LLVMBuildCast(fn->builder, op, src, insn_symbol_type(fn->module, insn), target_name);
> +       dtype = insn_symbol_type(fn->module, insn);
> +       switch (LLVMGetTypeKind(LLVMTypeOf(src))) {
> +       case LLVMPointerTypeKind:
> +               op = LLVMPtrToInt;
> +               break;
> +       case LLVMIntegerTypeKind:
> +               width = LLVMGetIntTypeWidth(LLVMTypeOf(src));
> +               if (insn->size < width)
> +                       op = LLVMTrunc;
> +               else if (insn->size == width)
> +                       op = LLVMBitCast;
> +               break;
> +       default:
> +               assert(0);
> +       }
>
> +       target = LLVMBuildCast(fn->builder, op, src, dtype, target_name);
>         insn->target->priv = target;
>  }
>


This doesn't quite work. The problem is that in op_cast, the pointer
is being cast to int, but subsequent operations expect a pointer.

Regards

Re: Sparse-LLVM issue compiling NULL pointers

[Luc Van Oostenryck]

On Fri, Mar 3, 2017 at 5:38 AM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
> Also by casting to an integer subsequent operations fail. Any idea why
> cast is being output rather than ptrcast?

Yes, it's because casts to void* is not considered as a pointer cast,
the rationale
being that a void* will need to be casted toa real pointer before being used.

Have you still problem after the patch I sent earlier (cfr:
https://patchwork.kernel.org/patch/9602045/ )?

Luc

Re: Sparse-LLVM issue compiling NULL pointers

[Dibyendu Majumdar]

On 3 March 2017 at 07:50, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Fri, Mar 3, 2017 at 5:38 AM, Dibyendu Majumdar
> <mobile@majumdar.org.uk> wrote:
>> Also by casting to an integer subsequent operations fail. Any idea why
>> cast is being output rather than ptrcast?
>
> Yes, it's because casts to void* is not considered as a pointer cast,
> the rationale
> being that a void* will need to be casted toa real pointer before being used.
>
> Have you still problem after the patch I sent earlier (cfr:
> https://patchwork.kernel.org/patch/9602045/ )?
>

I applied the patch but there is still the same problem as the op_cast
is converting a pointer to int, but then the following operation
expects a pointer.

Regards

Re: [PATCH] llvm: fix output_op_[ptr]cast()

[Dibyendu Majumdar]

On 3 March 2017 at 07:37, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> On 3 March 2017 at 05:24, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> This should fox the problems of mixed types in casts.
>> Without much testing but should be essentialy OK.
>>
>> CC: Dibyendu Majumdar <mobile@majumdar.org.uk>
>> ---
>>  sparse-llvm.c | 40 +++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/sparse-llvm.c b/sparse-llvm.c
>> index edd0615ec..92ad26845 100644
>> --- a/sparse-llvm.c
>> +++ b/sparse-llvm.c
>> @@ -763,6 +763,8 @@ static void output_op_phi(struct function *fn, struct instruction *insn)
>>  static void output_op_ptrcast(struct function *fn, struct instruction *insn)
>>  {
>>         LLVMValueRef src, target;
>> +       LLVMTypeRef dtype;
>> +       LLVMOpcode op;
>>         char target_name[64];
>>
>>         src = insn->src->priv;
>> @@ -773,15 +775,31 @@ static void output_op_ptrcast(struct function *fn, struct instruction *insn)
>>
>>         assert(!symbol_is_fp_type(insn->type));
>>
>> -       target = LLVMBuildBitCast(fn->builder, src, insn_symbol_type(fn->module, insn), target_name);
>> +       dtype = insn_symbol_type(fn->module, insn);
>> +       switch (LLVMGetTypeKind(LLVMTypeOf(src))) {
>> +       case LLVMPointerTypeKind:
>> +               op = LLVMBitCast;
>> +               break;
>> +       case LLVMIntegerTypeKind:
>> +               op = LLVMIntToPtr;
>> +               break;
>> +       default:
>> +               assert(0);
>> +       }
>>
>> +       target = LLVMBuildCast(fn->builder, op, src, dtype, target_name);
>>         insn->target->priv = target;
>>  }
>>
>>  static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOpcode op)
>>  {
>>         LLVMValueRef src, target;
>> +       LLVMTypeRef dtype;
>>         char target_name[64];
>> +       unsigned int width;
>> +
>> +       if (is_ptr_type(insn->type))
>> +               return output_op_ptrcast(fn, insn);
>>
>>         src = insn->src->priv;
>>         if (!src)
>> @@ -791,11 +809,23 @@ static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOp
>>
>>         assert(!symbol_is_fp_type(insn->type));
>>
>> -       if (insn->size < LLVMGetIntTypeWidth(LLVMTypeOf(src)))
>> -               target = LLVMBuildTrunc(fn->builder, src, insn_symbol_type(fn->module, insn), target_name);
>> -       else
>> -               target = LLVMBuildCast(fn->builder, op, src, insn_symbol_type(fn->module, insn), target_name);
>> +       dtype = insn_symbol_type(fn->module, insn);
>> +       switch (LLVMGetTypeKind(LLVMTypeOf(src))) {
>> +       case LLVMPointerTypeKind:
>> +               op = LLVMPtrToInt;
>> +               break;
>> +       case LLVMIntegerTypeKind:
>> +               width = LLVMGetIntTypeWidth(LLVMTypeOf(src));
>> +               if (insn->size < width)
>> +                       op = LLVMTrunc;
>> +               else if (insn->size == width)
>> +                       op = LLVMBitCast;
>> +               break;
>> +       default:
>> +               assert(0);
>> +       }
>>
>> +       target = LLVMBuildCast(fn->builder, op, src, dtype, target_name);
>>         insn->target->priv = target;
>>  }
>>
>
>
> This doesn't quite work. The problem is that in op_cast, the pointer
> is being cast to int, but subsequent operations expect a pointer.
>

The problem occurs in this sequence:

       ptrcast.64  %r26 <- (64) %r20
       add.64      %r27 <- %r26, %r23
       cast.64     %r28 <- (64) %r27
       store.64    %r28 -> 16[%arg1]

The last cast finds that the instruction type in an integer and does a
cast to Integer, so that causes the store to fail as it expects a
pointer.

Question in my mind is what is the result type of an add operation
when one of the arguments is a pointer?

Regards
Dibyendu

Re: [PATCH] llvm: fix output_op_[ptr]cast()

[Dibyendu Majumdar]

Perhaps we need these fixes?

https://patchwork.kernel.org/patch/8175541/

On 3 March 2017 at 18:06, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> On 3 March 2017 at 07:37, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
>> On 3 March 2017 at 05:24, Luc Van Oostenryck
>> <luc.vanoostenryck@gmail.com> wrote:
>>> This should fox the problems of mixed types in casts.
>>> Without much testing but should be essentialy OK.
>>>
>>> CC: Dibyendu Majumdar <mobile@majumdar.org.uk>
>>> ---
>>>  sparse-llvm.c | 40 +++++++++++++++++++++++++++++++++++-----
>>>  1 file changed, 35 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/sparse-llvm.c b/sparse-llvm.c
>>> index edd0615ec..92ad26845 100644
>>> --- a/sparse-llvm.c
>>> +++ b/sparse-llvm.c
>>> @@ -763,6 +763,8 @@ static void output_op_phi(struct function *fn, struct instruction *insn)
>>>  static void output_op_ptrcast(struct function *fn, struct instruction *insn)
>>>  {
>>>         LLVMValueRef src, target;
>>> +       LLVMTypeRef dtype;
>>> +       LLVMOpcode op;
>>>         char target_name[64];
>>>
>>>         src = insn->src->priv;
>>> @@ -773,15 +775,31 @@ static void output_op_ptrcast(struct function *fn, struct instruction *insn)
>>>
>>>         assert(!symbol_is_fp_type(insn->type));
>>>
>>> -       target = LLVMBuildBitCast(fn->builder, src, insn_symbol_type(fn->module, insn), target_name);
>>> +       dtype = insn_symbol_type(fn->module, insn);
>>> +       switch (LLVMGetTypeKind(LLVMTypeOf(src))) {
>>> +       case LLVMPointerTypeKind:
>>> +               op = LLVMBitCast;
>>> +               break;
>>> +       case LLVMIntegerTypeKind:
>>> +               op = LLVMIntToPtr;
>>> +               break;
>>> +       default:
>>> +               assert(0);
>>> +       }
>>>
>>> +       target = LLVMBuildCast(fn->builder, op, src, dtype, target_name);
>>>         insn->target->priv = target;
>>>  }
>>>
>>>  static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOpcode op)
>>>  {
>>>         LLVMValueRef src, target;
>>> +       LLVMTypeRef dtype;
>>>         char target_name[64];
>>> +       unsigned int width;
>>> +
>>> +       if (is_ptr_type(insn->type))
>>> +               return output_op_ptrcast(fn, insn);
>>>
>>>         src = insn->src->priv;
>>>         if (!src)
>>> @@ -791,11 +809,23 @@ static void output_op_cast(struct function *fn, struct instruction *insn, LLVMOp
>>>
>>>         assert(!symbol_is_fp_type(insn->type));
>>>
>>> -       if (insn->size < LLVMGetIntTypeWidth(LLVMTypeOf(src)))
>>> -               target = LLVMBuildTrunc(fn->builder, src, insn_symbol_type(fn->module, insn), target_name);
>>> -       else
>>> -               target = LLVMBuildCast(fn->builder, op, src, insn_symbol_type(fn->module, insn), target_name);
>>> +       dtype = insn_symbol_type(fn->module, insn);
>>> +       switch (LLVMGetTypeKind(LLVMTypeOf(src))) {
>>> +       case LLVMPointerTypeKind:
>>> +               op = LLVMPtrToInt;
>>> +               break;
>>> +       case LLVMIntegerTypeKind:
>>> +               width = LLVMGetIntTypeWidth(LLVMTypeOf(src));
>>> +               if (insn->size < width)
>>> +                       op = LLVMTrunc;
>>> +               else if (insn->size == width)
>>> +                       op = LLVMBitCast;
>>> +               break;
>>> +       default:
>>> +               assert(0);
>>> +       }
>>>
>>> +       target = LLVMBuildCast(fn->builder, op, src, dtype, target_name);
>>>         insn->target->priv = target;
>>>  }
>>>
>>
>>
>> This doesn't quite work. The problem is that in op_cast, the pointer
>> is being cast to int, but subsequent operations expect a pointer.
>>
>
> The problem occurs in this sequence:
>
>        ptrcast.64  %r26 <- (64) %r20
>        add.64      %r27 <- %r26, %r23
>        cast.64     %r28 <- (64) %r27
>        store.64    %r28 -> 16[%arg1]
>
> The last cast finds that the instruction type in an integer and does a
> cast to Integer, so that causes the store to fail as it expects a
> pointer.
>
> Question in my mind is what is the result type of an add operation
> when one of the arguments is a pointer?
>
> Regards
> Dibyendu

Re: [PATCH] llvm: fix output_op_[ptr]cast()

[Luc Van Oostenryck]

On Fri, Mar 3, 2017 at 7:06 PM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
> The problem occurs in this sequence:
>
>        ptrcast.64  %r26 <- (64) %r20
>        add.64      %r27 <- %r26, %r23
>        cast.64     %r28 <- (64) %r27
>        store.64    %r28 -> 16[%arg1]
>
> The last cast finds that the instruction type in an integer and does a
> cast to Integer, so that causes the store to fail as it expects a
> pointer.

What is the corresponding C code?
With the patches I've posted, the three example you've given are handled
without error for me.

> Question in my mind is what is the result type of an add operation
> when one of the arguments is a pointer?

This should only occurs as some form of pointer arithmetic, so the result
should be some kind of pointer.

Luc

Re: [PATCH] llvm: fix output_op_[ptr]cast()

[Dibyendu Majumdar]

On 3 March 2017 at 19:50, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Fri, Mar 3, 2017 at 7:06 PM, Dibyendu Majumdar
> <mobile@majumdar.org.uk> wrote:
>> The problem occurs in this sequence:
>>
>>        ptrcast.64  %r26 <- (64) %r20
>>        add.64      %r27 <- %r26, %r23
>>        cast.64     %r28 <- (64) %r27
>>        store.64    %r28 -> 16[%arg1]
>>
>> The last cast finds that the instruction type in an integer and does a
>> cast to Integer, so that causes the store to fail as it expects a
>> pointer.
>
> What is the corresponding C code?
> With the patches I've posted, the three example you've given are handled
> without error for me.
>

C code below, it is the last += that is failing.

typedef unsigned long long size_t;
struct buffer_type_st {
 struct buffer_type_st    *next_buffer;
 char           *buffer;
};
typedef struct buffer_type_st buffer_type_t;
struct link_st {
 struct link_st           *next;
};
typedef struct link_st link_t;
struct allocator_st {
 buffer_type_t    *buffer_list;
 link_t           *free_list;
 char           *next_avail;
 char           *last;
 size_t          size;
 size_t          n;
};
typedef struct allocator_st allocator;
extern void *
alloc_node(allocator * a);
extern void
grow_allocator(allocator * a);
void *
alloc_node(allocator * a)
{
 link_t           *tmp;
 tmp = a->free_list;
 if (a->free_list) {
  a->free_list = (link_t *) (a->free_list->next);
  return (void *) tmp;
 } else {
  if (a->next_avail == a->last) {
   grow_allocator(a);
  }
  {
   void           *tmp;
   tmp = (void *) a->next_avail;
   a->next_avail += a->size;
   return tmp;
  }
 }
}

Re: [PATCH] llvm: fix output_op_[ptr]cast()

[Luc Van Oostenryck]

On Fri, Mar 3, 2017 at 7:30 PM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
> Perhaps we need these fixes?
>
> https://patchwork.kernel.org/patch/8175541/

I very much doubt it would make any difference.
This patch is part of a serie enforcing correct handling
of constant expression *as defined by the standard*.
It won't change the type of an expression or something like this.

Luc

[PATCH] llvm: fix: do not mix pointers and floats when doing compares

[Luc Van Oostenryck]

In output_op_compare() everything that is not of interger
type is treated as floats, pointers disagree.

Fix this by rearranging the code and use a switch to easily
treat pointers like integers as required for LLVM's icmp.

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

diff --git a/sparse-llvm.c b/sparse-llvm.c
index 7c7b70027..be47c5e75 100644
--- a/sparse-llvm.c
+++ b/sparse-llvm.c
@@ -587,14 +587,27 @@ static void output_op_compare(struct function *fn, struct instruction *insn)
 
 	LLVMTypeRef dst_type = insn_symbol_type(insn);
 
-	if (LLVMGetTypeKind(LLVMTypeOf(lhs)) == LLVMIntegerTypeKind) {
+	switch  (LLVMGetTypeKind(LLVMTypeOf(lhs))) {
+	case LLVMPointerTypeKind:
+	case LLVMIntegerTypeKind: {
 		LLVMIntPredicate op = translate_op(insn->opcode);
 
 		target = LLVMBuildICmp(fn->builder, op, lhs, rhs, target_name);
-	} else {
+		break;
+	}
+	case LLVMHalfTypeKind:
+	case LLVMFloatTypeKind:
+	case LLVMDoubleTypeKind:
+	case LLVMX86_FP80TypeKind:
+	case LLVMFP128TypeKind:
+	case LLVMPPC_FP128TypeKind: {
 		LLVMRealPredicate op = translate_fop(insn->opcode);
 
 		target = LLVMBuildFCmp(fn->builder, op, lhs, rhs, target_name);
+		break;
+	}
+	default:
+		assert(0);
 	}
 
 	target = LLVMBuildZExt(fn->builder, target, dst_type, target_name);
-- 
2.11.1

Re: [PATCH] llvm: fix output_op_[ptr]cast()

[Christopher Li]

On Sat, Mar 4, 2017 at 3:55 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Fri, Mar 3, 2017 at 7:30 PM, Dibyendu Majumdar
> <mobile@majumdar.org.uk> wrote:
>> Perhaps we need these fixes?
>>
>> https://patchwork.kernel.org/patch/8175541/
>
> I very much doubt it would make any difference.
> This patch is part of a serie enforcing correct handling
> of constant expression *as defined by the standard*.
> It won't change the type of an expression or something like this.

I second that. Those patch series give warnings but it should not
affect the IR.

Chris