On 07/05/2025 18:21, Richard Sandiford wrote:
> Richard Earnshaw <richard.earns...@arm.com> writes:
>> 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...
>
> Richard and I discussed this more off-list, and it turns out that the
> above construct started to work after:
>
> https://git.savannah.gnu.org/gitweb/?p=dejagnu.git;a=commit;h=569f8718b534a2cd9511a7d640352eb0126ff492
>
> which was first releasted in 1.6 (9 years ago). Before that, the "what"
> (compile/assemble/etc.) in the last dg-do won, regardless of whether
> the dg-do was selected or deselected.
>
> I suppose the question is whether we can reasonably assume that people
> are using dejagnu 1.6+ or whether we need to support older dejagnus.
>
> It looks like Alex added dg-do-if (in
> e6f5fadec5f6a719145ed2ed513209ec3e8eeb2f)
> to support older dejagnu, so that's an option. I.e.:
>
> /* { dg-do compile } */
> /* { dg-do-if assemble { target aarch64_asm_cmpbr_ok } } */
>
> Which if it works (haven't tried!) also avoids having to specify the
> selector twice. Not sure whether it's worth going back and changing
> all existing aarch64 tests to this style though.
If the default action for the directory is "compile", as it often is, you don't
even need the initial dg-do.
R.
>
> TIL :) Thanks Richard for bringing it up.
>
> Richard
