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

[James Greenhalgh <james.greenhalgh@arm.com>]

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