[PATCH][AArch64] Skip gcc.target/aarch64/assembler_arch_1.c if assembler does not support it

[Christophe Lyon]

On 16 February 2016 at 10:53, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> On Tue, Feb 16, 2016 at 09:27:00AM +0000, Kyrill Tkachov wrote:
>> Hi all,
>>
>> As Christophe reported at:
>> https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00784.html
>>
>> The test gcc.target/aarch64/assembler_arch_1.c fails to assemble on older
>> assemblers that don't support the LSE architecture extension.
>
> Actually, Christophe's report is that the ".arch_extension" directive
> isn't understood by older assemblers...
>
>> I'd really like to keep the test an assemble test rather than just a compile
>> one since it is extracted from a real error when building the Linux kernel,
>> so this patch is the simplest way I can think of skipping the test for older
>> assemblers.
>>
>> I've tested that the test appears UNSUPPORTED when used with a binutils that
>> doesn't support LSE and PASSes normally on latest binutils.
>>
>> Do we want to have such a specialised check for assembler capabilities?  Or
>> do we want to just let this test FAIL on older assemblers and see this
>> failure go away in the future as users/testers migrate to later binutils
>> releases?
>
> ...so, this test conflates two features we're asking for from the assembler,
> support for LSE and support for .arch_extension. For your test, that's
> exactly what you need, and for binutils it will never matter, but it feels
> like a misnamed check to me.
>
>> If such a check is desired, here's a patch that does it.
>
> Yes, the check is probably useful (if we need to test other LSE features, we
> will want it - e.g. if we wanted to bump some of the armv8.1+lse atomics
> tests up to assemble tests).
>
> Anyway, this patch is OK by me as is, as I don't see a way to disambiguate
> the test name without ending up with two tiny effective-target checks. But
> maybe someone who knows the testsuite better would have other opinions?
>
I agree this is useful, and it would probably be overkill to split this further.

Thanks for taking care of that.

>> 2016-02-16  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>     * lib/target-supports.exp
>>     (check_effective_target_aarch64_lse_assembler): New effective target
>>     check.
>>     * gcc.target/aarch64/assembler_arch_1.c: Add arch64_lse_assembler
>>     effective target check.
>
>> diff --git a/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c b/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
>> index 901e50a178d7a4a443a5ad0abe63f624688db268..ef34e76cef9c3076a3c32e76aa7a7f658774f786 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
>> @@ -1,4 +1,5 @@
>>  /* { dg-do assemble } */
>> +/* { dg-require-effective-target aarch64_lse_assembler } */
>>  /* { dg-options "-march=armv8-a" } */
>>
>>  /* Make sure that the function header in assembly doesn't override
>> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
>> index 645981a8733b68a7c37919a53b324841d2c125aa..08b4eacc26f44506d00fda58f99c203a4199856b 100644
>> --- a/gcc/testsuite/lib/target-supports.exp
>> +++ b/gcc/testsuite/lib/target-supports.exp
>> @@ -6718,6 +6718,18 @@ proc check_effective_target_aarch64_tiny { } {
>>      }
>>  }
>>
>> +# Return 1 if the target AArch64 assembler supports the LSE extensions.
>
> I think the "extensions" here is redundant :-).
>
> Thanks,
> James
>
>