From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.smtpout.orange.fr (smtp08.smtpout.orange.fr [80.12.242.130]) by sourceware.org (Postfix) with ESMTPS id C68803857BB6 for ; Fri, 23 Sep 2022 07:54:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C68803857BB6 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=orange.fr Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=orange.fr Received: from [192.168.1.17] ([86.215.174.255]) by smtp.orange.fr with ESMTPA id bdVmo79HNvd84bdW3ooyA6; Fri, 23 Sep 2022 09:54:32 +0200 X-ME-Helo: [192.168.1.17] X-ME-Auth: bW9yaW4tbWlrYWVsQG9yYW5nZS5mcg== X-ME-Date: Fri, 23 Sep 2022 09:54:32 +0200 X-ME-IP: 86.215.174.255 Message-ID: Date: Fri, 23 Sep 2022 09:54:14 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH v2 0/9] fortran: clobber fixes [PR41453] To: fortran@gcc.gnu.org Cc: gcc-patches@gcc.gnu.org References: <20220918201545.453296-1-mikael@gcc.gnu.org> Content-Language: fr, en-US From: Mikael Morin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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.