Message ID | 20220918201545.453296-1-mikael@gcc.gnu.org |
---|---|
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4022A3850233 for <patchwork@sourceware.org>; Sun, 18 Sep 2022 20:17:02 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4022A3850233 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1663532222; bh=lc3oalQvx8uKDoIn5Uy3SfZvcyTj4yVcDixGx8USxbI=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=sf/oR9u9+s8kIRRG3DOvbhvrJzZGhxaFvsSGHcGmRSER9ief3y0WkwRhX8lyjZhIU 4b+4aQpfAhMTdG1nDX8Km+atGvhdP+m0veBe58vABtxZeos9wcHLlZvQwNeQ7bZ3ah 9cyCpD5Y039sGJWFFdRxwVZiK9tM/5DY+DCUDL2s= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp.smtpout.orange.fr (smtp-27.smtpout.orange.fr [80.12.242.27]) by sourceware.org (Postfix) with ESMTPS id 47BEE3858C00 for <gcc-patches@gcc.gnu.org>; Sun, 18 Sep 2022 20:15:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 47BEE3858C00 Received: from cyrano.home ([86.215.174.255]) by smtp.orange.fr with ESMTPA id a0hgorqvnjJi0a0hmokkE8; Sun, 18 Sep 2022 22:15:54 +0200 X-ME-Helo: cyrano.home X-ME-Auth: bW9yaW4tbWlrYWVsQG9yYW5nZS5mcg== X-ME-Date: Sun, 18 Sep 2022 22:15:54 +0200 X-ME-IP: 86.215.174.255 To: gcc-patches@gcc.gnu.org, fortran@gcc.gnu.org Subject: [PATCH v2 0/9] fortran: clobber fixes [PR41453] Date: Sun, 18 Sep 2022 22:15:36 +0200 Message-Id: <20220918201545.453296-1-mikael@gcc.gnu.org> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.3 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NEUTRAL, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Mikael Morin via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Mikael Morin <mikael@gcc.gnu.org> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
fortran: clobber fixes [PR41453]
|
|
Message
Mikael Morin
Sept. 18, 2022, 8:15 p.m. UTC
Hello, this is the second version of a set of changes around the clobber we generate in the caller before a procedure call, for each actual argument whose associated dummy has the INTENT(OUT) attribute. The clobbering of partial variables (array elements, structure components) proved to be unsupported by the middle-end optimizers, even if it seemed to work in practice. So this version just removes it. v1 of the series was posted here: https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601713.html https://gcc.gnu.org/pipermail/fortran/2022-September/058165.html 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. The following patches are gradual condition loosenings to enable clobber generation in more and more cases. 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? 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
Comments
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.
Le 23/09/2022 à 09:54, Mikael Morin a écrit : > Le 22/09/2022 à 22:42, Harald Anlauf via Fortran a écrit : >> 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. > Patches pushed: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=77bbf69d2981dafc2ef3e59bfbefb645d88bab9d Changes from v2: - patches 6 and 7: A test for each has been added. - patches 8 and 9: The tests have been renumbered. - patches 6 and 7: The PR number used in the subject line has been changed, from the different regression PRs to the one optimization PR. - patches 5 and 8: The commit message has been modified: the commit the patch partly reverts is mentioned, and the associated PR number as well. - patch 7: The regression PR number this refers to has been changed.
Le 23/09/2022 à 09:54, Mikael Morin a écrit : > Le 22/09/2022 à 22:42, Harald Anlauf via Fortran a écrit : >> 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. > I'm going to backport 1-3 as you suggested, it's so much easier.