This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [AArch64] Fix simd intrinsics bug on float vminnm/vmaxnm
- From: James Greenhalgh <james dot greenhalgh at arm dot com>
- To: Jiong Wang <jiong dot wang at foss dot arm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, <nd at arm dot com>, <christophe dot lyon at linaro dot org>, <kyrtka01 at arm dot com>
- Date: Wed, 6 Jul 2016 16:29:13 +0100
- Subject: Re: [AArch64] Fix simd intrinsics bug on float vminnm/vmaxnm
- Authentication-results: sourceware.org; auth=none
- Nodisclaimer: True
- References: <f8a47ab3-ee52-4964-79b8-199e0562152e@foss.arm.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On Wed, Jul 06, 2016 at 02:11:51PM +0100, Jiong Wang wrote:
> The current vmaxnm/vminnm float intrinsics are implemented using
> __builtin_aarch64_smax/min<mode> which are mapping to backend patterns
> using smin/smax rtl operators. However as documented in rtl.def
>
> "Further, if both operands are zeros, or if either operand is NaN, then
> it is unspecified which of the two operands is returned as the result."
>
> There is no guarantee that a number will always be returned through
> smin/smax operator, and further tests show gcc will optimize something
> like smin (1.0f, Nan) to Nan, so current the vmaxnm and vminnm intrinsics
> will evetually fail the new added testcases included in this patch.
>
> This patch:
>
> * Migrate vminnm/vmaxnm float intrinsics to "<fmaxmin><mode>3" pattern
> which guarantee fminnm/fmaxnm sematics.
>
> * Add new testcases for vminnm and vmaxnm intrinsics which were missing
> previously. They are marked as XFAIL on arm*-*-* as ARM hasn't
> implemented these intrinsics.
>
> OK for trunk?
The AArch64 parts are OK. I can't remember whether the ARM port prefers
to have missing intrinsics XFAIL'd or if there is another way to disable
the tests that are not supported there. Kyrill/Christophe would you mind
commenting on whether this patch is correct for the intrinsics testsuite?
Thanks,
James
> 2016-07-06 Jiong Wang <jiong.wang@arm.com>
>
> gcc/
> * config/aarch64/aarch64-simd-builtins.def (smax): Remove float variants.
> (smin): Likewise.
> (fmax): New entry.
> (fmin): Likewise.
> * config/aarch64/arm_neon.h (vmaxnm_f32): Use __builtin_aarch64_fmaxv2sf.
> (vmaxnmq_f32): Likewise.
> (vmaxnmq_f64): Likewise.
> (vminnm_f32): Likewise.
> (vminnmq_f32): Likewise.
> (vminnmq_f64): Likewise.
>
> gcc/testsuite/
> * gcc.target/aarch64/advsimd-intrinsics/binary_op_no64.inc: Support HAS_INTEGER_VARIANT.
> * gcc.target/aarch64/advsimd-intrinsics/vrhadd.c: Define HAS_INTEGER_VARIANT.
> * gcc.target/aarch64/advsimd-intrinsics/vhadd.c: Define HAS_INTEGER_VARIANT.
> * gcc.target/aarch64/advsimd-intrinsics/vhsub.c: Define HAS_INTEGER_VARIANT.
> * gcc.target/aarch64/advsimd-intrinsics/vmax.c: Define HAS_INTEGER_VARIANT.
> * gcc.target/aarch64/advsimd-intrinsics/vmin.c: Define HAS_INTEGER_VARIANT.
> * gcc.target/aarch64/advsimd-intrinsics/vhadd.c: Define HAS_INTEGER_VARIANT.
> * gcc.target/aarch64/advsimd-intrinsics/vmaxnm.c: New.
> * gcc.target/aarch64/advsimd-intrinsics/vminnm.c: New.