| Message ID | 562F98F4.7050606@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Oct 27, 2015 at 03:32:04PM +0000, Matthew Wahab wrote: > On 24/10/15 08:16, Bernhard Reutner-Fischer wrote: > >On October 23, 2015 2:24:26 PM GMT+02:00, Matthew Wahab <matthew.wahab@foss.arm.com> wrote: > >>The ARMv8.1 architecture extension adds two Adv.SIMD instructions,. > >>This > >>patch adds support in Dejagnu for ARMv8.1 Adv.SIMD specifiers and > >>checks. > >> > >>The new test options are > >>- { dg-add-options arm_v8_1a_neon }: Add compiler options needed to > >> enable ARMv8.1 Adv.SIMD. > >>- { dg-require-effective-target arm_v8_1a_neon_hw }: Require a target > >> capable of executing ARMv8.1 Adv.SIMD instructions. > >> > > > >Please error with something more meaningful than FOO, !__ARM_FEATURE_QRDMX comes to mind. > > > >TIA, > > > > I've reworked the patch so that the error is "__ARM_FEATURE_QRDMX not > defined" and also strengthened the check_effective_target tests. > > Retested for aarch64-none-elf with cross-compiled check-gcc on an > ARMv8.1 emulator. Also tested with a version of the compiler that > doesn't define the ACLE feature macro. Hi Matthew, I have a couple of comments below. Neither need to block the patch, but I'd appreciate a reply before I say OK. > From b12969882298cb79737e882c48398c58a45161b9 Mon Sep 17 00:00:00 2001 > From: Matthew Wahab <matthew.wahab@arm.com> > Date: Mon, 26 Oct 2015 14:58:36 +0000 > Subject: [PATCH 5/7] [Testsuite] Add dejagnu options for armv8.1 neon > > Change-Id: Ib58b8c4930ad3971af3ea682eda043e14cd2e8b3 > --- > gcc/testsuite/lib/target-supports.exp | 56 ++++++++++++++++++++++++++++++++++- > 1 file changed, 55 insertions(+), 1 deletion(-) > > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > index 4d5b0a3d..0fb679d 100644 > --- a/gcc/testsuite/lib/target-supports.exp > +++ b/gcc/testsuite/lib/target-supports.exp > @@ -2700,6 +2700,16 @@ proc add_options_for_arm_v8_neon { flags } { > return "$flags $et_arm_v8_neon_flags -march=armv8-a" > } > > +# Add the options needed for ARMv8.1 Adv.SIMD. > + > +proc add_options_for_arm_v8_1a_neon { flags } { > + if { [istarget aarch64*-*-*] } { > + return "$flags -march=armv8.1-a" Should this be -march=armv8.1-a+simd or some other feature flag? > + } else { > + return "$flags" > + } > +} > + > proc add_options_for_arm_crc { flags } { > if { ! [check_effective_target_arm_crc_ok] } { > return "$flags" > @@ -2984,7 +2994,8 @@ foreach { armfunc armflag armdef } { v4 "-march=armv4 -marm" __ARM_ARCH_4__ > v7r "-march=armv7-r" __ARM_ARCH_7R__ > v7m "-march=armv7-m -mthumb" __ARM_ARCH_7M__ > v7em "-march=armv7e-m -mthumb" __ARM_ARCH_7EM__ > - v8a "-march=armv8-a" __ARM_ARCH_8A__ } { > + v8a "-march=armv8-a" __ARM_ARCH_8A__ > + v8_1a "-march=armv8.1a" __ARM_ARCH_8A__ } { > eval [string map [list FUNC $armfunc FLAG $armflag DEF $armdef ] { > proc check_effective_target_arm_arch_FUNC_ok { } { > if { [ string match "*-marm*" "FLAG" ] && > @@ -3141,6 +3152,25 @@ proc check_effective_target_arm_neonv2_hw { } { > } [add_options_for_arm_neonv2 ""]] > } > > +# Return 1 if the target supports the ARMv8.1 Adv.SIMD extension, 0 > +# otherwise. The test is valid for AArch64. > + > +proc check_effective_target_arm_v8_1a_neon_ok_nocache { } { > + if { ![istarget aarch64*-*-*] } { > + return 0 > + } > + return [check_no_compiler_messages_nocache arm_v8_1a_neon_ok assembly { > + #if !defined (__ARM_FEATURE_QRDMX) > + #error "__ARM_FEATURE_QRDMX not defined" > + #endif > + } [add_options_for_arm_v8_1a_neon ""]] > +} > + > +proc check_effective_target_arm_v8_1a_neon_ok { } { > + return [check_cached_effective_target arm_v8_1a_neon_ok \ > + check_effective_target_arm_v8_1a_neon_ok_nocache] > +} > + > # Return 1 if the target supports executing ARMv8 NEON instructions, 0 > # otherwise. > > @@ -3159,6 +3189,30 @@ proc check_effective_target_arm_v8_neon_hw { } { > } [add_options_for_arm_v8_neon ""]] > } > > +# Return 1 if the target supports executing the ARMv8.1 Adv.SIMD extension, 0 > +# otherwise. The test is valid for AArch64. > + > +proc check_effective_target_arm_v8_1a_neon_hw { } { > + if { ![check_effective_target_arm_v8_1a_neon_ok] } { > + return 0; > + } > + return [check_runtime_nocache arm_v8_1a_neon_hw_available { > + int > + main (void) > + { > + long long a = 0, b = 1; > + long long result = 0; > + > + asm ("sqrdmlah %s0,%s1,%s2" > + : "=w"(result) > + : "w"(a), "w"(b) > + : /* No clobbers. */); Hm, those types look wrong, I guess this works but it is an unusual way to write it. I presume this is to avoid including arm_neon.h each time, but you could just directly use the internal type names for the arm_neon types. That is to say __Int32x4_t (or whichever mode you intend to use). > + > + return result; > + } > + } [add_options_for_arm_v8_1a_neon ""]] > +} > + > # Return 1 if this is a ARM target with NEON enabled. > > proc check_effective_target_arm_neon { } { Thanks, James
On 23/11/15 12:24, James Greenhalgh wrote: > On Tue, Oct 27, 2015 at 03:32:04PM +0000, Matthew Wahab wrote: >> On 24/10/15 08:16, Bernhard Reutner-Fischer wrote: >>> On October 23, 2015 2:24:26 PM GMT+02:00, Matthew Wahab <matthew.wahab@foss.arm.com> wrote: >>>> The ARMv8.1 architecture extension adds two Adv.SIMD instructions,. >>>> This >>>> patch adds support in Dejagnu for ARMv8.1 Adv.SIMD specifiers and >>>> checks. >>>> >>>> The new test options are >>>> - { dg-add-options arm_v8_1a_neon }: Add compiler options needed to >>>> enable ARMv8.1 Adv.SIMD. >>>> - { dg-require-effective-target arm_v8_1a_neon_hw }: Require a target >>>> capable of executing ARMv8.1 Adv.SIMD instructions. >>>> > Hi Matthew, > > I have a couple of comments below. Neither need to block the patch, but > I'd appreciate a reply before I say OK. > >> >> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp >> index 4d5b0a3d..0fb679d 100644 >> --- a/gcc/testsuite/lib/target-supports.exp >> +++ b/gcc/testsuite/lib/target-supports.exp >> @@ -2700,6 +2700,16 @@ proc add_options_for_arm_v8_neon { flags } { >> return "$flags $et_arm_v8_neon_flags -march=armv8-a" >> } >> >> +# Add the options needed for ARMv8.1 Adv.SIMD. >> + >> +proc add_options_for_arm_v8_1a_neon { flags } { >> + if { [istarget aarch64*-*-*] } { >> + return "$flags -march=armv8.1-a" > > Should this be -march=armv8.1-a+simd or some other feature flag? > I think it should by armv8.1-a only. +simd is enabled by all -march settings so it seems redundant to add it here. An alternative is to add +rdma but that's also enabled by armv8.1-a. (I've a patch at https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01973.html which gets rid for +rdma as part of an armv8.1-a command line clean up.) >> +# Return 1 if the target supports executing the ARMv8.1 Adv.SIMD extension, 0 >> +# otherwise. The test is valid for AArch64. >> + >> +proc check_effective_target_arm_v8_1a_neon_hw { } { >> + if { ![check_effective_target_arm_v8_1a_neon_ok] } { >> + return 0; >> + } >> + return [check_runtime_nocache arm_v8_1a_neon_hw_available { >> + int >> + main (void) >> + { >> + long long a = 0, b = 1; >> + long long result = 0; >> + >> + asm ("sqrdmlah %s0,%s1,%s2" >> + : "=w"(result) >> + : "w"(a), "w"(b) >> + : /* No clobbers. */); > > Hm, those types look wrong, I guess this works but it is an unusual way > to write it. I presume this is to avoid including arm_neon.h each time, but > you could just directly use the internal type names for the arm_neon types. > That is to say __Int32x4_t (or whichever mode you intend to use). > I'll rework the patch to use the internal types names. Matthew
From b12969882298cb79737e882c48398c58a45161b9 Mon Sep 17 00:00:00 2001 From: Matthew Wahab <matthew.wahab@arm.com> Date: Mon, 26 Oct 2015 14:58:36 +0000 Subject: [PATCH 5/7] [Testsuite] Add dejagnu options for armv8.1 neon Change-Id: Ib58b8c4930ad3971af3ea682eda043e14cd2e8b3 --- gcc/testsuite/lib/target-supports.exp | 56 ++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 4d5b0a3d..0fb679d 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -2700,6 +2700,16 @@ proc add_options_for_arm_v8_neon { flags } { return "$flags $et_arm_v8_neon_flags -march=armv8-a" } +# Add the options needed for ARMv8.1 Adv.SIMD. + +proc add_options_for_arm_v8_1a_neon { flags } { + if { [istarget aarch64*-*-*] } { + return "$flags -march=armv8.1-a" + } else { + return "$flags" + } +} + proc add_options_for_arm_crc { flags } { if { ! [check_effective_target_arm_crc_ok] } { return "$flags" @@ -2984,7 +2994,8 @@ foreach { armfunc armflag armdef } { v4 "-march=armv4 -marm" __ARM_ARCH_4__ v7r "-march=armv7-r" __ARM_ARCH_7R__ v7m "-march=armv7-m -mthumb" __ARM_ARCH_7M__ v7em "-march=armv7e-m -mthumb" __ARM_ARCH_7EM__ - v8a "-march=armv8-a" __ARM_ARCH_8A__ } { + v8a "-march=armv8-a" __ARM_ARCH_8A__ + v8_1a "-march=armv8.1a" __ARM_ARCH_8A__ } { eval [string map [list FUNC $armfunc FLAG $armflag DEF $armdef ] { proc check_effective_target_arm_arch_FUNC_ok { } { if { [ string match "*-marm*" "FLAG" ] && @@ -3141,6 +3152,25 @@ proc check_effective_target_arm_neonv2_hw { } { } [add_options_for_arm_neonv2 ""]] } +# Return 1 if the target supports the ARMv8.1 Adv.SIMD extension, 0 +# otherwise. The test is valid for AArch64. + +proc check_effective_target_arm_v8_1a_neon_ok_nocache { } { + if { ![istarget aarch64*-*-*] } { + return 0 + } + return [check_no_compiler_messages_nocache arm_v8_1a_neon_ok assembly { + #if !defined (__ARM_FEATURE_QRDMX) + #error "__ARM_FEATURE_QRDMX not defined" + #endif + } [add_options_for_arm_v8_1a_neon ""]] +} + +proc check_effective_target_arm_v8_1a_neon_ok { } { + return [check_cached_effective_target arm_v8_1a_neon_ok \ + check_effective_target_arm_v8_1a_neon_ok_nocache] +} + # Return 1 if the target supports executing ARMv8 NEON instructions, 0 # otherwise. @@ -3159,6 +3189,30 @@ proc check_effective_target_arm_v8_neon_hw { } { } [add_options_for_arm_v8_neon ""]] } +# Return 1 if the target supports executing the ARMv8.1 Adv.SIMD extension, 0 +# otherwise. The test is valid for AArch64. + +proc check_effective_target_arm_v8_1a_neon_hw { } { + if { ![check_effective_target_arm_v8_1a_neon_ok] } { + return 0; + } + return [check_runtime_nocache arm_v8_1a_neon_hw_available { + int + main (void) + { + long long a = 0, b = 1; + long long result = 0; + + asm ("sqrdmlah %s0,%s1,%s2" + : "=w"(result) + : "w"(a), "w"(b) + : /* No clobbers. */); + + return result; + } + } [add_options_for_arm_v8_1a_neon ""]] +} + # Return 1 if this is a ARM target with NEON enabled. proc check_effective_target_arm_neon { } { -- 2.1.4