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. 

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
> > > 
> 

Reply via email to