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;
__bdos(p) -> N
So we need to be smart on how we provide the size
information for x->f to the backend.
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
> > >
>
