Re: C++ modules and AAPCS/ARM EABI clash on inline key methods

[Richard Earnshaw <Richard.Earnshaw@foss.arm.com>]

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.
>>