* C++ modules and AAPCS/ARM EABI clash on inline key methods @ 2022-03-31 7:32 Alexandre Oliva 2022-04-05 4:53 ` Alexandre Oliva 0 siblings, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2022-03-31 7:32 UTC (permalink / raw) To: gcc-patches; +Cc: nathan 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? 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; -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Ask me about <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2022-03-31 7:32 C++ modules and AAPCS/ARM EABI clash on inline key methods Alexandre Oliva @ 2022-04-05 4:53 ` Alexandre Oliva 2023-02-17 6:09 ` Alexandre Oliva 0 siblings, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2022-04-05 4:53 UTC (permalink / raw) To: gcc-patches; +Cc: nathan 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*-*-* } } } -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Ask me about <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2022-04-05 4:53 ` Alexandre Oliva @ 2023-02-17 6:09 ` Alexandre Oliva 2023-02-21 16:31 ` Richard Earnshaw 0 siblings, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2023-02-17 6:09 UTC (permalink / raw) To: gcc-patches; +Cc: nathan, nickc, richard.earnshaw, ramana.gcc, kyrylo.tkachov 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*-*-* } } } -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Ask me about <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2023-02-17 6:09 ` Alexandre Oliva @ 2023-02-21 16:31 ` Richard Earnshaw 2023-02-21 16:48 ` Richard Earnshaw 2023-02-21 22:27 ` Nathan Sidwell 0 siblings, 2 replies; 13+ messages in thread From: Richard Earnshaw @ 2023-02-21 16:31 UTC (permalink / raw) To: Alexandre Oliva, gcc-patches Cc: nathan, nickc, richard.earnshaw, ramana.gcc, kyrylo.tkachov 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*-*-* } } } > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2023-02-21 16:31 ` Richard Earnshaw @ 2023-02-21 16:48 ` Richard Earnshaw 2023-02-22 19:57 ` Alexandre Oliva 2023-02-21 22:27 ` Nathan Sidwell 1 sibling, 1 reply; 13+ messages in thread From: Richard Earnshaw @ 2023-02-21 16:48 UTC (permalink / raw) To: Alexandre Oliva, gcc-patches Cc: nathan, nickc, richard.earnshaw, ramana.gcc, kyrylo.tkachov 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. >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2023-02-21 16:48 ` Richard Earnshaw @ 2023-02-22 19:57 ` Alexandre Oliva 2023-02-23 10:14 ` Richard Earnshaw 0 siblings, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2023-02-22 19:57 UTC (permalink / raw) To: Richard Earnshaw Cc: gcc-patches, nathan, nickc, richard.earnshaw, ramana.gcc, kyrylo.tkachov 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 } } } } -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Ask me about <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2023-02-22 19:57 ` Alexandre Oliva @ 2023-02-23 10:14 ` Richard Earnshaw 2023-02-23 17:12 ` Alexandre Oliva 0 siblings, 1 reply; 13+ messages in thread From: Richard Earnshaw @ 2023-02-23 10:14 UTC (permalink / raw) To: Alexandre Oliva Cc: gcc-patches, nathan, nickc, richard.earnshaw, ramana.gcc, kyrylo.tkachov 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 } } } } > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2023-02-23 10:14 ` Richard Earnshaw @ 2023-02-23 17:12 ` Alexandre Oliva 2023-02-23 21:20 ` Alexandre Oliva 0 siblings, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2023-02-23 17:12 UTC (permalink / raw) To: Richard Earnshaw Cc: gcc-patches, nathan, nickc, richard.earnshaw, ramana.gcc, kyrylo.tkachov 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 } -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Ask me about <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2023-02-23 17:12 ` Alexandre Oliva @ 2023-02-23 21:20 ` Alexandre Oliva 2023-02-24 10:23 ` Richard Earnshaw 0 siblings, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2023-02-23 21:20 UTC (permalink / raw) To: Richard Earnshaw Cc: gcc-patches, nathan, nickc, richard.earnshaw, ramana.gcc, kyrylo.tkachov 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; -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Ask me about <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2023-02-23 21:20 ` Alexandre Oliva @ 2023-02-24 10:23 ` Richard Earnshaw 2023-02-24 10:30 ` Iain Sandoe 2023-02-24 14:39 ` Alexandre Oliva 0 siblings, 2 replies; 13+ messages in thread From: Richard Earnshaw @ 2023-02-24 10:23 UTC (permalink / raw) To: Alexandre Oliva Cc: gcc-patches, nathan, nickc, richard.earnshaw, ramana.gcc, kyrylo.tkachov 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; > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2023-02-24 10:23 ` Richard Earnshaw @ 2023-02-24 10:30 ` Iain Sandoe 2023-02-24 14:39 ` Alexandre Oliva 1 sibling, 0 replies; 13+ messages in thread From: Iain Sandoe @ 2023-02-24 10:30 UTC (permalink / raw) To: Richard Earnshaw Cc: Alexandre Oliva, GCC Patches, Nathan Sidwell, nickc, richard.earnshaw, ramana.gcc, kyrylo.tkachov > 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; ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2023-02-24 10:23 ` Richard Earnshaw 2023-02-24 10:30 ` Iain Sandoe @ 2023-02-24 14:39 ` Alexandre Oliva 1 sibling, 0 replies; 13+ messages in thread From: Alexandre Oliva @ 2023-02-24 14:39 UTC (permalink / raw) To: Richard Earnshaw Cc: gcc-patches, nathan, nickc, richard.earnshaw, ramana.gcc, kyrylo.tkachov 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; -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Ask me about <https://stallmansupport.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: C++ modules and AAPCS/ARM EABI clash on inline key methods 2023-02-21 16:31 ` Richard Earnshaw 2023-02-21 16:48 ` Richard Earnshaw @ 2023-02-21 22:27 ` Nathan Sidwell 1 sibling, 0 replies; 13+ messages in thread From: Nathan Sidwell @ 2023-02-21 22:27 UTC (permalink / raw) To: Richard Earnshaw, Alexandre Oliva, gcc-patches Cc: nickc, richard.earnshaw, ramana.gcc, kyrylo.tkachov 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*-*-* } } } >> >> -- Nathan Sidwell ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-02-24 14:40 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-31 7:32 C++ modules and AAPCS/ARM EABI clash on inline key methods Alexandre Oliva 2022-04-05 4:53 ` Alexandre Oliva 2023-02-17 6:09 ` Alexandre Oliva 2023-02-21 16:31 ` Richard Earnshaw 2023-02-21 16:48 ` Richard Earnshaw 2023-02-22 19:57 ` Alexandre Oliva 2023-02-23 10:14 ` Richard Earnshaw 2023-02-23 17:12 ` Alexandre Oliva 2023-02-23 21:20 ` Alexandre Oliva 2023-02-24 10:23 ` Richard Earnshaw 2023-02-24 10:30 ` Iain Sandoe 2023-02-24 14:39 ` Alexandre Oliva 2023-02-21 22:27 ` Nathan Sidwell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).