> Am 20.10.2023 um 19:09 schrieb Qing Zhao <qing.z...@oracle.com>:
>
> Sid,
>
> (Richard, can you please help me to make sure this? Thanks a lot)
>
> I studied a little bit more on the following question you raised during the
> review process:
>
> For the following small testing case:
>
> 1 struct annotated {
> 2 int foo;
> 3 char array[] __attribute__((counted_by (foo)));
> 4 };
> 5
> 6 extern struct annotated * alloc_buf (int);
> 7
> 8 int test (int sz)
> 9 {
> 10 struct annotated * array_annotated = alloc_buf (sz);
> 11 array_annotated->foo = sz;
> 12 return __builtin_dynamic_object_size (array_annotated->array, 1);
> 13 }
>
> Whether the assignment of the size to the counted_by field at line 11 and the
> consumer of the size at line 12 at call to __bdos might be reordered by GCC?
>
> The following is my thought:
>
> 1. _bdos computation passes (both pass_early_object_sizes and
> pass_object_sizes) are in the early stage of SSA optimizations. In which,
> pass_early_object_sizes happens before almost all the optimizations, no
> reordering is possible in this pass;
>
> 2. Then how about the pass “pass_object_sizes”?
>
> Immediately after the pass_build_ssa, the IR for the routine “test” is
> with the SSA form: (compiled with -O3):
>
> 1 int test (int sz)
> 2 {
> 3 struct annotated * array_annotated;
> 4 char[0:] * _1;
> 5 long unsigned int _2;
> 6 int _8;
> 7
> 8 <bb 2> :
> 9 array_annotated_6 = alloc_buf (sz_4(D));
> 10 array_annotated_6->foo = sz_4(D);
> 11 _1 = &array_annotated_6->array;
> 12 _2 = __builtin_dynamic_object_size (_1, 1);
> 13 _8 = (int) _2;
> 14 return _8;
> 15 }
>
> In the above IR, the key portion is line 10 and line 11: (whether these two
> lines might be reordered with SSA optimization?)
>
> 10 array_annotated_6->foo = sz_4(D);
> 11 _1 = &array_annotated_6->array;
>
> The major question here is: whether the SSA optimizations are able to
> distinguish the object “array_annotated_6->foo” at line 10 is independent with
> the object “array_annotated-_6->array” at line 11?
>
> If the SSA optimizations can distinguish “array_annotated_6->foo” from
> “array_annotated_6->array”, then these two lines might be reordered.
> Otherwise, these two lines will not be reordered by SSA optimizations.
>
> I am not very familiar with the details of the SSA optimizations, but my
> guess is, two fields of the same structure might not be distinguished by the
> SSA optimizations, then line 10 and line 11 will not be reordered by SSA
> optimizations.
>
> Richard, is my guess correct?
There is no data dependence between the memory access and the address
computation so nothing prevents the reordering. If you put another same bos
call before the access I expect the addresses to be CSEd, effectively moving
the later before the access.
Richard
> Thanks a lot for your help.
>
> Qing
>
>>>> On Oct 5, 2023, at 4:08 PM, Siddhesh Poyarekar <siddh...@gotplt.org> wrote:
>>>
>>> I hope the review was helpful. Overall, a couple of things to consider:
>>>
>>> 1. How would you handle potential reordering between assignment of the size
>>> to the counted_by field with the __bdos call that may consume it? You'll
>>> probably need to express some kind of dependency there or in the worst
>>> case, insert a barrier to disallow reordering.
>>
>> Good point!
>>
>> So, your example in the respond to [V3][PATCH 2/3]Use the counted_by
>> atribute info in builtin object size [PR108896]:
>> “
>> Maybe another test where the allocation, size assignment and __bdos call
>> happen in the same function, where the allocator is not recognized by gcc:
>>
>> void *
>> __attribute__ ((noinline))
>> alloc (size_t sz)
>> {
>> return __builtin_malloc (sz);
>> }
>>
>> void test (size_t sz)
>> {
>> array_annotated = alloc (sz);
>> array_annotated->b = sz;
>> return __builtin_dynamic_object_size (array_annotated->c, 1);
>> }
>>
>> The interesting thing to test (and ensure in the codegen) is that the
>> assignment to array_annotated->b does not get reordered to below the
>> __builtin_dynamic_object_size call since technically there is no data
>> dependency between the two.
>> “
>> Will test on this.
>>
>> Not sure whether the current GCC alias analysis is able to distinguish one
>> field of a structure from another field of the same structure, if YES, then
>> We need to add an explicit dependency edge from the write to
>> “array_annotated->b” to the call to
>> “__builtin_dynamic_object_size(array_annotated->c,1)”.
>> I will check on this and see how to resolve this issue.
>>
>> I guess the possible solution is that we can add an implicit ref to
>> “array_annotated->b” at the call to
>> “__builtin_dynamic_object_size(array_annotated->c, 1)” if the counted_by
>> attribute is available. That should resolve the issue.
>>
>> Richard, what do you think on this?
>>
>
