On 7 July 2016 at 11:16, Jiong Wang <jiong.w...@foss.arm.com> wrote: > On 06/07/16 16:55, Christophe Lyon wrote: >> >> On 6 July 2016 at 17:44, Kyrill Tkachov <kyrylo.tkac...@foss.arm.com> >> wrote: >>> >>> Hi all, >>> >>> >>> On 06/07/16 16:29, James Greenhalgh wrote: >>>> >>>> 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.w...@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. >>> >>> >>> These intrinsics are supposed to be available for arm as well *except* >>> for >>> vminnmq_f64, vmaxnmq_f64. >>> >> I missed that point. >> So, I agree with Kyrill: >> - skip the ones that aren't supposed to be available for arm >> - xfail the ones that aren't implemented yet. >> >> Christophe > > I was using dg-xfail-if, (the description is still using "marked as > XFAIL"...), > but later found it's actually broken under advsimd-intrinsics, UNRESOLVEDs > are > given at the same time instead of clean XFAIL, I suspect those dg-do-what > overriding broken dejangu internal variable, Christophe, do you mind have a > look? > I've made a quick attempt (replacing an existing dg-skip-if with dg-xfail-if in vcvt_high-1.c) and I do see XFAIL and UNRESOLVED. But this seems normal in this case, because: - when using dg-skip-if, the test was not compiled (skipped) - when using dg-xfail-if, the test is actually compiled, leading to compilation errors which are reported as UNRESOLVED
> Meanwhile, as the new vminnm and vmaxnm testcases are using the existed test > infrastructure of vmin/vmax infrastructure which is based on > binary-op-no64.inc > where only float32 defined. If we enable float64x2, there will be two > issues: > > * The naming of binary-iop-no64.inc needs to be updated, but it's used by > several other files, so not sure it's the correct approach. > > * You will want to xfail only float64x2 testing on ARM inside vmin.c and > vmax.c, I don't know how to do that. > > For the vminnm and vmaxnm testing, I really want to go ahead to implement > them > for ARM so we won't be bothered by xfail, there is backend pattern already > which > is fmin/fax, however they are standard name without "neon_" prefix, while > unlike > AArch64, ARM neon builtins infrastructure force the prefix to be "neon_". > The > macro expand infrastructure needs to be updated. > > For this patch, I am going to change dg-skip-if to dg-xfail-if, but please > be > prepare with those UNRESOLVED failures which will go away once > advsimd-intrinsics.exp fixed. Meanwhile I will create seperate test file > for > float64x2, and dg-skip them on ARM. > >
