Re: HELP: Will the reordering happen? Re: [V3][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2023-10-23 15:43, Qing Zhao wrote:
> 
> 
>> On Oct 23, 2023, at 2:43 PM, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>>
>> On 2023-10-23 14:06, Martin Uecker wrote:
>>> 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.
>>
>> This is essentially why there need to be frontend rules constraining reordering and reachability semantics of x.L, thus restricting DSE and reordering for it.
> 
> My understanding is that Restricting DSE and reordering should be done by the proper data flow information, with a new argument added to the BDOS call, this correct data flow information could be maintained, and then the DSE and reordering will not happen.
> 
> I don’t quite understand what kind of frontend rules should be added to constrain reordering and reachability semantics? Can you explain this a little bit more? Do you mean to add some rules or requirment to the new attribute that the users of the attribute should follow in the source code?

Yes, but let me try and summarize the issues and the potential solutions 
at the end:

> 
>>   This is not really a __bdos/__bos question, because that bit is trivial; if the structure is visible, the value is simply x.L.  This is also why adding a reference to x.L in __bos/__bdos is not sufficient or even possible in, e.g. the above case you note.
> 
> I am a little confused here, are we discussing how to resolve the potential reordering issue of the following:
> 
> "
> struct annotated {
>    size_t foo;
>    char array[] __attribute__((counted_by (foo)));
> };
> 
>    p->foo = 10;
>    size = __builtin_dynamic_object_size (p->array,1);
> “?
> 
> Or a bigger issue?

Right, so the problem we're trying to solve is the reordering of __bdos 
w.r.t. initialization of the size parameter but to also account for DSE 
of the assignment, we can abstract this problem to that of DFA being 
unable to see implicit use of the size parameter.  __bdos is the one 
such implicit user of the size parameter and you're proposing to solve 
this by encoding the relationship between buffer and size at the __bdos 
call site.  But what about the case when the instantiation of the object 
is not at the same place as the __bdos call site, i.e. the DFA is unable 
to make that relationship?

The example Martin showed where the subobject gets "hidden" behind a 
pointer was a trivial one where DFA *may* actually work in practice 
(because the object-size pass can thread through these assignments) but 
think about this one:

struct A
{
   size_t size;
   char buf[] __attribute__((counted_by(size)));
}

static size_t
get_size_of (void *ptr)
{
   return __bdos (ptr, 1);
}

void
foo (size_t sz)
{
   struct A *obj = __builtin_malloc (sz);
   obj->size = sz;

   ...
   __builtin_printf ("%zu\n", get_size_of (obj->array));
   ...
}

Until get_size_of is inlined, no DFA can see the __bdos call in the same 
place as the point where obj is allocated.  As a result, the assignment 
to obj->size could get reordered (or the store eliminated) w.r.t. the 
__bdos call until the inlining happens.

As a result, the relationship between buf and size established by the 
attribute needs to be encoded into the type somehow.  There are two options:

Option 1: Encode the relationship in the type of buf

This is kinda what you end up doing with component_ref_has_counted_by 
and it does show the relationship if one is looking (through that call), 
but nothing more that can be used to, e.g. prevent reordering or tell 
the optimizer that the reference to the buf member may imply a reference 
to the size member as well.  This could be remedied by somehow encoding 
the USES relationship for size into the type of buf that the 
optimization passes can see.  I feel like this may be a bit convoluted 
to specify in a future language extension in a way that will actually be 
well understood by developers, but it will likely generate faster 
runtime code.  This will also likely require a bigger change across passes.

Option 2: Encode the relationship in the type of size

The other option is to enhance the type of size somehow so that it 
discourages reordering and store elimination, basically pessimizing 
code.  I think volatile semantics might be the way to do this and may 
even be straightforward to specify in the future language extension 
given that it builds on a known language construct and is thematically 
related.  However it does pessimize output for code that implements 
__counted_by__.

Thanks,
Sid