> On Oct 23, 2023, at 2:31 PM, Martin Uecker <uec...@tugraz.at> wrote:
>
> Am Montag, dem 23.10.2023 um 20:06 +0200 schrieb Martin Uecker:
>> Am Montag, dem 23.10.2023 um 16:37 +0000 schrieb Qing Zhao:
>>>
>>>> On Oct 23, 2023, at 11:57 AM, Richard Biener <richard.guent...@gmail.com>
>>>> wrote:
>>>>
>>>>
>>>>
>>>>> Am 23.10.2023 um 16:56 schrieb Qing Zhao <qing.z...@oracle.com>:
>>>>>
>>>>>
>>>>>
>>>>>> On Oct 23, 2023, at 3:57 AM, Richard Biener <richard.guent...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> On Fri, Oct 20, 2023 at 10:41 PM Qing Zhao <qing.z...@oracle.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> On Oct 20, 2023, at 3:10 PM, Siddhesh Poyarekar <siddh...@gotplt.org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> On 2023-10-20 14:38, Qing Zhao wrote:
>>>>>>>>> How about the following:
>>>>>>>>> Add one more parameter to __builtin_dynamic_object_size(), i.e
>>>>>>>>> __builtin_dynamic_object_size (_1,1,array_annotated->foo)?
>>>>>>>>> When we see the structure field has counted_by attribute.
>>>>>>>>
>>>>>>>> Or maybe add a barrier preventing any assignments to
>>>>>>>> array_annotated->foo from being reordered below the __bdos call?
>>>>>>>> Basically an __asm__ with array_annotated->foo in the clobber list
>>>>>>>> ought to do it I think.
>>>>>>>
>>>>>>> Maybe just adding the array_annotated->foo to the use list of the call
>>>>>>> to __builtin_dynamic_object_size should be enough?
>>>>>>>
>>>>>>> But I am not sure how to implement this in the TREE level, is there a
>>>>>>> USE_LIST/CLOBBER_LIST for each call? Then I can just simply add the
>>>>>>> counted_by field “array_annotated->foo” to the USE_LIST of the call to
>>>>>>> __bdos?
>>>>>>>
>>>>>>> This might be the simplest solution?
>>>>>>
>>>>>> If the dynamic object size is derived of a field then I think you need to
>>>>>> put the "load" of that memory location at the point (as argument)
>>>>>> of the __bos call right at parsing time. I know that's awkward because
>>>>>> you try to play tricks "discovering" that field only late, but that's not
>>>>>> going to work.
>>>>>
>>>>> Is it better to do this at gimplification phase instead of FE?
>>>>>
>>>>> VLA decls are handled in gimplification phase, the size calculation and
>>>>> call to alloca are all generated during this phase. (gimplify_vla_decl).
>>>>>
>>>>> For __bdos calls, we can add an additional argument if the object’s first
>>>>> argument’s type include the counted_by attribute, i.e
>>>>>
>>>>> ***During gimplification,
>>>>> For a call to __builtin_dynamic_object_size (ptr, type)
>>>>> Check whether the type of ptr includes counted_by attribute, if so,
>>>>> change the call to
>>>>> __builtin_dynamic_object_size (ptr, type, counted_by field)
>>>>>
>>>>> Then the correct data dependence should be represented well in the IR.
>>>>>
>>>>> **During object size phase,
>>>>>
>>>>> The call to __builtin_dynamic_object_size will become an expression
>>>>> includes the counted_by field or -1/0 when we cannot decide the size, the
>>>>> correct data dependence will be kept even the call to
>>>>> __builtin_dynamic_object_size is gone.
>>>>
>>>> But the whole point of the BOS pass is to derive information that is not
>>>> available at parsing time, and that’s the cases you are after. The case
>>>> where the connection to the field with the length is apparent during
>>>> parsing is easy - you simply insert a load of the value before the BOS
>>>> call.
>>>
>>> Yes, this is true.
>>> I prefer to implement this in gimplification phase since I am more familiar
>>> with the code there.. (I think that implementing it in gimplification
>>> should be very similar as implementing it in FE? Or do I miss anything
>>> here?)
>>>
>>> Joseph, if implement this in FE, where in the FE I should look at?
>>>
>>
>> We should aim for a good integration with the BDOS pass, so
>> that it can propagate the information further, e.g. the
>> following should work:
>>
>> struct { int L; char buf[] __counted_by(L) } x;
>> x.L = N;
>> x.buf = ...;
>> char *p = &x->f;
>> __bdos(p) -> N
>>
>> So we need to be smart on how we provide the size
>> information for x->f to the backend.
>
> To follow up on this. I do not think we should change the
> builtin in the FE or gimplification. Instead, we want
> to change the field access and compute the size there.
Could you please clarify on this? What do you mean by "change the field access
and compute the size there”?
>
> In my toy patch I then made this have a VLA type that
> encodes the size. Here, this would need to be done
> differently.
>
> But still, what we are missing in both cases
> is a proper way to pass the information down to BDOS.
What’ s the issue with adding a new argument (x.L) to the BDOS call? What’s
missing with this approach?
>
> For VLAs this works because BDOS can see the size of
> the definition. For calls to allocation functions
> it is read from an attribute.
You mean for VLA, BDOS see the size of the definition from the attribute for
the allocation function?
Yes, that’s the case for VLA.
For VLA, the size computation and storage allocation are all done by the
compiler (through “gimplify_vla_decl” in gimplification phase),
So these two can be tied together by the compiler.
However, for FMA with counted_by attribute, the storage allocation and the
counted_by assignment are done by the user.
Qing
>
> But I am not sure what would be the best way to encode
> this information so that BDOS can later access it.
>
> Martin
>
>
>
>
>>
>> This would also be desirable for the language extension.
>>
>> Martin
>>
>>
>>> Thanks a lot for the help.
>>>
>>> Qing
>>>
>>>> For the late case there’s no way to invent data flow dependence without
>>>> inadvertently pessimizing optimization.
>>>>
>>>> Richard
>>>>
>>>>>
>>>>>>
>>>>>> A related issue is that assignment to the field and storage allocation
>>>>>> are not tied together
>>>>>
>>>>> Yes, this is different from VLA, in which, the size assignment and the
>>>>> storage allocation are generated and tied together by the compiler.
>>>>>
>>>>> For the flexible array member, the storage allocation and the size
>>>>> assignment are all done by the user. So, We need to clarify such
>>>>> requirement in the document to guide user to write correct code. And
>>>>> also, we might need to provide tools (warnings and sanitizer option) to
>>>>> help users to catch such coding error.
>>>>>
>>>>>> - if there's no use of the size data we might
>>>>>> remove the store of it as dead.
>>>>>
>>>>> Yes, when __bdos cannot decide the size, we need to remove the dead store
>>>>> to the field.
>>>>> I guess that the compiler should be able to do this automatically?
>>>>>
>>>>> thanks.
>>>>>
>>>>> Qing
>>>>>>
>>>>>> Of course I guess __bos then behaves like sizeof ().
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>>
>>>>>>> Qing
>>>>>>>
>>>>>>>>
>>>>>>>> It may not work for something like this though:
>>>>>>>>
>>>>>>>> static size_t
>>>>>>>> get_size_of (void *ptr)
>>>>>>>> {
>>>>>>>> return __bdos (ptr, 1);
>>>>>>>> }
>>>>>>>>
>>>>>>>> void
>>>>>>>> foo (size_t sz)
>>>>>>>> {
>>>>>>>> array_annotated = __builtin_malloc (sz);
>>>>>>>> array_annotated = sz;
>>>>>>>>
>>>>>>>> ...
>>>>>>>> __builtin_printf ("%zu\n", get_size_of (array_annotated->foo));
>>>>>>>> ...
>>>>>>>> }
>>>>>>>>
>>>>>>>> because the call to get_size_of () may not have been inlined that
>>>>>>>> early.
>>>>>>>>
>>>>>>>> The more fool-proof alternative may be to put a compile time barrier
>>>>>>>> right below the assignment to array_annotated->foo; I reckon you could
>>>>>>>> do that early in the front end by marking the size identifier and then
>>>>>>>> tracking assignments to that identifier. That may have a slight
>>>>>>>> runtime performance overhead since it may prevent even legitimate
>>>>>>>> reordering. I can't think of another alternative at the moment...
>>>>>>>>
>>>>>>>> Sid
