Re: sparse-llvm array size computation issue

[Dibyendu Majumdar <mobile@majumdar.org.uk>]

Hi Luc,

On 29 March 2017 at 12:32, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> On 28 March 2017 at 23:21, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>>> >> I think that there is a bug in examine_node_type() in symbol.c - it
>>> >> should set the base_type's bit_size perhaps? See the line marked as
>>> >> FIX below,
>>> >>
>>> >> /* Unsized array? The size might come from the initializer.. */
>>> >> if (bit_size < 0 && base_type->type == SYM_ARRAY) {
>>> >> struct expression *initializer = get_symbol_initializer(sym);
>>> >> if (initializer) {
>>> >> struct symbol *node_type = base_type->ctype.base_type;
>>> >> int count = count_array_initializer(S, node_type, initializer);
>>> >>
>>> >> if (node_type && node_type->bit_size >= 0)
>>> >> bit_size = array_element_offset(S->C->target, node_type->bit_size, count);
>>> >> base_type->bit_size = bit_size;   /*** FIX set base_type->bit_size ***/
>>> >> }
>>> >> }
>>> >
>>> > I'm far from sure.
>>> > Yes, here it will works but in general you have no idea who else is
>>> > using the base_type. For other users it may be legitimally still
>>> > be unsized.
>>> >
>>>
>>> I saw that if I set the array size in the C code then
>>> base_type->bit_size gets set. So my reasoning was that this is okay,
>>> as only the way the size is determined is changed, but the array still
>>> has a size. What do you think?
>>
>> Yes, that's normal.
>> The SYM_ARRAY has its size set if the size is given.
>>
>> Otherwise the size will:
>> -) stay unsized. For example, extern int array[] in an include
>>    Most files will just see it and use it like this, and only
>>    a single file will define the array and give it a size.
>> -) will receive a size later, at the end of the initialization.
>>    In this case, the array is given a SYM_NODE and the size
>>    is set there.
>>
>> Important information is stored in the SYM_NODE and it's wrong
>> to just jump over it.
>>
>
> Okay thank you for the insight. It seems then that sparse-llvm is not
> handing this correctly.
>

I looked at this again briefly today. I think that not having the size
information on the array type poses some problems.

+ will instructions have access to the SYM_NODE always? It doesn't
appear to be the case.
+ sparse-llvm caches the symbol's type in symbol->aux. For array
types, we would still need to do this - storing the type against
SYM_NODE objects is probably not correct.

So I feel that given that the size is an integral part of the array
type then it makes sense that it should be present on the array type.
In the testing I have done so far, this appears to work fine (although
I have not yet run the sparse tests - only my own tests).

Thanks and Regards
Dibyendu