Message ID | 20230809202122.695376-1-mikael@gcc.gnu.org |
---|---|
Headers |
Return-Path: <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c44e:0:b0:3f2:4152:657d with SMTP id w14csp3053065vqr; Wed, 9 Aug 2023 13:23:14 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEKPCBqP/7S7n32ivcSfDnI/IrEejr+BSbwWNG2FrtBC8/rBBaix9KybzzSB5FVGdY+JEgx X-Received: by 2002:adf:d0c2:0:b0:315:a043:5e0a with SMTP id z2-20020adfd0c2000000b00315a0435e0amr330084wrh.56.1691612594183; Wed, 09 Aug 2023 13:23:14 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691612594; cv=none; d=google.com; s=arc-20160816; b=TfLHwOj+yLzPglemctcCbM+tsOYV4qQA7LmjvOKsjgv4tTR7bEYHOSKSIXHc9Ga4gh DnYkolVCm7mzt3wuMQXOseQxajiSy4/fQGrlYe6h3MSI2FHvU/5nf/Z8fyEtYxcnk7TM TnsmWbygDyBYaM3DWopF4uBcB/Tp6rHNGGI5h5UE9VrT2Lk4OIyQeZCrbTj+1KhUIgqT j5twTLa1TtFzOCqR4Opv86gLPLfwTDrnNKMbDRIkHsn4WczMzUxdwN2lTp/9VCw0+OZM r7BEJ0TT0wKY5UhR8ZA1cz25ZTJqOw+c+tlkXPkq7G8FB/5DiJhk99Ocs51jITtGkghr Y9UA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:to :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=VnhhFZDVf8pIrmDRmGEkgJsplwCpFfTBMGzA74mnBig=; fh=EAqAZnhg4AYtcBjfPm18lEF5V0R2rkI9MSWQf+svVaI=; b=zAUcXpmHGBm+UUvXllHFTPJcjFIdV+AdciXdulVXFIHPHBc+HJsUZeJzPOxUTGmLz4 1AHpg173NUGGIV7/9chmb1bkdjGPr5NdX9rKhJAu9JPEKJr8wHFdxl0VK6LlLtMwP6fe +SR33gLO9RIUT1SFqmWEovjGSFivtujIBCp3+wxjwdy3QgerQw1y368/x/4nHcmvAG5V dTrjas7rKyyw2lujnoQVkl2jJ5XEzxr+17ZEgfffbqkYzDM693uVuTPNsuw2v2cYX2vJ KnV7QaN4LsJJzXwOqLrCXTq1wzTMGYDCIhLWyHL56sCpw7XxQLIhCuHJ2c63+h1SiafH 6Oeg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=qHI3dtB+; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (ip-8-43-85-97.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id k23-20020aa7c397000000b00523271b3ee8si7511040edq.199.2023.08.09.13.23.13 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Aug 2023 13:23:14 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=qHI3dtB+; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 41EA63882018 for <ouuuleilei@gmail.com>; Wed, 9 Aug 2023 20:22:16 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 41EA63882018 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1691612536; bh=VnhhFZDVf8pIrmDRmGEkgJsplwCpFfTBMGzA74mnBig=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=qHI3dtB+nsOlfC9DPV2K9HSmCRohVNV6/5HN+5cWJlNAxBbFKg+jevvogiOPWK9aU Ed1Ir1af6clLjdEMdAmJq8CkCRfoQd+yl6vJEenp5V8R4zj6/4nbmXo99wjGE+mrey BXO6LmtpELuzUCOuCvGYiWXTtqlNe+KnojFBz974= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp.smtpout.orange.fr (smtp-19.smtpout.orange.fr [80.12.242.19]) by sourceware.org (Postfix) with ESMTPS id B5A35385E012 for <gcc-patches@gcc.gnu.org>; Wed, 9 Aug 2023 20:21:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B5A35385E012 Received: from cyrano.home ([86.215.161.51]) by smtp.orange.fr with ESMTPA id TpgJquxgNE5mdTpgQq56Yn; Wed, 09 Aug 2023 22:21:30 +0200 X-ME-Helo: cyrano.home X-ME-Auth: bW9yaW4tbWlrYWVsQG9yYW5nZS5mcg== X-ME-Date: Wed, 09 Aug 2023 22:21:30 +0200 X-ME-IP: 86.215.161.51 To: fortran@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH 0/3] fortran: fix length one character dummy args [PR110419] Date: Wed, 9 Aug 2023 22:21:19 +0200 Message-Id: <20230809202122.695376-1-mikael@gcc.gnu.org> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, FORGED_SPF_HELO, JMQ_SPF_NEUTRAL, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, 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+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773784367166184227 X-GMAIL-MSGID: 1773784367166184227 |
Series |
fortran: fix length one character dummy args [PR110419]
|
|
Message
Mikael Morin
Aug. 9, 2023, 8:21 p.m. UTC
Hello, I propose with this patchset a fix for the test value_9.f90 which has been failing on 32 bits powerpc since it was added a few weeks back (see PR110360 and PR110419). The problem is an argument type mismatch between a procedure declaration, and the argument value for a call of that same procedure, in the specific case of length one character dummy arguments with the value attribute. Admittedly, our argument passing conventions [1] for those are currently unspecified. Before PR110360, character dummy arguments with value attribute were arrays passed by value, but the actual argument was still passed as reference. PR110360 changed that to pass length one dummies as bare character (i.e. scalar integer), like in the bind(c) case (but with length argument still present). However, the argument type in the function declaration wasn't changed at the same time, so the test was failing on big-endian 32 bits targets. Surprisingly, on most targets the middle-end, back-end and runtime are happy to get a scalar value passed where a length one array is expected. This can be fixed, either by reverting back to arguments represented as arrays passed by value with calls fixed, or by keeping the new representation with single characters for arguments and fixing the procedure types accordingly. I haven't really tried the first way, this is using the second one. The first patch is a preliminary refactoring. The main change is the second patch. It modifies the types of length one character dummy argsuments with value attribute in the procedure declarations, so that they are scalar integer types, consistently with how arguments are passed for calls. The third patch is a change of error codes in the testcase. I have regression tested this on x86_64-unknown-linux-gnu, and powerpc64-unknown-linux-gnu (both -m32 and -m64). OK for master? [1] https://gcc.gnu.org/onlinedocs/gfortran/Argument-passing-conventions.html Mikael Morin (3): fortran: New predicate gfc_length_one_character_type_p fortran: Fix length one character dummy arg type [PR110419] testsuite: Use distinct explicit error codes in value_9.f90 gcc/fortran/check.cc | 7 +- gcc/fortran/decl.cc | 4 +- gcc/fortran/gfortran.h | 15 +++ gcc/fortran/trans-expr.cc | 39 ++++--- gcc/fortran/trans-types.cc | 5 +- gcc/testsuite/gfortran.dg/bind_c_usage_13.f03 | 8 +- gcc/testsuite/gfortran.dg/value_9.f90 | 108 +++++++++--------- 7 files changed, 103 insertions(+), 83 deletions(-)
Comments
Hi Mikael, Am 09.08.23 um 22:21 schrieb Mikael Morin via Gcc-patches: > Hello, > > I propose with this patchset a fix for the test value_9.f90 which has been > failing on 32 bits powerpc since it was added a few weeks back (see PR110360 > and PR110419). > > The problem is an argument type mismatch between a procedure declaration, > and the argument value for a call of that same procedure, in the specific > case of length one character dummy arguments with the value attribute. > Admittedly, our argument passing conventions [1] for those are currently > unspecified. > > Before PR110360, character dummy arguments with value attribute were > arrays passed by value, but the actual argument was still passed as > reference. PR110360 changed that to pass length one dummies as bare > character (i.e. scalar integer), like in the bind(c) case (but with length > argument still present). However, the argument type in the function declaration > wasn't changed at the same time, so the test was failing on big-endian 32 bits > targets. Surprisingly, on most targets the middle-end, back-end and runtime > are happy to get a scalar value passed where a length one array is expected. > > This can be fixed, either by reverting back to arguments represented as > arrays passed by value with calls fixed, or by keeping the new > representation with single characters for arguments and fixing the procedure > types accordingly. > > I haven't really tried the first way, this is using the second one. > The first patch is a preliminary refactoring. The main change is the > second patch. It modifies the types of length one character dummy argsuments > with value attribute in the procedure declarations, so that they are scalar > integer types, consistently with how arguments are passed for calls. > The third patch is a change of error codes in the testcase. > > I have regression tested this on x86_64-unknown-linux-gnu, and > powerpc64-unknown-linux-gnu (both -m32 and -m64). > OK for master? this looks good to me. There was only one thing I was uncertain what the right way is: you chose to use mpz_cmp_ui in the length check in the new helper function gfc_length_one_character_type_p, while in many other places the length check uses mpz_cmp_si. Admittedly, a negative (effective/declared) character length can never occur, except maybe at intermediate times during resolution before this is fixed up in accordance with the standard. So this is probably more a cosmetic decision, and you can decide to use either variant. Thanks for the patch! Harald > [1] https://gcc.gnu.org/onlinedocs/gfortran/Argument-passing-conventions.html > > > Mikael Morin (3): > fortran: New predicate gfc_length_one_character_type_p > fortran: Fix length one character dummy arg type [PR110419] > testsuite: Use distinct explicit error codes in value_9.f90 > > gcc/fortran/check.cc | 7 +- > gcc/fortran/decl.cc | 4 +- > gcc/fortran/gfortran.h | 15 +++ > gcc/fortran/trans-expr.cc | 39 ++++--- > gcc/fortran/trans-types.cc | 5 +- > gcc/testsuite/gfortran.dg/bind_c_usage_13.f03 | 8 +- > gcc/testsuite/gfortran.dg/value_9.f90 | 108 +++++++++--------- > 7 files changed, 103 insertions(+), 83 deletions(-) >
Hello, Le 13/08/2023 à 23:16, Harald Anlauf via Fortran a écrit : > Hi Mikael, > > Am 09.08.23 um 22:21 schrieb Mikael Morin via Gcc-patches: >> Hello, >> (...) >> >> I have regression tested this on x86_64-unknown-linux-gnu, and >> powerpc64-unknown-linux-gnu (both -m32 and -m64). >> OK for master? > > this looks good to me. > > There was only one thing I was uncertain what the right way is: > you chose to use mpz_cmp_ui in the length check in the new helper > function gfc_length_one_character_type_p, while in many other places > the length check uses mpz_cmp_si. > > Admittedly, a negative (effective/declared) character length can never > occur, except maybe at intermediate times during resolution before this > is fixed up in accordance with the standard. So this is probably more > a cosmetic decision, and you can decide to use either variant. > That's well spotted, but I think it doesn't matter in this case. There are only two cases: whether the length is 1, and whether the length is different from 1. In each of those two cases, gfc_cmp_si and gfc_cmp_ui return both either zero or non-zero. I'm afraid of last-minute changes, so I prefer to keep the patch as is. Thanks for the review. Mikael