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

2015-11-17 Thread James Greenhalgh
On Tue, Oct 27, 2015 at 11:33:21AM +, 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
  



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

2015-10-27 Thread James Greenhalgh
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  
> 
>   * config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Add
>   ARM_FEATURE_QRDMX.
>