diff mbox

[AArch64,4/7] Add ACLE feature macro for ARMv8.1,Adv.SIMD instructions.

Message ID 562A2678.6060203@foss.arm.com
State New
Headers show

Commit Message

Matthew Wahab Oct. 23, 2015, 12:22 p.m. UTC
The ARMv8.1 architecture extension adds two Adv.SIMD instructions,
sqrdmlah and sqrdmlsh. This patch adds the feature macro
__ARM_FEATURE_QRDMX to indicate the presence of these instructions,
generating it when the feature is available, as it is when
-march=armv8.1-a is selected.

Tested the series for aarch64-none-linux-gnu with native bootstrap and
make check on an ARMv8 architecture. Also tested aarch64-none-elf with
cross-compiled check-gcc on an ARMv8.1 emulator.

Ok for trunk?
Matthew

gcc/
2015-10-23  Matthew Wahab  <matthew.wahab@arm.com>

	* config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Add
	ARM_FEATURE_QRDMX.

Comments

James Greenhalgh Oct. 27, 2015, 11:33 a.m. UTC | #1
On Fri, Oct 23, 2015 at 01:22:16PM +0100, Matthew Wahab wrote:
> The ARMv8.1 architecture extension adds two Adv.SIMD instructions,
> sqrdmlah and sqrdmlsh. This patch adds the feature macro
> __ARM_FEATURE_QRDMX to indicate the presence of these instructions,
> generating it when the feature is available, as it is when
> -march=armv8.1-a is selected.
> 
> Tested the series for aarch64-none-linux-gnu with native bootstrap and
> make check on an ARMv8 architecture. Also tested aarch64-none-elf with
> cross-compiled check-gcc on an ARMv8.1 emulator.
> 
> Ok for trunk?
> Matthew

I don't see this macro documented in the versions of ACLE available from
the ARM documentation sites, and googling doesn't show anything other
than your patches. You don't explicitly mention anywhere in cover text for
this series where these new features are (or will be?) documented.

Could you please write a more complete description of where these new
macros and intrinsics come from and what they are intended to do? I would
not like to accept them without some confidence that these names have
been finalized, and I am nervous about having the best description of the
behaviour of them be the GCC source code.

Richard, Marcus?

Thanks,
James

> 
> gcc/
> 2015-10-23  Matthew Wahab  <matthew.wahab@arm.com>
> 
> 	* config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Add
> 	ARM_FEATURE_QRDMX.
>
James Greenhalgh Nov. 17, 2015, 1:21 p.m. UTC | #2
On Tue, Oct 27, 2015 at 11:33:21AM +0000, James Greenhalgh wrote:
> On Fri, Oct 23, 2015 at 01:22:16PM +0100, Matthew Wahab wrote:
> > The ARMv8.1 architecture extension adds two Adv.SIMD instructions,
> > sqrdmlah and sqrdmlsh. This patch adds the feature macro
> > __ARM_FEATURE_QRDMX to indicate the presence of these instructions,
> > generating it when the feature is available, as it is when
> > -march=armv8.1-a is selected.
> > 
> > Tested the series for aarch64-none-linux-gnu with native bootstrap and
> > make check on an ARMv8 architecture. Also tested aarch64-none-elf with
> > cross-compiled check-gcc on an ARMv8.1 emulator.
> > 
> > Ok for trunk?
> > Matthew
> 
> I don't see this macro documented in the versions of ACLE available from
> the ARM documentation sites, and googling doesn't show anything other
> than your patches. You don't explicitly mention anywhere in cover text for
> this series where these new features are (or will be?) documented.
> 
> Could you please write a more complete description of where these new
> macros and intrinsics come from and what they are intended to do? I would
> not like to accept them without some confidence that these names have
> been finalized, and I am nervous about having the best description of the
> behaviour of them be the GCC source code.

This macro and the intrinsics included in this patch set are as they will
appear in a future release of ACLE.

__ARM_FEATURE_QRDMX will be defined to 1 if the SQRDMLAH and SQRDMLSH
instructions are available.

The intrinsics added take this form for the non-lane intrinsics:

  int16x4_t vqrdmlah_s16 (int16x4_t a, int16x4_t b, int16x4_t c)
    a -> Vd.4H, b -> Vn.4H, c-> Vm.4h
    VQRDMLAH Vd.4H,Vn.4H,Vm.4H
    Vd.4H -> result

And this form for the lane intrinsics:

  int16x4_t vqrdmlah_lane_s16 (int16x4_t a, int16x4_t b,
			       int16x4_t v, const int lane)
    a -> Vd.4H, b -> Vn.4H, v -> Vm.4h, 0 <= lane <= 3
    VQRDMLAH Vd.4H,Vn.4H,Vm.H[lane]
    Vd.4H -> result

Using the same syntax as is in the ARM Neon Intrinsics Reference [1].

These intrinsics are only available when __ARM_FEATURE_QRDMX is defined.

With all that said...

This patch is OK, but please fix the ChangeLog entry:

> > 	* config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Add
> > 	ARM_FEATURE_QRDMX.

  s/ARM_FEATURE_QRDMX/__ARM_FEATURE_QRDMX/

Thanks,
James

---
[1]: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0073a/IHI0073A_arm_neon_intrinsics_ref.pdf
diff mbox

Patch

From 3af8c483a2def95abec264ca8591547d6c0e0b3e Mon Sep 17 00:00:00 2001
From: Matthew Wahab <matthew.wahab@arm.com>
Date: Thu, 27 Aug 2015 13:31:49 +0100
Subject: [PATCH 4/7] Add ACLE QRDMX feature macro.

Change-Id: I91af172637603ea89fc93a8e715973d7d304a92f
---
 gcc/config/aarch64/aarch64-c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/config/aarch64/aarch64-c.c b/gcc/config/aarch64/aarch64-c.c
index 303025f..ad95c78 100644
--- a/gcc/config/aarch64/aarch64-c.c
+++ b/gcc/config/aarch64/aarch64-c.c
@@ -126,6 +126,7 @@  aarch64_update_cpp_builtins (cpp_reader *pfile)
   aarch64_def_or_undef (TARGET_ILP32, "__ILP32__", pfile);
 
   aarch64_def_or_undef (TARGET_CRYPTO, "__ARM_FEATURE_CRYPTO", pfile);
+  aarch64_def_or_undef (TARGET_SIMD_RDMA, "__ARM_FEATURE_QRDMX", pfile);
 }
 
 /* Implement TARGET_CPU_CPP_BUILTINS.  */
-- 
2.1.4