Re: [PATCH v2 0/9] fortran: clobber fixes [PR41453]

[Harald Anlauf <anlauf@gmx.de>]

Hi Mikael,

thanks for this impressive series of patches.

Am 18.09.22 um 22:15 schrieb Mikael Morin via Fortran:
> Changes from v1:
>   - patch 9/10 (clobber subreferences) has been dropped.
>   - patch 10/10 (now 9/9): The test has been adjusted because some checks
>     were failing without the dropped patch.  Basically there are less
>     clobbers generated.
>   - patch 5: In the test, an explicit kind has been added to integers,
>     so that the dump match is not dependent on the
>     -fdefault-integer-8 option.
>   - patches 3, 4, 5, 8, and 10/10 (now 9/9): The tests have been renamed
>     so that they are numbered in increasing order.
> 
> The first patch is a refactoring moving the clobber generation in
> gfc_conv_procedure_call where it feels more appropriate.
> The second patch is a fix for the ICE originally motivating my work
> on this topic.
> The third patch is a fix for some wrong code issue discovered with an
> earlier version of this series.

This LGTM.  It also fixes a regression introduced with r9-3030 :-)
If you think that this set (1-3) is backportable, feel free to do so.

> The following patches are gradual condition loosenings to enable clobber
> generation in more and more cases.

Patches 4-8 all look fine.  Since they address missed optimizations,
they are probably something for mainline.

I was wondering if you could add a test for the change in patch 7
addressing the clobber generation for an associate-name, e.g. by
adding to testcase intent_optimize_7.f90 near the end:

   associate (av => ct)
     av = 111222333
     call foo(av)
   end associate
   if (ct /= 42) stop 3

plus the adjustments in the patterns.

Regarding patch 9, I do not see anything wrong with it, but then
independent eyes might see more.  I think it is ok for mainline
as is.

> Each patch has been tested through an incremental bootstrap and a
> partial testsuite run on fortran *intent* tests, and the whole lot has
> been run through the full fortran regression testsuite.
> OK for master?

Yes.

Thanks,
Harald

> 
> Harald Anlauf (1):
>    fortran: Support clobbering with implicit interfaces [PR105012]
> 
> Mikael Morin (8):
>    fortran: Move the clobber generation code
>    fortran: Fix invalid function decl clobber ICE [PR105012]
>    fortran: Move clobbers after evaluation of all arguments [PR106817]
>    fortran: Support clobbering of reference variables [PR41453]
>    fortran: Support clobbering of SAVE variables [PR87395]
>    fortran: Support clobbering of ASSOCIATE variables [PR87397]
>    fortran: Support clobbering of allocatables and pointers [PR41453]
>    fortran: Support clobbering of derived types [PR41453]
> 
>   gcc/fortran/trans-expr.cc                     | 81 ++++++++++++-------
>   gcc/fortran/trans.h                           |  3 +-
>   .../gfortran.dg/intent_optimize_4.f90         | 43 ++++++++++
>   .../gfortran.dg/intent_optimize_5.f90         | 24 ++++++
>   .../gfortran.dg/intent_optimize_6.f90         | 34 ++++++++
>   .../gfortran.dg/intent_optimize_7.f90         | 42 ++++++++++
>   .../gfortran.dg/intent_optimize_8.f90         | 66 +++++++++++++++
>   gcc/testsuite/gfortran.dg/intent_out_15.f90   | 27 +++++++
>   8 files changed, 290 insertions(+), 30 deletions(-)
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_4.f90
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_5.f90
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_6.f90
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_7.f90
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_optimize_8.f90
>   create mode 100644 gcc/testsuite/gfortran.dg/intent_out_15.f90
>