On 07/05/2025 17:28, Richard Earnshaw (lists) wrote:
> On 07/05/2025 16:54, Richard Sandiford wrote:
>> Richard Earnshaw <richard.earns...@arm.com> writes:
>>> On 07/05/2025 13:57, Richard Sandiford wrote:
>>>> Kyrylo Tkachov <ktkac...@nvidia.com> writes:
>>>>>> On 7 May 2025, at 12:27, Karl Meakin <karl.mea...@arm.com> wrote:
>>>>>>
>>>>>> Commit the test file `cmpbr.c` before rules for generating the new
>>>>>> instructions are added, so that the changes in codegen are more obvious
>>>>>> in the next commit.
>>>>>
>>>>> I guess that’s an LLVM best practice.
>>>>> In GCC since we have the check-function-bodies mechanism we usually
>>>>> prefer to include the relevant test together with the patch that adds the
>>>>> optimization.
>>>>> But this is not wrong either.
>>>>>
>>>>>
>>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>> * gcc.target/aarch64/cmpbr.c: New test.
>>>>>> ---
>>>>>> gcc/testsuite/gcc.target/aarch64/cmpbr.c | 1378 ++++++++++++++++++++++
>>>>>> 1 file changed, 1378 insertions(+)
>>>>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/cmpbr.c
>>>>>>
>>>>>> diff --git a/gcc/testsuite/gcc.target/aarch64/cmpbr.c
>>>>>> b/gcc/testsuite/gcc.target/aarch64/cmpbr.c
>>>>>> new file mode 100644
>>>>>> index 00000000000..728d6ead91c
>>>>>> --- /dev/null
>>>>>> +++ b/gcc/testsuite/gcc.target/aarch64/cmpbr.c
>>>>>> @@ -0,0 +1,1378 @@
>>>>>> +/* Test that the instructions added by FEAT_CMPBR are emitted */
>>>>>> +/* { dg-do compile } */
>>>>>> +/* { dg-options "-march=armv9.5-a+cmpbr -O2" } */
>>>>>> +/* { dg-final { check-function-bodies "**" "" "" } } */
>>>>>
>>>>> As you’ll be adding new instructions to the compiler it’d be good to have
>>>>> it a dg-do assemble test where possible.
>>>>
>>>> Agreed FWIW, but:
>>>>
>>>>> For that you’ll need to create a new aarch64_asm_cmpbr_ok target and use
>>>>> it like so to fallback to dg-do compile when the assembler is too old:
>>>>> /* { dg-do compile { target aarch64_asm_cmpbr_ok } } */
>>>>
>>>> ...dg-do assemble for this one :)
>>>
>>> I don't think that works. If the first dg-do fails the test is just skipped.
>>>
>>> You need to replicate the test with separate dg-do directives, IIRC.
>>
>> Hmm, can you remember the circumstances when you saw that?
>> We've been using the construct that Kyrill suggested with apparent
>> success in things like aarch64-sve2-acle-asm.exp. E.g.:
>
> Well, the implementation of dg-do contains the comment:
>
> # Note: A previous occurrence of `dg-do' with target/xfail selectors
> # is a user mistake. We clobber previous values here.
>
> So one might interpret that as meaning multiple dg-do's are not intended to
> be supported.
>
> But I might have misremembered the exact scenario I was facing. I think it
> might have been that a test failed to fall back to the dg-do-default if a
> specific dg-do didn't match. The scenario I remember was something like
> dg-do-default = compile, then the test was trying to change that to execute
> if HW was available; but that meant that if it wasn't we didn't fall back to
> checking the assembler output.
>
The comment at the head of the function says:
# Multiple instances are supported (since we don't support target and xfail
# selectors on one line), though it doesn't make much sense to change the
# compile/assemble/link/run field. Nor does it make any sense to have
# multiple lines of target selectors (use one line).
So maybe the code is intended to support multiple reasons for skipping the test
(but why not use require-effective-target
for that).
I'm not sure now what's going on...
R.
> R.
>
>>
>> /* { dg-do assemble { target aarch64_asm_sve2p1_ok } } */
>> /* { dg-do compile { target { ! aarch64_asm_sve2p1_ok } } } */
>> /* { dg-final { check-function-bodies "**" "" "-DCHECK_ASM" } } */
>>
>> If I run this normally, it picks the assemble route (tests run with -c).
>> If I force aarch64_asm_sve2p1_ok to false by mangling the test
>> instruction, the test picks the compile route (tests with run -S).
>>
>> Thanks,
>> Richard
>
