sparse-llvm incorrect type when ealing with union type

sparse-llvm incorrect type when ealing with union type

[Dibyendu Majumdar]

Hi,

I am investigating a type error in LLVM. Here is the snippet of code:

typedef union GCObject GCObject;
typedef unsigned char lu_byte;
typedef double lua_Number;
typedef union {
 GCObject *gc;
 void *p;
 lua_Number n;
 int b;
} Value;
typedef struct lua_TValue {
 Value value;
 int tt;
} TValue;
typedef struct UpVal {
 GCObject *next;
 lu_byte tt;
 lu_byte marked;
 TValue *v;
 union {
  TValue value;
  struct {
   struct UpVal *prev;
   struct UpVal *next;
  } l;
 } u;
} UpVal;
static void close(UpVal *uv)
{
 uv->v = &uv->u.value;
}

The linearized output is:

close:
.L0:
        <entry-point>
        add.64      %r2 <- %arg1, $24
        store.64    %r2 -> 16[%arg1]
        ret

The result of the add operation ends up the same type as arg1 which is
UpVal*. But this is then assigned to uv->v which is expected to be
TValue*. So LLVM doesn't like it.

While in this case we could cast to the expected value before the
store - I am worried that in other cases where the value is read and
used somewhere it will have the wrong type.

I see that the add instruction's type is 'void *'.

Any suggestions on how to solve this issue?

Thanks and Regards
Dibyendu

Re: sparse-llvm incorrect type when ealing with union type

[Dibyendu Majumdar]

On 13 March 2017 at 22:54, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> I am investigating a type error in LLVM. Here is the snippet of code:
>
> typedef union GCObject GCObject;
> typedef unsigned char lu_byte;
> typedef double lua_Number;
> typedef union {
>  GCObject *gc;
>  void *p;
>  lua_Number n;
>  int b;
> } Value;
> typedef struct lua_TValue {
>  Value value;
>  int tt;
> } TValue;
> typedef struct UpVal {
>  GCObject *next;
>  lu_byte tt;
>  lu_byte marked;
>  TValue *v;
>  union {
>   TValue value;
>   struct {
>    struct UpVal *prev;
>    struct UpVal *next;
>   } l;
>  } u;
> } UpVal;
> static void close(UpVal *uv)
> {
>  uv->v = &uv->u.value;
> }
>
> The linearized output is:
>
> close:
> .L0:
>         <entry-point>
>         add.64      %r2 <- %arg1, $24
>         store.64    %r2 -> 16[%arg1]
>         ret
>
> The result of the add operation ends up the same type as arg1 which is
> UpVal*. But this is then assigned to uv->v which is expected to be
> TValue*. So LLVM doesn't like it.
>
> While in this case we could cast to the expected value before the
> store - I am worried that in other cases where the value is read and
> used somewhere it will have the wrong type.
>
> I see that the add instruction's type is 'void *'.
>
> Any suggestions on how to solve this issue?
>

For now I am casting the stored value in output_op_store():

 target_in = pseudo_to_value(C, fn, insn, insn->target);
 desttype = insn_symbol_type(C, fn->module, insn);

 /* Cast to the right type - to resolve issue with union types */
 target_in = LLVMBuildBitCast(fn->builder, target_in, desttype, "");
 /* perform store */
 LLVMBuildStore(fn->builder, target_in, addr);

Re: sparse-llvm incorrect type when ealing with union type

[Dibyendu Majumdar]

On 13 March 2017 at 23:44, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
>> I am investigating a type error in LLVM. Here is the snippet of code:
>>
>> typedef union GCObject GCObject;
>> typedef unsigned char lu_byte;
>> typedef double lua_Number;
>> typedef union {
>>  GCObject *gc;
>>  void *p;
>>  lua_Number n;
>>  int b;
>> } Value;
>> typedef struct lua_TValue {
>>  Value value;
>>  int tt;
>> } TValue;
>> typedef struct UpVal {
>>  GCObject *next;
>>  lu_byte tt;
>>  lu_byte marked;
>>  TValue *v;
>>  union {
>>   TValue value;
>>   struct {
>>    struct UpVal *prev;
>>    struct UpVal *next;
>>   } l;
>>  } u;
>> } UpVal;
>> static void close(UpVal *uv)
>> {
>>  uv->v = &uv->u.value;
>> }
>>
>> The linearized output is:
>>
>> close:
>> .L0:
>>         <entry-point>
>>         add.64      %r2 <- %arg1, $24
>>         store.64    %r2 -> 16[%arg1]
>>         ret
>>
>> The result of the add operation ends up the same type as arg1 which is
>> UpVal*. But this is then assigned to uv->v which is expected to be
>> TValue*. So LLVM doesn't like it.
>>
>> While in this case we could cast to the expected value before the
>> store - I am worried that in other cases where the value is read and
>> used somewhere it will have the wrong type.
>>
>> I see that the add instruction's type is 'void *'.
>>
>> Any suggestions on how to solve this issue?
>>
>
> For now I am casting the stored value in output_op_store():
>
>  target_in = pseudo_to_value(C, fn, insn, insn->target);
>  desttype = insn_symbol_type(C, fn->module, insn);
>
>  /* Cast to the right type - to resolve issue with union types */
>  target_in = LLVMBuildBitCast(fn->builder, target_in, desttype, "");
>  /* perform store */
>  LLVMBuildStore(fn->builder, target_in, addr);

Additionally also casting the stores to phi instructions.

Regards

Re: sparse-llvm incorrect type when ealing with union type

[Luc Van Oostenryck]

On Mon, Mar 13, 2017 at 10:54:01PM +0000, Dibyendu Majumdar wrote:
> Hi,
> 
> I am investigating a type error in LLVM. Here is the snippet of code:

This seems to be fixed in the llvm-fixes-v3 serie I submitted yesterday.
But I never saw exactly what was the specific problem here.

-- Luc

Re: sparse-llvm incorrect type when ealing with union type

[Dibyendu Majumdar]

Hi Luc,

On 19 March 2017 at 15:36, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Mon, Mar 13, 2017 at 10:54:01PM +0000, Dibyendu Majumdar wrote:
>> I am investigating a type error in LLVM. Here is the snippet of code:
>
> This seems to be fixed in the llvm-fixes-v3 serie I submitted yesterday.
> But I never saw exactly what was the specific problem here.
>

You will need to run code that uses union types extensively. For
example please try to compile:

https://github.com/dibyendumajumdar/dmr_c/blob/master/tests/minilua/minilua.c

Regards
Dibyendu

Re: sparse-llvm incorrect type when ealing with union type

[Dibyendu Majumdar]

Hi Luc,

On 19 March 2017 at 15:36, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> This seems to be fixed in the llvm-fixes-v3 serie I submitted yesterday.
> But I never saw exactly what was the specific problem here.
>

I noticed that in the latest patch series for add/sub you are not
using GEP anymore, and there is a conversion to target type at the end
of a binary op. I will try out these changes and report back.

Thanks and Regards
Dibyendu

Re: sparse-llvm incorrect type when ealing with union type

[Luc Van Oostenryck]

On Sun, Mar 19, 2017 at 5:37 PM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
> Hi Luc,
>
> On 19 March 2017 at 15:36, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>>
>> This seems to be fixed in the llvm-fixes-v3 serie I submitted yesterday.
>> But I never saw exactly what was the specific problem here.
>>
>
> I noticed that in the latest patch series for add/sub you are not
> using GEP anymore, and there is a conversion to target type at the end
> of a binary op. I will try out these changes and report back.

Yes. It's not that the GEP was wrong but it was not enough and the more
complete solution made this one unneeded.

Luc

Re: sparse-llvm incorrect type when ealing with union type

[Dibyendu Majumdar]

Hi Luc,

On 19 March 2017 at 17:15, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Sun, Mar 19, 2017 at 5:37 PM, Dibyendu Majumdar
> <mobile@majumdar.org.uk> wrote:
>> On 19 March 2017 at 15:36, Luc Van Oostenryck
>> <luc.vanoostenryck@gmail.com> wrote:
>>>
>> I noticed that in the latest patch series for add/sub you are not
>> using GEP anymore, and there is a conversion to target type at the end
>> of a binary op. I will try out these changes and report back.
>
> Yes. It's not that the GEP was wrong but it was not enough and the more
> complete solution made this one unneeded.
>

I have now tested the changes to binary op implementation - and I can
confirm that all my tests pass, and this appears to be a better
approach than the previous one that used the GEP instruction.

Thanks and Regards
Dibyendu

Re: sparse-llvm incorrect type when ealing with union type

[Dibyendu Majumdar]

Hi Luc,

On 19 March 2017 at 17:55, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> On 19 March 2017 at 17:15, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> On Sun, Mar 19, 2017 at 5:37 PM, Dibyendu Majumdar
>> <mobile@majumdar.org.uk> wrote:
>>> On 19 March 2017 at 15:36, Luc Van Oostenryck
>>> <luc.vanoostenryck@gmail.com> wrote:
>>>>
>>> I noticed that in the latest patch series for add/sub you are not
>>> using GEP anymore, and there is a conversion to target type at the end
>>> of a binary op. I will try out these changes and report back.
>>
>> Yes. It's not that the GEP was wrong but it was not enough and the more
>> complete solution made this one unneeded.
>>
>
> I have now tested the changes to binary op implementation - and I can
> confirm that all my tests pass, and this appears to be a better
> approach than the previous one that used the GEP instruction.
>

It seems that we can replace the other GEP too with pointer
arithmetic. This makes LLVM output closer to the IR generated by
sparse.

The change required is in calc_memop_addr():

 /* int type large enough to hold a pointer */
 int_type = LLVMIntType(bits_in_pointer);
 /* IMPORTANT offset can be negative */
 off = LLVMConstInt(int_type, (int) insn->offset, 0);
 /* convert src to the effective pointer type */
 src = pseudo_to_value(fn, insn, insn->src);
 as = LLVMGetPointerAddressSpace(LLVMTypeOf(src));
 addr_type = LLVMPointerType(insn_symbol_type(insn), as);
 src = value_to_ivalue(fn, src);
 addr = LLVMBuildAdd(fn->builder, src, off, "");
 addr = LLVMBuildIntToPtr(fn->builder, addr, addr_type, "");
 return addr;


Regards
Dibyendu