> On Oct 23, 2023, at 2:06 PM, Martin Uecker <[email protected]> wrote:
>
> Am Montag, dem 23.10.2023 um 16:37 +0000 schrieb Qing Zhao:
>>
>>> On Oct 23, 2023, at 11:57 AM, Richard Biener <[email protected]>
>>> wrote:
>>>
>>>
>>>
>>>> Am 23.10.2023 um 16:56 schrieb Qing Zhao <[email protected]>:
>>>>
>>>>
>>>>
>>>>> On Oct 23, 2023, at 3:57 AM, Richard Biener <[email protected]>
>>>>> wrote:
>>>>>
>>>>>> On Fri, Oct 20, 2023 at 10:41 PM Qing Zhao <[email protected]> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On Oct 20, 2023, at 3:10 PM, Siddhesh Poyarekar <[email protected]>
>>>>>>> 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;
Is the above line should be:
char *p = &x.buf
?
> __bdos(p) -> N
>
> So we need to be smart on how we provide the size
> information for x->f to the backend.
Do you have any other suggestion here?
(Right now, what we’d like to do is to add one more argument for the function
__bdos as
__bdos (p, type, x.L))
>
> This would also be desirable for the language extension.
Yes.
Qing
>
> 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
