Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]

[Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>]

On 17 September 2022 21:33:22 CEST, Mikael Morin <morin-mikael@orange.fr> wrote:
>Le 17/09/2022 à 19:03, Thomas Koenig via Fortran a écrit :
>> 
>> Hi Mikael,
>> 
>>> This adds support for clobbering of partial variable references, when
>>> they are passed as actual argument and the associated dummy has the
>>> INTENT(OUT) attribute.
>>> Support includes array elements, derived type component references,
>>> and complex real or imaginary parts.
>>> 
>>> This is done by removing the check for lack of subreferences, which is
>>> basically a revert of r9-4911-gbd810d637041dba49a5aca3d085504575374ac6f.
>>> This removal allows more expressions than just array elements,
>>> components and complex parts, but the other expressions are excluded by
>>> other conditions: substrings are excluded by the check on expression
>>> type (CHARACTER is excluded), KIND and LEN references are rejected by
>>> the compiler as not valid in a variable definition context.
>>> 
>>> The check for scalarness is also updated as it was only valid when there
>>> was no subreference.
>> 
>> First, thanks a lot for digging into this subject. I have looked through
>> the patch series, and it looks very good so far.

I second that!
The series looks plausible IMO.

>> 
>> I have a concern about this part, though.  My understanding at the
>> time was that it is not possible to clobber an individual array
>> element, but that this clobbers anything off the pointer that this
>> is based on.
>> 
>Well, we need the middle-end guys to give a definitive answer on this topic, but I think it would be a very penalizing limitation if that was the case.  I have assumed that the clobber spanned the value it was applied on, neither more nor less, so just the array element in case of array elements.

I would assume the same, fwiw.
Let's blame the ME iff something goes amiss then, but I doubt it will.

>> So,
>> 
>>    integer, dimension(3) :: a
>> 
>>    a(1) = 1
>>    a(3) = 3
>>    call foo(a(1))
>> 
>> would also invalidate the store to a(3).  Is my understanding correct?
>
>I think it was the case before patch 2 in in the series, because the clobber was applied to the symbol decl, so in the case of the expression A(1), it was applied to A which is the full array.  After patch 2, the clobber is applied to the expression A(1), so the element alone.

Yep.

>> If so, I think this we cannot revert that patch (which was introduced
>> because of a regression).
>> 
>The testcase from the patch was not specifically checking lack of side-effect clobbers, so I have double-checked with the following testcase, which should lift your concerns.
>I propose to keep the patch with the testcase added to it.  What do you think?

I cannot approve it but the series looks good to me.

Thanks!