Le 22/09/2022 à 22:42, Harald Anlauf via Fortran a écrit :
Hi Mikael,

thanks for this impressive series of patches.

Am 18.09.22 um 22:15 schrieb Mikael Morin via Fortran:
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.

Yes, 2 and 3 are worth backporting, I will see how dependent they are on 1.

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.

Indeed, I didn't add a test because there was one already, but the existing test hasn't the check for clobber generation and store removal. I prefer to create a new test though, so that the patch and the test come together, and the test for patch 8 is not encumbered with unrelated stuff.

By the way, the same could be said about patch 6.
I will create a test for that one as well.

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 for the review.

Reply via email to