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

Message ID 20220916202439.549820-10-mikael@gcc.gnu.org
State New
Headers
Series fortran: clobber fixes [PR41453] |

Commit Message

Mikael Morin Sept. 16, 2022, 8:24 p.m. UTC
  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.

	PR fortran/88364
	PR fortran/41453

gcc/fortran/ChangeLog:

	* trans-expr.cc (gfc_conv_procedure_call): Don’t check for lack
	of subreference.  Check the global expression rank instead of
	the root symbol dimension attribute.

gcc/testsuite/ChangeLog:

	* gfortran.dg/intent_optimize_7.f90: New test.
---
 gcc/fortran/trans-expr.cc                     |  5 +-
 .../gfortran.dg/intent_optimize_7.f90         | 65 +++++++++++++++++++
 2 files changed, 66 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_7.f90
  

Comments

Thomas Koenig Sept. 17, 2022, 5:03 p.m. UTC | #1
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 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.

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?
If so, I think this we cannot revert that patch (which was introduced
because of a regression).

Best regards

	Thomas
  
Mikael Morin Sept. 17, 2022, 7:33 p.m. UTC | #2
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 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.

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

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

Mikael
  
Bernhard Reutner-Fischer Sept. 17, 2022, 7:49 p.m. UTC | #3
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!
  
Mikael Morin Sept. 17, 2022, 7:50 p.m. UTC | #4
Le 17/09/2022 à 21:33, Mikael Morin a écrit :
> 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.
> 
The dump matches didn’t fail as expected with patch 2/10 reversed.
This testcase should be better.
  
Bernhard Reutner-Fischer Sept. 17, 2022, 9:24 p.m. UTC | #5
On 17 September 2022 21:50:20 CEST, Mikael Morin <morin-mikael@orange.fr> wrote:
>Le 17/09/2022 à 21:33, Mikael Morin a écrit :
>> 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.
>> 
>The dump matches didn’t fail as expected with patch 2/10 reversed.
>This testcase should be better.

! { dg-final { scan-tree-dump-times "456" 0 "optimized" { target __OPTIMIZE__ } } }

I'd spell this as scan-tree-dump-not, fwiw.

That said, plain scan-tree-dump is usually only viable in arch influenced checks which in fortran we do not usually have. Here, we should for the most part use -not or a specific -times.

I think you had a check for integer(kind=4) in there, too, which might not work all that well for -fdefault-integer-8 or, for the corresponding real scan, -fdefault-real-8, eventually. Easily tweaked on top if anyone (certainly will) complain later on, though..

fore, either way, I'd say :-)
thanks,
  
Richard Biener Sept. 18, 2022, 6:12 a.m. UTC | #6
On Sat, Sep 17, 2022 at 9:33 PM 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 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.

There is IL verification that the LHS of a CLOBBER is either
a declaration or a pointer dereference, no array or component
selection is allowed there.  Now, nothing should go wrong here,
but we might eventually just drop those CLOBBERs or ICE if
we frontend hands us an "invalid" one.

Richard.

> > 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.
>
> > 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?
>
> Mikael
>
  
Mikael Morin Sept. 18, 2022, 9:10 a.m. UTC | #7
Le 18/09/2022 à 08:12, Richard Biener a écrit :
> On Sat, Sep 17, 2022 at 9:33 PM Mikael Morin <morin-mikael@orange.fr> wrote:
>>
>> Le 17/09/2022 à 19:03, Thomas Koenig via Fortran a écrit :
>>>
>>> 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.
> 
> There is IL verification that the LHS of a CLOBBER is either
> a declaration or a pointer dereference, no array or component
> selection is allowed there.  Now, nothing should go wrong here,
> but we might eventually just drop those CLOBBERs or ICE if
> we frontend hands us an "invalid" one.
> 

Obviously I have assumed too much here; it's probably best to drop this 
patch.
It is unfortunate as there is some desirable behavior within reach here. 
  The test shows that the patch permits the elimination of a useless 
store.  And IL verification doesn't seem that upset with it by the way.
Does *(&a[1]) count as a pointer dereference?  Even in the original dump 
it is already simplified to a straight a[1].
  
Thomas Koenig Sept. 18, 2022, 10:23 a.m. UTC | #8
On 18.09.22 11:10, Mikael Morin wrote:
> Le 18/09/2022 à 08:12, Richard Biener a écrit :
>> On Sat, Sep 17, 2022 at 9:33 PM Mikael Morin <morin-mikael@orange.fr> 
>> wrote:
>>>
>>> Le 17/09/2022 à 19:03, Thomas Koenig via Fortran a écrit :
>>>>
>>>> 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.
>>
>> There is IL verification that the LHS of a CLOBBER is either
>> a declaration or a pointer dereference, no array or component
>> selection is allowed there.  Now, nothing should go wrong here,
>> but we might eventually just drop those CLOBBERs or ICE if
>> we frontend hands us an "invalid" one.
>>
> 
> Obviously I have assumed too much here; it's probably best to drop this 
> patch.

Probably, yes.

> It is unfortunate as there is some desirable behavior within reach here. 

I think some of the desired behavior can still be salvaged.  For
example, for

   subroutine foo(a,n)
     integer :: n
     integer, dimension(n), intent(in) :: n

...

   subroutine bar(a)
     integer, intent(out) :: a

...

   integer :: a(3)

   call foo(a,3)
   call foo(a(1),3)

clobbers for the whole array can still be generated, but not for

   call foo(a(2),2)

so one would have to look at the lower bound.

For this case, it would be helpful to clobber a range a(2:), but that
is a wishlist item for the future ;-)

What is unsafe, currently, is

   call bar(a(1))

Best regards

	Thomas
  
Richard Biener Sept. 18, 2022, 10:48 a.m. UTC | #9
> Am 18.09.2022 um 11:10 schrieb Mikael Morin <morin-mikael@orange.fr>:
> 
> Le 18/09/2022 à 08:12, Richard Biener a écrit :
>>> On Sat, Sep 17, 2022 at 9:33 PM Mikael Morin <morin-mikael@orange.fr> wrote:
>>> 
>>> Le 17/09/2022 à 19:03, Thomas Koenig via Fortran a écrit :
>>>> 
>>>> 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.
>> There is IL verification that the LHS of a CLOBBER is either
>> a declaration or a pointer dereference, no array or component
>> selection is allowed there.  Now, nothing should go wrong here,
>> but we might eventually just drop those CLOBBERs or ICE if
>> we frontend hands us an "invalid" one.
> 
> Obviously I have assumed too much here; it's probably best to drop this patch.
> It is unfortunate as there is some desirable behavior within reach here.  The test shows that the patch permits the elimination of a useless store.  And IL verification doesn't seem that upset with it by the way.
> Does *(&a[1]) count as a pointer dereference?

Yes, technically.

>  Even in the original dump it is already simplified to a straight a[1].

But this not anymore.  The check can probably be relaxed, it stems from the dual purpose of CLOBBER.

Richard
  
Harald Anlauf Sept. 18, 2022, 6:32 p.m. UTC | #10
On 18.09.22 12:23, Thomas Koenig via Gcc-patches wrote:
>
> On 18.09.22 11:10, Mikael Morin wrote:
>> It is unfortunate as there is some desirable behavior within reach here.
>
> I think some of the desired behavior can still be salvaged.  For
> example, for
>
>    subroutine foo(a,n)
>      integer :: n
>      integer, dimension(n), intent(in) :: n

     integer, dimension(n), intent(out) :: a  ! ?

> ...
>
>    subroutine bar(a)
>      integer, intent(out) :: a
>
> ...
>
>    integer :: a(3)
>
>    call foo(a,3)
>    call foo(a(1),3)
>
> clobbers for the whole array can still be generated, but not for
>
>    call foo(a(2),2)
>
> so one would have to look at the lower bound.
>
> For this case, it would be helpful to clobber a range a(2:), but that
> is a wishlist item for the future ;-)
>
> What is unsafe, currently, is
>
>    call bar(a(1))

We'll need a good coverage by testcases for the different handling
required for assumed shape / assumed size / explicit size dummies
to avoid new regressions.  Assumed shape will be on the easy side,
while assumed size likely needs to be excluded for clobbering.

Harald
  
Mikael Morin Sept. 18, 2022, 8:43 p.m. UTC | #11
Le 18/09/2022 à 12:23, Thomas Koenig a écrit :
> 
> I think some of the desired behavior can still be salvaged.  For
> example, for
> 
(...)
> 
> clobbers for the whole array can still be generated, but not for
> 
>    call foo(a(2),2)
> 
> so one would have to look at the lower bound.
> 
Well, my patches were for scalars only,
I prefer to stay away from arrays; they are just too full of traps.
  
Mikael Morin Sept. 18, 2022, 8:55 p.m. UTC | #12
Le 18/09/2022 à 20:32, Harald Anlauf a écrit :
> 
> Assumed shape will be on the easy side,
> while assumed size likely needs to be excluded for clobbering.
> 
Isn’t it the converse that is true?
Assumed shape can be non-contiguous so have to be excluded, but assumed 
size are contiguous, so valid candidates for clobbering. No?

No way, really, arrays are going to be a maze of complexity.
  
Mikael Morin Sept. 19, 2022, 7:11 a.m. UTC | #13
Le 18/09/2022 à 22:55, Mikael Morin a écrit :
> Assumed shape can be non-contiguous so have to be excluded, 
> 
Thinking about it again, assumed shape are probably acceptable, but 
there should be a check for contiguousness on the actual argument.
  
Mikael Morin Sept. 19, 2022, 7:31 a.m. UTC | #14
Le 18/09/2022 à 12:48, Richard Biener a écrit :
> 
>> Does *(&a[1]) count as a pointer dereference?
> 
> Yes, technically.
> 
>>   Even in the original dump it is already simplified to a straight a[1].
> 
> But this not anymore.  The check can probably be relaxed, it stems from the dual purpose of CLOBBER.
> 
So the following makes the frontend-emitted IL valid, by handing the 
simplification over to the middle-end, but I can't help thinking that 
behavior won't be more reliable.


diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index f8fcd2d97d9..5fb9a3a536d 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6544,8 +6544,9 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * 
sym,
                           && !sym->attr.elemental)
                         {
                           tree var;
-                         var = build_fold_indirect_ref_loc (input_location,
-                                                            parmse.expr);
+                         var = build1_loc (input_location, INDIRECT_REF,
+                                           TREE_TYPE (TREE_TYPE 
(parmse.expr)),
+                                           parmse.expr);
                           tree clobber = build_clobber (TREE_TYPE (var));
                           gfc_add_modify (&clobbers, var, clobber);
                         }
  
Richard Biener Sept. 19, 2022, 7:58 a.m. UTC | #15
On Mon, Sep 19, 2022 at 9:31 AM Mikael Morin <morin-mikael@orange.fr> wrote:
>
> Le 18/09/2022 à 12:48, Richard Biener a écrit :
> >
> >> Does *(&a[1]) count as a pointer dereference?
> >
> > Yes, technically.
> >
> >>   Even in the original dump it is already simplified to a straight a[1].
> >
> > But this not anymore.  The check can probably be relaxed, it stems from the dual purpose of CLOBBER.
> >
> So the following makes the frontend-emitted IL valid, by handing the
> simplification over to the middle-end, but I can't help thinking that
> behavior won't be more reliable.

I think that will just delay the folding.  We are basically relying on
the frontends to
restrict what they produce, the *ptr case was specifically for C++
this in CTOR/DTORs
and during inlining we throw away all clobbers that end up "wrong" but
I guess we
might have latent issues when we obfuscate the address in the caller enough and
get undesired simplification ...

>
> diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
> index f8fcd2d97d9..5fb9a3a536d 100644
> --- a/gcc/fortran/trans-expr.cc
> +++ b/gcc/fortran/trans-expr.cc
> @@ -6544,8 +6544,9 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol *
> sym,
>                            && !sym->attr.elemental)
>                          {
>                            tree var;
> -                         var = build_fold_indirect_ref_loc (input_location,
> -                                                            parmse.expr);
> +                         var = build1_loc (input_location, INDIRECT_REF,
> +                                           TREE_TYPE (TREE_TYPE
> (parmse.expr)),
> +                                           parmse.expr);
>                            tree clobber = build_clobber (TREE_TYPE (var));
>                            gfc_add_modify (&clobbers, var, clobber);
>                          }
>
  
Harald Anlauf Sept. 19, 2022, 7:46 p.m. UTC | #16
Am 18.09.22 um 22:55 schrieb Mikael Morin:
> Le 18/09/2022 à 20:32, Harald Anlauf a écrit :
>>
>> Assumed shape will be on the easy side,
>> while assumed size likely needs to be excluded for clobbering.
>>
> Isn’t it the converse that is true?
> Assumed shape can be non-contiguous so have to be excluded, but assumed 
> size are contiguous, so valid candidates for clobbering. No?

I really was referring here to *dummies*, as in the following example:

program p
   integer :: a(4)
   a = 1
   call sub (a(1), 2)
   print *, a
contains
   subroutine sub (b, k)
     integer, intent(in)  :: k
     integer, intent(out) :: b(*)
!   integer, intent(out) :: b(k)
     if (k > 2) b(k) = k
   end subroutine sub
end program p

Assumed size (*) is just a contiguous hunk of memory of possibly
unknown size, which can be zero.  So you couldn't set a clobber
for the a(1) actual argument.

> No way, really, arrays are going to be a maze of complexity.

Agreed.
  
Mikael Morin Sept. 19, 2022, 8:50 p.m. UTC | #17
Le 19/09/2022 à 21:46, Harald Anlauf a écrit :
> Am 18.09.22 um 22:55 schrieb Mikael Morin:
>> Le 18/09/2022 à 20:32, Harald Anlauf a écrit :
>>>
>>> Assumed shape will be on the easy side,
>>> while assumed size likely needs to be excluded for clobbering.
>>>
>> Isn’t it the converse that is true?
>> Assumed shape can be non-contiguous so have to be excluded, but assumed
>> size are contiguous, so valid candidates for clobbering. No?
> 
> I really was referring here to *dummies*, as in the following example:
> 
> program p
>    integer :: a(4)
>    a = 1
>    call sub (a(1), 2)
>    print *, a
> contains
>    subroutine sub (b, k)
>      integer, intent(in)  :: k
>      integer, intent(out) :: b(*)
> !   integer, intent(out) :: b(k)
>      if (k > 2) b(k) = k
>    end subroutine sub
> end program p
> 
> Assumed size (*) is just a contiguous hunk of memory of possibly
> unknown size, which can be zero.  So you couldn't set a clobber
> for the a(1) actual argument.
> 
Couldn't you clobber A entirely?  If no element of B is initialized in 
SUB, well, A has undefined values on return from SUB.  That's how 
INTENT(OUT) works.
  
Thomas Koenig Sept. 20, 2022, 6:54 a.m. UTC | #18
On 19.09.22 22:50, Mikael Morin wrote:
> Le 19/09/2022 à 21:46, Harald Anlauf a écrit :
>> Am 18.09.22 um 22:55 schrieb Mikael Morin:
>>> Le 18/09/2022 à 20:32, Harald Anlauf a écrit :
>>>>
>>>> Assumed shape will be on the easy side,
>>>> while assumed size likely needs to be excluded for clobbering.
>>>>
>>> Isn’t it the converse that is true?
>>> Assumed shape can be non-contiguous so have to be excluded, but assumed
>>> size are contiguous, so valid candidates for clobbering. No?
>>
>> I really was referring here to *dummies*, as in the following example:
>>
>> program p
>>    integer :: a(4)
>>    a = 1
>>    call sub (a(1), 2)
>>    print *, a
>> contains
>>    subroutine sub (b, k)
>>      integer, intent(in)  :: k
>>      integer, intent(out) :: b(*)
>> !   integer, intent(out) :: b(k)
>>      if (k > 2) b(k) = k
>>    end subroutine sub
>> end program p
>>
>> Assumed size (*) is just a contiguous hunk of memory of possibly
>> unknown size, which can be zero.  So you couldn't set a clobber
>> for the a(1) actual argument.
>>
> Couldn't you clobber A entirely?  If no element of B is initialized in 
> SUB, well, A has undefined values on return from SUB.  That's how 
> INTENT(OUT) works.

Yes, I think so - you are passing the starting element of an array
to an assumed-size array via storage association rules.

It has to be an explicit interface, of course, otherwise it is
unclear if an array or an array element is passed.

Best regards

	Thomas
  
Mikael Morin Sept. 20, 2022, 8:46 a.m. UTC | #19
Le 20/09/2022 à 08:54, Thomas Koenig a écrit :
> 
> On 19.09.22 22:50, Mikael Morin wrote:
>> Le 19/09/2022 à 21:46, Harald Anlauf a écrit :
>>>
>>> Assumed size (*) is just a contiguous hunk of memory of possibly
>>> unknown size, which can be zero.  So you couldn't set a clobber
>>> for the a(1) actual argument.
>>>
>> Couldn't you clobber A entirely?  If no element of B is initialized in 
>> SUB, well, A has undefined values on return from SUB.  That's how 
>> INTENT(OUT) works.
> 
> Yes, I think so - you are passing the starting element of an array
> to an assumed-size array via storage association rules.
> 
> It has to be an explicit interface, of course, otherwise it is
> unclear if an array or an array element is passed.
> 

I have looked for the relevant excerpts from the standard.
 From 15.5.2.11 (sequence association):

> If the dummy argument is not of type character
> with default or C character kind, and the actual argument is an array element designator, the element sequence
> consists of that array element and each element that follows it in array element order.

> If the dummy argument is
> assumed-size, the number of elements in the dummy argument is exactly the number of elements in the element
> sequence.

So the dummy size, even if not known to the programmer, is clearly 
defined (to the full array size in your example).
  
Harald Anlauf Sept. 20, 2022, 9:08 p.m. UTC | #20
Am 19.09.22 um 22:50 schrieb Mikael Morin:
> Le 19/09/2022 à 21:46, Harald Anlauf a écrit :
>> Am 18.09.22 um 22:55 schrieb Mikael Morin:
>>> Le 18/09/2022 à 20:32, Harald Anlauf a écrit :
>>>>
>>>> Assumed shape will be on the easy side,
>>>> while assumed size likely needs to be excluded for clobbering.
>>>>
>>> Isn’t it the converse that is true?
>>> Assumed shape can be non-contiguous so have to be excluded, but assumed
>>> size are contiguous, so valid candidates for clobbering. No?
>>
>> I really was referring here to *dummies*, as in the following example:
>>
>> program p
>>    integer :: a(4)
>>    a = 1
>>    call sub (a(1), 2)
>>    print *, a
>> contains
>>    subroutine sub (b, k)
>>      integer, intent(in)  :: k
>>      integer, intent(out) :: b(*)
>> !   integer, intent(out) :: b(k)
>>      if (k > 2) b(k) = k
>>    end subroutine sub
>> end program p
>>
>> Assumed size (*) is just a contiguous hunk of memory of possibly
>> unknown size, which can be zero.  So you couldn't set a clobber
>> for the a(1) actual argument.
>>
> Couldn't you clobber A entirely?  If no element of B is initialized in
> SUB, well, A has undefined values on return from SUB.  That's how
> INTENT(OUT) works.
>

I think I understand much of what is said, but I feel that I do
not really understand what *clobber* means for the different
beasts we are discussing (although I have an impression of what
it means for a scalar object).
  
Thomas Koenig Sept. 21, 2022, 9:57 a.m. UTC | #21
Hi Harald,

> I think I understand much of what is said, but I feel that I do
> not really understand what *clobber* means for the different
> beasts we are discussing (although I have an impression of what
> it means for a scalar object).

Obviously, "clobber" means taking a big stick and hitting the beast
in question over the head with it :-)

More seriously: My understanding of a clobber it is a hint to
the middle end that the value in question will not be used,
and that operations leading to this value can be removed,
unless they are used otherwise.

If I'm wrong or imprecise, I'm sure somebody will correct me :-)

Regards

	Thomas
  
Mikael Morin Sept. 21, 2022, 6:56 p.m. UTC | #22
Le 21/09/2022 à 11:57, Thomas Koenig a écrit :
> 
> Hi Harald,
> 
>> I think I understand much of what is said, but I feel that I do
>> not really understand what *clobber* means for the different
>> beasts we are discussing (although I have an impression of what
>> it means for a scalar object).
> 
> Obviously, "clobber" means taking a big stick and hitting the beast
> in question over the head with it :-)
> 
> More seriously: My understanding of a clobber it is a hint to
> the middle end that the value in question will not be used,
> and that operations leading to this value can be removed,
> unless they are used otherwise.
> 
My understanding is that "clobber" means "overwrite with garbage" for 
all the beasts we have been discussing, which translates to nothing in 
the final code, but can be used by the optimizers as Thomas said.

This is a bit off-topic but clobbers model registers having their values 
changed unpredictably or by ways unknown to the compiler, in the backend 
code, or in inline assembly statements.
Here is an excerpt from rtl.texi:
> @item (clobber @var{x})
> Represents the storing or possible storing of an unpredictable,
> undescribed value into @var{x}

I Hope it helps.
  
Harald Anlauf Sept. 21, 2022, 7:12 p.m. UTC | #23
Hi Mikael,

Am 21.09.22 um 20:56 schrieb Mikael Morin:
> Le 21/09/2022 à 11:57, Thomas Koenig a écrit :
>>
>> Hi Harald,
>>
>>> I think I understand much of what is said, but I feel that I do
>>> not really understand what *clobber* means for the different
>>> beasts we are discussing (although I have an impression of what
>>> it means for a scalar object).

>>
>> More seriously: My understanding of a clobber it is a hint to
>> the middle end that the value in question will not be used,
>> and that operations leading to this value can be removed,
>> unless they are used otherwise.
>>
> My understanding is that "clobber" means "overwrite with garbage" for
> all the beasts we have been discussing, which translates to nothing in
> the final code, but can be used by the optimizers as Thomas said.
>
> This is a bit off-topic but clobbers model registers having their values
> changed unpredictably or by ways unknown to the compiler, in the backend
> code, or in inline assembly statements.
> Here is an excerpt from rtl.texi:
>> @item (clobber @var{x})
>> Represents the storing or possible storing of an unpredictable,
>> undescribed value into @var{x}

ah, I missed that file.  I only found references to assembly,
and references to registers etc. were not really helpful here.

It also says:

 > If @var{x} is @code{(mem:BLK (const_int 0))} or
 > @code{(mem:BLK (scratch))}, it means that all memory
 > locations must be presumed clobbered.  ...

so this goes into the direction I was thinking of.

> I Hope it helps.
>
  

Patch

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index ae685157e22..f1026d7f309 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6521,10 +6521,7 @@  gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 			  && !dsym->attr.allocatable
 			  && !dsym->attr.pointer
 			  && e->expr_type == EXPR_VARIABLE
-			  && e->ref == NULL
-			  && e->symtree
-			  && e->symtree->n.sym
-			  && !e->symtree->n.sym->attr.dimension
+			  && e->rank == 0
 			  && e->ts.type != BT_CHARACTER
 			  && e->ts.type != BT_DERIVED
 			  && e->ts.type != BT_CLASS
diff --git a/gcc/testsuite/gfortran.dg/intent_optimize_7.f90 b/gcc/testsuite/gfortran.dg/intent_optimize_7.f90
new file mode 100644
index 00000000000..14dcfd9961b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/intent_optimize_7.f90
@@ -0,0 +1,65 @@ 
+! { dg-do run }
+! { dg-additional-options "-fno-inline -fno-ipa-modref -fdump-tree-optimized -fdump-tree-original" }
+!
+! PR fortran/41453
+! Check that the INTENT(OUT) attribute causes one clobber to be emitted in
+! the caller before each call to FOO or BAR in the *.original dump, and the
+! initialization constants to be optimized away in the *.optimized dump,
+! in the case of scalar array elements, derived type components,
+! and complex real and imaginary part.
+
+module x
+implicit none
+contains
+  subroutine foo(a)
+    integer, intent(out) :: a
+    a = 42
+  end subroutine foo
+  subroutine bar(a)
+    real, intent(out) :: a
+    a = 24.0
+  end subroutine bar
+end module x
+
+program main
+  use x
+  implicit none
+  type :: t
+    integer :: c
+  end type t
+  type(t) :: dc
+  integer :: ac(3)
+  complex :: xc, xd
+
+  dc = t(123456789)
+  call foo(dc%c)
+  if (dc%c /= 42) stop 1
+
+  ac = 100
+  ac(2) = 987654321
+  call foo(ac(2))
+  if (any(ac /= [100, 42, 100])) stop 2
+
+  xc = (12345.0, 11.0)
+  call bar(xc%re)
+  if (xc /= (24.0, 11.0)) stop 3
+
+  xd = (17.0, 67890.0)
+  call bar(xd%im)
+  if (xd /= (17.0, 24.0)) stop 4
+
+end program main
+
+! { dg-final { scan-tree-dump-times "CLOBBER" 4 "original" } }
+! { dg-final { scan-tree-dump "dc\\.c = {CLOBBER};" "original" } }
+! { dg-final { scan-tree-dump "ac\\\[1\\\] = {CLOBBER};" "original" } }
+! { dg-final { scan-tree-dump "REALPART_EXPR <xc> = {CLOBBER};" "original" } }
+! { dg-final { scan-tree-dump "IMAGPART_EXPR <xd> = {CLOBBER};" "original" } }
+! { dg-final { scan-tree-dump     "123456789" "original" } }
+! { dg-final { scan-tree-dump-not "123456789" "optimized" { target __OPTIMIZE__ } } }
+! { dg-final { scan-tree-dump     "987654321" "original" } }
+! { dg-final { scan-tree-dump-not "987654321" "optimized" { target __OPTIMIZE__ } } }
+! { dg-final { scan-tree-dump     "1\\.2345e\\+4" "original"  } }
+! { dg-final { scan-tree-dump-not "1\\.2345e\\+4" "optimized" { target __OPTIMIZE__ } } }
+! { dg-final { scan-tree-dump     "6\\.789e\\+4" "original"  } }
+! { dg-final { scan-tree-dump-not "6\\.789e\\+4" "optimized" { target __OPTIMIZE__ } } }