| Message ID | orsfqy7f16.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| 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 49BDF385DC02 for <patchwork@sourceware.org>; Thu, 31 Mar 2022 07:33:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 49BDF385DC02 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1648712016; bh=F92ThoaiPDj9407CJm1LJCAyQajsqOSyNc3sdPdkkTI=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=XLpcy3v5c7iu2ZiRTvYacE8V2YKmQ2m2vXhGl4jFOqpBz902G35/+l/uz6FvP8Uyp 38n/49et9YUMOXluFpdsSNFUZBSMTubVz6qJQVC7ZUQ5/r1kf6EpFGv3MCQRDXOo28 ILwaaFC8W4tsWkIKD+t2wycAIkWj7bm1ovHr2jfs= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from rock.gnat.com (rock.gnat.com [IPv6:2620:20:4000:0:a9e:1ff:fe9b:1d1]) by sourceware.org (Postfix) with ESMTPS id 61943385800E for <gcc-patches@gcc.gnu.org>; Thu, 31 Mar 2022 07:33:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 61943385800E Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 3F167116588; Thu, 31 Mar 2022 03:33:03 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id T1IurY2q3rrW; Thu, 31 Mar 2022 03:33:03 -0400 (EDT) Received: from free.home (tron.gnat.com [IPv6:2620:20:4000:0:46a8:42ff:fe0e:e294]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id D4D38116547; Thu, 31 Mar 2022 03:33:02 -0400 (EDT) Received: from livre (livre.home [172.31.160.2]) by free.home (8.15.2/8.15.2) with ESMTPS id 22V7Wr0b1702343 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 31 Mar 2022 04:32:55 -0300 To: gcc-patches@gcc.gnu.org Subject: C++ modules and AAPCS/ARM EABI clash on inline key methods Organization: Free thinker, does not speak for AdaCore Date: Thu, 31 Mar 2022 04:32:53 -0300 Message-ID: <orsfqy7f16.fsf@lxoliva.fsfla.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Alexandre Oliva via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Alexandre Oliva <oliva@adacore.com> Cc: nathan@acm.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 |
C++ modules and AAPCS/ARM EABI clash on inline key methods
|
|
Commit Message
Alexandre Oliva
March 31, 2022, 7:32 a.m. UTC
g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets that use the AAPCS variant. ARM is the only target that overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way the clash between AAPCS and C++ Modules design should be resolved, but currently it favors AAPCS and thus the test fails. Should we skip the test on ARM, XFAIL it, or put in some kludge like the patchlet below?
Comments
On Mar 31, 2022, Alexandre Oliva <oliva@adacore.com> wrote: > g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets > that use the AAPCS variant. ARM is the only target that overrides > TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way the > clash between AAPCS and C++ Modules design should be resolved, but > currently it favors AAPCS and thus the test fails. > Should we skip the test on ARM, XFAIL it, or put in some kludge like > the patchlet below? That kludge doesn't work: subsequent virt tests fail with it, on arm. Would something like this be acceptable/desirable? It's overreaching, in that not all arm platforms are expected to fail, but the result on them will be an unexpected pass, which is not quite as bad as the unexpected fail we get on most arm variants now. diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C index 9115cc19cc286..0b780645708ba 100644 --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C @@ -22,6 +22,6 @@ export int Visit (Visitor *v) } // Emit here -// { dg-final { scan-assembler {_ZTV7Visitor:} } } -// { dg-final { scan-assembler {_ZTI7Visitor:} } } -// { dg-final { scan-assembler {_ZTS7Visitor:} } } +// { dg-final { scan-assembler {_ZTV7Visitor:} { xfail arm*-*-* } } } +// { dg-final { scan-assembler {_ZTI7Visitor:} { xfail arm*-*-* } } } +// { dg-final { scan-assembler {_ZTS7Visitor:} { xfail arm*-*-* } } }
On Apr 5, 2022, Alexandre Oliva <oliva@adacore.com> wrote: > Would something like this be acceptable/desirable? It's overreaching, > in that not all arm platforms are expected to fail, but the result on > them will be an unexpected pass, which is not quite as bad as the > unexpected fail we get on most arm variants now. Ping? https://gcc.gnu.org/pipermail/gcc-patches/2022-April/592763.html [PR105224] C++ modules and AAPCS/ARM EABI clash on inline key methods g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets that use the AAPCS variant. ARM is the only target that overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way the clash between AAPCS and C++ Modules design should be resolved, but currently it favors AAPCS and thus the test fails. Skipping the test or conditionally dropping the inline keyword breaks subsequent tests, so I'm XFAILing the expectation that vtable and rtti symbols are output on arm*-*-*. Retested on arm-vxworks7 (gcc-12) and arm-eabi (trunk). Ok to install? for gcc/testsuite/ChangeLog PR c++/105224 * g++.dg/modules/virt-2_a.C: XFAIL syms on arm*-*-*. --- gcc/testsuite/g++.dg/modules/virt-2_a.C | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C index 580552be5a0d8..b265515e2c7fd 100644 --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C @@ -22,6 +22,6 @@ export int Visit (Visitor *v) } // Emit here -// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} } } -// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} } } -// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} } } +// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} { xfail arm*-*-* } } } +// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} { xfail arm*-*-* } } } +// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} { xfail arm*-*-* } } }
On 17/02/2023 06:09, Alexandre Oliva via Gcc-patches wrote: > On Apr 5, 2022, Alexandre Oliva <oliva@adacore.com> wrote: > >> Would something like this be acceptable/desirable? It's overreaching, >> in that not all arm platforms are expected to fail, but the result on >> them will be an unexpected pass, which is not quite as bad as the >> unexpected fail we get on most arm variants now. > > Ping? > https://gcc.gnu.org/pipermail/gcc-patches/2022-April/592763.html > > [PR105224] C++ modules and AAPCS/ARM EABI clash on inline key methods > > g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets > that use the AAPCS variant. ARM is the only target that overrides > TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way > the clash between AAPCS and C++ Modules design should be resolved, but > currently it favors AAPCS and thus the test fails. > > Skipping the test or conditionally dropping the inline keyword breaks > subsequent tests, so I'm XFAILing the expectation that vtable and rtti > symbols are output on arm*-*-*. > > Retested on arm-vxworks7 (gcc-12) and arm-eabi (trunk). Ok to install? > I started looking at this a few weeks back, but I was a bit confused by the testcase and then never got around to following up. The Arm C++ binding rules normally exclude using an inline function definition from being chosen as the key function because this not uncommonly appears in a header file; instead a later function in the class is defined to take that role, if such a function exists (in effect an inline function is treated the same way as if the function definition appeared within the class definition itself). But in this class we have only the one function, so in effect this testcase appears to fall back to the 'no key function' rule and as such I'd expect the class impedimenta to be required in all instances of the function. That doesn't seem to be happening, so either there's something I'm missing, or there's something the compiler is doing wrong for this case. Nathan, your insights would be appreciated here. R. > > for gcc/testsuite/ChangeLog > > PR c++/105224 > * g++.dg/modules/virt-2_a.C: XFAIL syms on arm*-*-*. > --- > gcc/testsuite/g++.dg/modules/virt-2_a.C | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C > index 580552be5a0d8..b265515e2c7fd 100644 > --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C > +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C > @@ -22,6 +22,6 @@ export int Visit (Visitor *v) > } > > // Emit here > -// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} } } > -// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} } } > -// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} } } > +// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} { xfail arm*-*-* } } } > +// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} { xfail arm*-*-* } } } > +// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} { xfail arm*-*-* } } } > >
On 21/02/2023 16:31, Richard Earnshaw via Gcc-patches wrote: > On 17/02/2023 06:09, Alexandre Oliva via Gcc-patches wrote: >> On Apr 5, 2022, Alexandre Oliva <oliva@adacore.com> wrote: >> >>> Would something like this be acceptable/desirable? It's overreaching, >>> in that not all arm platforms are expected to fail, but the result on >>> them will be an unexpected pass, which is not quite as bad as the >>> unexpected fail we get on most arm variants now. >> >> Ping? >> https://gcc.gnu.org/pipermail/gcc-patches/2022-April/592763.html >> >> [PR105224] C++ modules and AAPCS/ARM EABI clash on inline key methods >> >> g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets >> that use the AAPCS variant. ARM is the only target that overrides >> TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way >> the clash between AAPCS and C++ Modules design should be resolved, but >> currently it favors AAPCS and thus the test fails. >> >> Skipping the test or conditionally dropping the inline keyword breaks >> subsequent tests, so I'm XFAILing the expectation that vtable and rtti >> symbols are output on arm*-*-*. >> >> Retested on arm-vxworks7 (gcc-12) and arm-eabi (trunk). Ok to install? >> > > I started looking at this a few weeks back, but I was a bit confused by > the testcase and then never got around to following up. > > The Arm C++ binding rules normally exclude using an inline function > definition from being chosen as the key function because this not > uncommonly appears in a header file; instead a later function in the > class is defined to take that role, if such a function exists (in effect > an inline function is treated the same way as if the function definition > appeared within the class definition itself). > > But in this class we have only the one function, so in effect this > testcase appears to fall back to the 'no key function' rule and as such > I'd expect the class impedimenta to be required in all instances of the > function. That doesn't seem to be happening, so either there's > something I'm missing, or there's something the compiler is doing wrong > for this case. > > Nathan, your insights would be appreciated here. > > R. > > >> >> for gcc/testsuite/ChangeLog >> >> PR c++/105224 >> * g++.dg/modules/virt-2_a.C: XFAIL syms on arm*-*-*. >> --- >> gcc/testsuite/g++.dg/modules/virt-2_a.C | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C >> b/gcc/testsuite/g++.dg/modules/virt-2_a.C >> index 580552be5a0d8..b265515e2c7fd 100644 >> --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C >> +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C >> @@ -22,6 +22,6 @@ export int Visit (Visitor *v) >> } >> // Emit here >> -// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} } } >> -// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} } } >> -// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} } } >> +// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} { xfail arm*-*-* >> } } } >> +// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} { xfail arm*-*-* >> } } } >> +// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} { xfail arm*-*-* >> } } } >> Rather than scanning for the triplet, a better test would be { xfail { arm_eabi } } Or something along those lines. R. >>
On 2/21/23 11:31, Richard Earnshaw wrote: > I started looking at this a few weeks back, but I was a bit confused by the > testcase and then never got around to following up. > > The Arm C++ binding rules normally exclude using an inline function definition > from being chosen as the key function because this not uncommonly appears in a > header file; instead a later function in the class is defined to take that role, > if such a function exists (in effect an inline function is treated the same way > as if the function definition appeared within the class definition itself). > > But in this class we have only the one function, so in effect this testcase > appears to fall back to the 'no key function' rule and as such I'd expect the > class impedimenta to be required in all instances of the function. That doesn't > seem to be happening, so either there's something I'm missing, or there's > something the compiler is doing wrong for this case. > > Nathan, your insights would be appreciated here. Right in the ARM ABI we'll be emitting the vtables etc in any TU that might need them. IIUC that's any TU that creates (or destroys?) an object of type Visitor (or derived from there). That source file does not do that. I see I didn't add a testcase where the only vfunc was declared inline in the class itself. Thus there's no generic-abi equivalent of ARM's behaviour. I don't think arm's behavior should be an xfail, instead restrict the checks to !arm-eabi nathan > > R. > > >> >> for gcc/testsuite/ChangeLog >> >> PR c++/105224 >> * g++.dg/modules/virt-2_a.C: XFAIL syms on arm*-*-*. >> --- >> gcc/testsuite/g++.dg/modules/virt-2_a.C | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C >> b/gcc/testsuite/g++.dg/modules/virt-2_a.C >> index 580552be5a0d8..b265515e2c7fd 100644 >> --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C >> +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C >> @@ -22,6 +22,6 @@ export int Visit (Visitor *v) >> } >> // Emit here >> -// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} } } >> -// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} } } >> -// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} } } >> +// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} { xfail arm*-*-* } } } >> +// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} { xfail arm*-*-* } } } >> +// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} { xfail arm*-*-* } } } >> >>
On Feb 21, 2023, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote: > Rather than scanning for the triplet, a better test would be > { xfail { arm_eabi } } Indeed, thanks. Here's the updated patch, retested. Ok to install? [PR105224] C++ modules and AAPCS/ARM EABI clash on inline key methods From: Alexandre Oliva <oliva@adacore.com> g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets that use the AAPCS variant. ARM is the only target that overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way the clash between AAPCS and C++ Modules design should be resolved, but currently it favors AAPCS and thus the test fails. Skipping the test or conditionally dropping the inline keyword breaks subsequent tests, so I'm XFAILing the expectation that vtable and rtti symbols are output on arm_eabi targets. for gcc/testsuite/ChangeLog PR c++/105224 * g++.dg/modules/virt-2_a.C: XFAIL syms on arm_eabi. --- gcc/testsuite/g++.dg/modules/virt-2_a.C | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C index 580552be5a0d8..f5d68878f50fb 100644 --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C @@ -22,6 +22,6 @@ export int Visit (Visitor *v) } // Emit here -// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} } } -// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} } } -// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} } } +// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} { xfail { arm_eabi } } } } +// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} { xfail { arm_eabi } } } } +// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} { xfail { arm_eabi } } } }
On 22/02/2023 19:57, Alexandre Oliva wrote: > On Feb 21, 2023, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote: > >> Rather than scanning for the triplet, a better test would be > >> { xfail { arm_eabi } } > > Indeed, thanks. Here's the updated patch, retested. Ok to install? Based on Nathan's comments, we should just skip the test on arm_eabi, it's simply not applicable. R. > > > [PR105224] C++ modules and AAPCS/ARM EABI clash on inline key methods > > From: Alexandre Oliva <oliva@adacore.com> > > g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets > that use the AAPCS variant. ARM is the only target that overrides > TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way > the clash between AAPCS and C++ Modules design should be resolved, but > currently it favors AAPCS and thus the test fails. > > Skipping the test or conditionally dropping the inline keyword breaks > subsequent tests, so I'm XFAILing the expectation that vtable and rtti > symbols are output on arm_eabi targets. > > > for gcc/testsuite/ChangeLog > > PR c++/105224 > * g++.dg/modules/virt-2_a.C: XFAIL syms on arm_eabi. > --- > gcc/testsuite/g++.dg/modules/virt-2_a.C | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C > index 580552be5a0d8..f5d68878f50fb 100644 > --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C > +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C > @@ -22,6 +22,6 @@ export int Visit (Visitor *v) > } > > // Emit here > -// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} } } > -// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} } } > -// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} } } > +// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} { xfail { arm_eabi } } } } > +// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} { xfail { arm_eabi } } } } > +// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} { xfail { arm_eabi } } } } > >
On Feb 23, 2023, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote: > On 22/02/2023 19:57, Alexandre Oliva wrote: >> On Feb 21, 2023, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote: >> >>> Rather than scanning for the triplet, a better test would be >> >>> { xfail { arm_eabi } } >> >> Indeed, thanks. Here's the updated patch, retested. Ok to install? > Based on Nathan's comments, we should just skip the test on arm_eabi, > it's simply not applicable. Like this, I suppose. Retested on x86_64-linux-gnu (trunk) and arm-wrs-vxworks7 (gcc-12). Ok to install? [PR105224] C++ modules and AAPCS/ARM EABI clash on inline key methods From: Alexandre Oliva <oliva@adacore.com> g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets that use the AAPCS variant. ARM is the only target that overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way the clash between AAPCS and C++ Modules design should be resolved, but currently it favors AAPCS and thus the test fails, so skip it on arm_eabi. for gcc/testsuite/ChangeLog PR c++/105224 * g++.dg/modules/virt-2_a.C: Skip on arm_eabi. --- gcc/testsuite/g++.dg/modules/virt-2_a.C | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C index 580552be5a0d8..8fa42d97d72d7 100644 --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C @@ -1,4 +1,6 @@ -// { dg-module-do run } +// AAPCS overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE, +// in a way that invalidates this test. +// { dg-module-do run { target { ! arm_eabi } } } // { dg-additional-options -fmodules-ts } export module foo; // { dg-module-cmi foo }
On Feb 23, 2023, Alexandre Oliva <oliva@adacore.com> wrote: > On Feb 23, 2023, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote: >> On 22/02/2023 19:57, Alexandre Oliva wrote: >>> On Feb 21, 2023, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote: >>> >>>> Rather than scanning for the triplet, a better test would be >>> >>>> { xfail { arm_eabi } } >>> >>> Indeed, thanks. Here's the updated patch, retested. Ok to install? >> Based on Nathan's comments, we should just skip the test on arm_eabi, >> it's simply not applicable. > Like this, I suppose. Retested on x86_64-linux-gnu (trunk) and > arm-wrs-vxworks7 (gcc-12). Ok to install? Erhm, actually, that version still ran the assembler scans and failed. This one skips the testset entirely. [PR105224] C++ modules and AAPCS/ARM EABI clash on inline key methods From: Alexandre Oliva <oliva@adacore.com> g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets that use the AAPCS variant. ARM is the only target that overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way the clash between AAPCS and C++ Modules design should be resolved, but currently it favors AAPCS and thus the test fails, so skip it on arm_eabi. for gcc/testsuite/ChangeLog PR c++/105224 * g++.dg/modules/virt-2_a.C: Skip on arm_eabi. --- gcc/testsuite/g++.dg/modules/virt-2_a.C | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C index 580552be5a0d8..ede711c3e83be 100644 --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C @@ -1,3 +1,6 @@ +// AAPCS overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE, +// in a way that invalidates this test. +// { dg-skip-if "TARGET_CXX_KEY_METHOD_MAY_BE_INLINE" { arm_eabi } } // { dg-module-do run } // { dg-additional-options -fmodules-ts } export module foo;
On 23/02/2023 21:20, Alexandre Oliva wrote: > On Feb 23, 2023, Alexandre Oliva <oliva@adacore.com> wrote: > >> On Feb 23, 2023, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote: >>> On 22/02/2023 19:57, Alexandre Oliva wrote: >>>> On Feb 21, 2023, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote: >>>> >>>>> Rather than scanning for the triplet, a better test would be >>>> >>>>> { xfail { arm_eabi } } >>>> >>>> Indeed, thanks. Here's the updated patch, retested. Ok to install? > >>> Based on Nathan's comments, we should just skip the test on arm_eabi, >>> it's simply not applicable. > >> Like this, I suppose. Retested on x86_64-linux-gnu (trunk) and >> arm-wrs-vxworks7 (gcc-12). Ok to install? > > Erhm, actually, that version still ran the assembler scans and failed. > This one skips the testset entirely. Yeah, I tried something like that and it didn't appear to work. Perhaps it's a bug in the way dg-do-module is implemented. > > > [PR105224] C++ modules and AAPCS/ARM EABI clash on inline key methods > > From: Alexandre Oliva <oliva@adacore.com> > > g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets > that use the AAPCS variant. ARM is the only target that overrides > TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way > the clash between AAPCS and C++ Modules design should be resolved, but > currently it favors AAPCS and thus the test fails, so skip it on > arm_eabi. > > > for gcc/testsuite/ChangeLog > > PR c++/105224 > * g++.dg/modules/virt-2_a.C: Skip on arm_eabi. > --- > gcc/testsuite/g++.dg/modules/virt-2_a.C | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C > index 580552be5a0d8..ede711c3e83be 100644 > --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C > +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C > @@ -1,3 +1,6 @@ > +// AAPCS overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE, > +// in a way that invalidates this test. > +// { dg-skip-if "TARGET_CXX_KEY_METHOD_MAY_BE_INLINE" { arm_eabi } } Given the logic of this macro, the text should be "!TARGET_CXX_METHOD_MAY_BE_INLINE". OK with that change. R. > // { dg-module-do run } > // { dg-additional-options -fmodules-ts } > export module foo; > >
> On 24 Feb 2023, at 10:23, Richard Earnshaw via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > On 23/02/2023 21:20, Alexandre Oliva wrote: >> On Feb 23, 2023, Alexandre Oliva <oliva@adacore.com> wrote: >>> On Feb 23, 2023, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote: >>>> On 22/02/2023 19:57, Alexandre Oliva wrote: >>>>> On Feb 21, 2023, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote: >>>>> >>>>>> Rather than scanning for the triplet, a better test would be >>>>> >>>>>> { xfail { arm_eabi } } >>>>> >>>>> Indeed, thanks. Here's the updated patch, retested. Ok to install? >>>> Based on Nathan's comments, we should just skip the test on arm_eabi, >>>> it's simply not applicable. >>> Like this, I suppose. Retested on x86_64-linux-gnu (trunk) and >>> arm-wrs-vxworks7 (gcc-12). Ok to install? >> Erhm, actually, that version still ran the assembler scans and failed. >> This one skips the testset entirely. > > Yeah, I tried something like that and it didn't appear to work. Perhaps it's a bug in the way dg-do-module is implemented. I think if you suppress the dg-do run line (with the target selector) then it will just do the default (which is to compile only?) Skip seems like the correct thing to do here .. Iain > >> [PR105224] C++ modules and AAPCS/ARM EABI clash on inline key methods >> From: Alexandre Oliva <oliva@adacore.com> >> g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets >> that use the AAPCS variant. ARM is the only target that overrides >> TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way >> the clash between AAPCS and C++ Modules design should be resolved, but >> currently it favors AAPCS and thus the test fails, so skip it on >> arm_eabi. >> for gcc/testsuite/ChangeLog >> PR c++/105224 >> * g++.dg/modules/virt-2_a.C: Skip on arm_eabi. >> --- >> gcc/testsuite/g++.dg/modules/virt-2_a.C | 3 +++ >> 1 file changed, 3 insertions(+) >> diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C >> index 580552be5a0d8..ede711c3e83be 100644 >> --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C >> +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C >> @@ -1,3 +1,6 @@ >> +// AAPCS overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE, >> +// in a way that invalidates this test. >> +// { dg-skip-if "TARGET_CXX_KEY_METHOD_MAY_BE_INLINE" { arm_eabi } } > > Given the logic of this macro, the text should be "!TARGET_CXX_METHOD_MAY_BE_INLINE". > > OK with that change. > > R. > >> // { dg-module-do run } >> // { dg-additional-options -fmodules-ts } >> export module foo;
On Feb 24, 2023, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote: > Given the logic of this macro, the text should be > "!TARGET_CXX_METHOD_MAY_BE_INLINE". I was thinking just "related to that macro", but yeah, negating it makes sense. > OK with that change. Thanks, here's what I'm checking in. [PR105224] C++ modules and AAPCS/ARM EABI clash on inline key methods From: Alexandre Oliva <oliva@adacore.com> g++.dg/modules/virt-2_a.C fails on arm-eabi and many other arm targets that use the AAPCS variant. ARM is the only target that overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE. It's not clear to me which way the clash between AAPCS and C++ Modules design should be resolved, but currently it favors AAPCS and thus the test fails, so skip it on arm_eabi. for gcc/testsuite/ChangeLog PR c++/105224 * g++.dg/modules/virt-2_a.C: Skip on arm_eabi. --- gcc/testsuite/g++.dg/modules/virt-2_a.C | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C index 580552be5a0d8..b5050445c3f15 100644 --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C @@ -1,3 +1,6 @@ +// AAPCS overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE, +// in a way that invalidates this test. +// { dg-skip-if "!TARGET_CXX_KEY_METHOD_MAY_BE_INLINE" { arm_eabi } } // { dg-module-do run } // { dg-additional-options -fmodules-ts } export module foo;
diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C index 9115cc19cc286..40f30137f0086 100644 --- a/gcc/testsuite/g++.dg/modules/virt-2_a.C +++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C @@ -10,7 +10,9 @@ export struct Visitor // Key function explicitly inline (regardless of p1779's state) // We emit vtables & rtti only in this TU +#if !__ARM_EABI__ inline // Yoink! +#endif int Visitor::Visit () { return 0;