> On Dec 3, 2024, at 10:07, Martin Uecker <uec...@tugraz.at> wrote:
>
> Am Dienstag, dem 03.12.2024 um 14:31 +0000 schrieb Qing Zhao:
>>
>>> On Dec 3, 2024, at 01:33, Martin Uecker <uec...@tugraz.at> wrote:
>>>
>>> Am Montag, dem 02.12.2024 um 22:58 +0000 schrieb Qing Zhao:
>>>>
>>>>> On Dec 2, 2024, at 16:13, Martin Uecker <uec...@tugraz.at> wrote:
>>>>>
>>>>> Am Montag, dem 02.12.2024 um 20:15 +0000 schrieb Qing Zhao:
>>>>>>
>>>>>>> On Nov 30, 2024, at 07:22, Martin Uecker <uec...@tugraz.at> wrote:
>>>>>>>
>>>>>>> Am Dienstag, dem 26.11.2024 um 20:59 +0000 schrieb Qing Zhao:
>>>>>>>> Think this over these days, I have another thought that need some
>>>>>>>> feedback:
>>>>>>>>
>>>>>>>> The major issue right now is:
>>>>>>>>
>>>>>>>> 1. For the following structure in which the “counted_by” attributes is
>>>>>>>> attached to the pointer field.
>>>>>>>>
>>>>>>>> struct foo {
>>>>>>>> int n;
>>>>>>>> char *p __attribute__ ((counted_by (n)));
>>>>>>>> } *x;
>>>>>>>
>>>>>>> BTW: I also do not like the syntax 'n' for a lookup of a member
>>>>>>> in the member namespace. I think it should be '.n'. For FAM this
>>>>>>> is less problematic because it always at the end, but here it is
>>>>>>> problematic.
>>>>>> During my implementation of extending this attribute to pointer fields
>>>>>> of structure.
>>>>>> I didn’t notice issue with the current syntax ’n’ for the pointer
>>>>>> fields so far,
>>>>>> even though when the field “n” is declared after the corresponding
>>>>>> pointer field, i.e:
>>>>>>
>>>>>> struct foo {
>>>>>> {
>>>>>> char *p __attribute__ ((counted_by (n)));
>>>>>> int n;
>>>>>> }
>>>>>>
>>>>>> So, could you please explain a little bit more on what’s the potential
>>>>>> issue here?
>>>>>
>>>>> My issue with it is that it is not consistent to how C's scoping
>>>>> of identifiers work. In
>>>>>
>>>>> constexpr int n = 3;
>>>>> struct foo {
>>>>> {
>>>>> char (*p)[n] __attribute__ ((counted_by (n))
>>>>> int n;
>>>>> }
>>>>>
>>>>> the n in the type of p refers to the previous n in scope while
>>>>> the n in your attribute would refer to the member.
>>>>
>>>> Okay, I see your point here -:).
>>>>
>>>> Yes, I agree that if the compiler can accept the syntax “.n” in general,
>>>> then it will make the
>>>> “counted_by” attribute to allow more complicated expressions in general.
>>>>
>>>> We had this similar discussion before the design and implementation for
>>>> the “counted_by” attribute
>>>> on FAMs, and we agreed to delay the approach of accepting the syntax “.n”
>>>> in the future possible
>>>> language standard at that time.
>>>>
>>>> So, for the “counted_by attribute on FAMs, the implementation is,
>>>> searching the “n” in all the fields of the
>>>> containing structure and locating that specific field.
>>>>
>>>> Now, when extending “counted_by” attribute to pointer fields of
>>>> structures, the implementation is similar.
>>>
>>>>>
>>>>> This is incoherent and confusing. It becomes worse should
>>>>> you ever want to allow more complicated expressions.
>>>>
>>>> You are right, it’s hard to allow more complicated expressions for
>>>> “counted_by” based on the current
>>>> design.
>>>>
>>>> If we agree to accept the “.n” syntax in GCC in general, that’s of course
>>>> better.
>>>> Then how about the current “counted_by” for FAMs? Shall we also change it
>>>> to accept “.n” syntax?
>>>
>>> My recommendation would be to change it. It is also not ideal for
>>> this case - only less problematic.
>>
>> If I remembered correctly, the “counted_by” attribute for FAM has been added
>> into Linux Kernel already.
>> Changing its syntax now also involves changing Linux Kernel source code.
>>
>> Kees, what’s your comments and suggestions for this?
>>
>> Another issue is, we had the similar discussion before implementing the
>> “counted_by” attribute for FAM, at that time,
>> We decided to use the current syntax, and CLANG also used the same syntax.
>> Changing the syntax for the “counted_by” attribute for FAM at this moment is
>> not very simple.
>
> We could add the new syntax and still allow the old syntax only for the
> FAM case (possibly deprecating it).
This is a good approach!
So, for “counted_by” of pointer field, we will go with the “.n” syntax.
After that, add this syntax for “counted_by” of FAM.
Is that reasonable steps?
>
>>
>> Why it’s LESS problematic for FAM case? (From implementation point of view,
>> they are the same for both FAM or pointer inside a structure).
>
> Because the FAM is always last, so there is less room for confusion.
> It is still annoying, which is why I pointed this out before.
Okay. I see.
>
>
>
>>>
>>>>>
>>>>> It would be clearer if you the syntax ".n" which resembles
>>>>> the syntax for designated initializers that is already used
>>>>> in initializers to refer to struct members.
>>>>>
>>>>> constexpr int n;
>>>>> struct foo {
>>>>> {
>>>>> char (*p)[n] __attribute__ ((counted_by (.n))
>>>>> int n;
>>>>> }
>>>>>
>>>> Yes, I agree.
>>>>>
>>>>>>
>>>>>>
>>>>>>>>
>>>>>>>> There is one important additional requirement:
>>>>>>>>
>>>>>>>> x->n, x->p can ONLY be changed by changing the whole structure at the
>>>>>>>> same time.
>>>>>>>> Otherwise, x->n might not be consistent with x->p.
>>>>>>>
>>>>>>> By itself, this would still not fix the issue I pointed out.
>>>>>>>
>>>>>>> struct foo x;
>>>>>>> x = .. ; // set the whole structure
>>>>>>> char *p = x->p;
>>>>>>> x = ... ; // set the whole structure
>>>>>>>
>>>>>>> What is the bound for 'p' ?
>>>>>>
>>>>>> Since p was set to the pointer field of the old structure, then the
>>>>>> bound of it should be the old bound.
>>>>>>> With current rules it would be the old bound.
>>>>>>
>>>>>> I thought that this should be the correct behavior, isn’t it?
>>>>>
>>>>> Yes, sorry, what I meant was "with the current rules it would be
>>>>> the *new* bound”.
>>>>
>>>> struct foo x;
>>>> x=… ; // set the whole structure 1
>>>> char *p = x->p;
>>>> x=… ; // set the whole structure 2
>>>>
>>>> In the above, when “set the whole structure 1”, x1, x1->n and x1->p are
>>>> set at the same time;
>>>> After *p = x->p; the pointer “p” is pointing to “x1->p”, it’s bound is
>>>> “x1->n”;
>>>
>>> I agree.
>>>>
>>>> Then when “set the whole structure 2”, x2 is different than x1, x2->n and
>>>> x2->p are set at the same time, the pointer
>>>> ‘p’ still points to “x1->p”, therefore it’s bound should be “x1->n”.
>>>>
>>>> So, as long as the whole structure is set at the same time, should be
>>>> fine.
>>>>
>>>> Do I miss anything here?
>>>
>>> I was talking aout the pointer "p" which was obtained before setting the
>>> struct the second time in
>>>
>>> char *p = x->p;
>>>
>>> This pointer is still set to x1->p but the bound refers to x.n which is
>>> now set to x2->n.
>>
>> You mean:
>>
>> struct foo x;
>> x=… ; // set the whole structure 1
>> char *p = x->p;
>> x=… ; // set the whole structure 2
>> p[index] = 10; // at this point, p’s bound is x2->n, not x1->n?
>>
>> Yes, you are right here.
>>
>> So, is there similar problem with the corresponding language extension?
>>
>
> The language extension does not exist yet, so there is no problem.
Yeah, I should mention this as “corresponding future language extension” -:)
>
> But I hope we will get it and then specify it so that this works
> correctly without this footgun.
>
> Of course, if GCC gets the "counted_by" attribute wrong, there will
> be arguments later in WG14 why the feature is then different to it.
I think that we need to resolve this issue first in the design of “counted_by”
for pointer fields.
I guess that we might need to come up with some additional limitations for
using the “counted_by”
attribute for pointer fields at the source code level in order to avoid such
potential error. But not
sure what exactly the additional limitation should be at this moment.
Need some study here.
Any suggestions?
Thanks.
Qing
>
> Martin
