Stam Markianos-Wright <stam.markianos-wri...@arm.com> writes:
> On 12/13/19 11:15 AM, Richard Sandiford wrote:
>> Stam Markianos-Wright <stam.markianos-wri...@arm.com> writes:
>>> Hi all,
>>>
>>> This small patch adds support for the ARM v8.6 extensions +bf16 and
>>> +i8mm to the testsuite. This will be tested through other upcoming
>>> patches, which is why we are not providing any explicit tests here.
>>>
>>> Ok for trunk?
>>>
>>> Also I don't have commit rights, so if someone could commit on my
>>> behalf, that would be great :)
>>>
>>> The functionality here depends on CLI patches:
>>> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02415.html
>>> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02195.html
>>>
>>> but this patch applies cleanly without them, too.
>>>
>>> Cheers,
>>> Stam
>>>
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2019-12-11 Stam Markianos-Wright <stam.markianos-wri...@arm.com>
>>>
>>> * lib/target-supports.exp
>>> (check_effective_target_arm_v8_2a_i8mm_ok_nocache): New.
>>> (check_effective_target_arm_v8_2a_i8mm_ok): New.
>>> (add_options_for_arm_v8_2a_i8mm): New.
>>> (check_effective_target_arm_v8_2a_bf16_neon_ok_nocache): New.
>>> (check_effective_target_arm_v8_2a_bf16_neon_ok): New.
>>> (add_options_for_arm_v8_2a_bf16_neon): New.
>>
>> The new effective-target keywords need to be documented in
>> doc/sourcebuild.texi.
>
> Added in new diff :)
>
>>
>> LGTM otherwise. For:
>>
>>> diff --git a/gcc/testsuite/lib/target-supports.exp
>>> b/gcc/testsuite/lib/target-supports.exp
>>> index 5b4cc02f921..36fb63e9929 100644
>>> --- a/gcc/testsuite/lib/target-supports.exp
>>> +++ b/gcc/testsuite/lib/target-supports.exp
>>> @@ -4781,6 +4781,49 @@ proc add_options_for_arm_v8_2a_dotprod_neon { flags
>>> } {
>>> return "$flags $et_arm_v8_2a_dotprod_neon_flags"
>>> }
>>>
>>> +# Return 1 if the target supports ARMv8.2+i8mm Adv.SIMD Dot Product
>>> +# instructions, 0 otherwise. The test is valid for ARM and for AArch64.
>>> +# Record the command line options needed.
>>> +
>>> +proc check_effective_target_arm_v8_2a_i8mm_ok_nocache { } {
>>> + global et_arm_v8_2a_i8mm_flags
>>> + set et_arm_v8_2a_i8mm_flags ""
>>> +
>>> + if { ![istarget arm*-*-*] && ![istarget aarch64*-*-*] } {
>>> + return 0;
>>> + }
>>> +
>>> + # Iterate through sets of options to find the compiler flags that
>>> + # need to be added to the -march option.
>>> + foreach flags {"" "-mfloat-abi=hard -mfpu=neon-fp-armv8"
>>> "-mfloat-abi=softfp -mfpu=neon-fp-armv8" } {
>>> + if { [check_no_compiler_messages_nocache \
>>> + arm_v8_2a_i8mm_ok object {
>>> + #include <arm_neon.h>
>>> + #if !defined (__ARM_FEATURE_MATMUL_INT8)
>>> + #error "__ARM_FEATURE_MATMUL_INT8 not defined"
>>> + #endif
>>> + } "$flags -march=armv8.2-a+i8mm"] } {
>>> + set et_arm_v8_2a_i8mm_flags "$flags -march=armv8.2-a+i8mm"
>>> + return 1
>>> + }
>>> + }
>>
>> I wondered whether it would be better to add no options if testing
>> with something that already supports i8mm (e.g. -march=armv8.6).
>> That would mean trying:
>>
>> "" "-march=armv8.2-a+i8mm" "-march=armv8.2-a+i8mm -mfloat-abi..." ...
>>
>> instead. But there are arguments both ways, and the above follows
>> existing style, so OK.
>
> Not quite sure if I understanding this right, but I think that's what
> the "" option in foreach flags{} is for?
>
> i.e. currently what I'm seeing is:
>
> +/* { dg-require-effective-target arm_v8_2a_i8mm_ok } */
> +/* { dg-add-options arm_v8_2a_i8mm } */
>
> will pull through the first option that compiles to object file with no
> errors (check_no_compiler_messages_nocache arm_v8_2a_i8mm_ok object).
>
> So in a lot of cases it should just be fine for "" and only pull in
> -march=armv8.2-a+i8mm.
>
> I think that's right? Lmk if I'm not reading it properly!
Yeah, that's right, but it's also the "problem". The point was that
some people will run the tests with options like -march=armv8.6-a that
already support these instructions, e.g. using
--target_board unix/-march=armv8.6-a
on a native box. The code above will then override that -march option
with -march=armv8.2-a+i8mm even though the original option was OK.
The tests won't then actually get run with -march=armv8.6-a as the
dominant option, despite being Armv8.6 tests. :-)
The alternative above would have tried with no options at all and only
added -march= if that failed. But like I say, the current version
follows existing practice and so is fine too.
> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
> index 85573a49a2b..73408d12cbe 100644
> --- a/gcc/doc/sourcebuild.texi
> +++ b/gcc/doc/sourcebuild.texi
> @@ -1877,6 +1877,18 @@ ARM target supports extensions to generate the
> @code{VFMAL} and @code{VFMLS}
> half-precision floating-point instructions available from ARMv8.2-A and
> onwards. Some multilibs may be incompatible with these options.
>
> +@item arm_v8_2a_bf16_neon_ok
> +@anchor{arm_v8_2a_bf16_neon_ok}
> +ARM target supports options to generate instructions from ARMv8.2-A with
> +the BFloat16 extension (bf16). Some multilibs may be incompatible with these
> +options.
> +
> +@item arm_v8_2a_i8mm_ok
> +@anchor{arm_v8_2a_i8mm_ok}
> +ARM target supports options to generate instructions from ARMv8.2-A with
> +the 8-Bit Integer Matrix Multiply extension (i8mm). Some multilibs may be
> +incompatible with these options.
> +
> @item arm_prefer_ldrd_strd
> ARM target prefers @code{LDRD} and @code{STRD} instructions over
> @code{LDM} and @code{STM} instructions.
No need for the @anchors, since nothing wants to link to them yet.
OK with that change, thanks. Please follow the process on
https://gcc.gnu.org/svnwrite.html to get write access.
Richard
