Re: [PATCH, GCC/testsuite/ARM] Consistently check for neon in vect effective targets

[Thomas Preudhomme <thomas dot preudhomme at foss dot arm dot com>]

On 19/06/17 15:31, Christophe Lyon wrote:
> On 19 June 2017 at 16:11, Thomas Preudhomme
> <thomas.preudhomme@foss.arm.com> wrote:
> > On 19/06/17 10:16, Thomas Preudhomme wrote:
> > > On 19/06/17 08:41, Christophe Lyon wrote:
> > > > Hi Thomas,
> > > >
> > > >
> > > > On 15 June 2017 at 18:18, Thomas Preudhomme
> > > > <thomas.preudhomme@foss.arm.com> wrote:
> > > > > Hi,
> > > > >
> > > > > Conditions checked for ARM targets in vector-related effective targets
> > > > > are inconsistent:
> > > > >
> > > > > * sometimes arm*-*-* is checked
> > > > > * sometimes Neon is checked
> > > > > * sometimes arm_neon_ok and sometimes arm_neon is used for neon check
> > > > > * sometimes check_effective_target_* is used, sometimes
> > > > > is-effective-target
> > > > >
> > > > > This patch consolidate all of these check into using is-effective-target
> > > > > arm_neon and when little endian was checked, the check is kept.
> > > > >
> > > > > ChangeLog entry is as follows:
> > > > >
> > > > > *** gcc/testsuite/ChangeLog ***
> > > > >
> > > > > 2017-06-06  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > > > >
> > > > >         * lib/target-supports.exp (check_effective_target_vect_int):
> > > > > Replace
> > > > >         current ARM check by ARM NEON's availability check.
> > > > >         (check_effective_target_vect_intfloat_cvt): Likewise.
> > > > >         (check_effective_target_vect_uintfloat_cvt): Likewise.
> > > > >         (check_effective_target_vect_floatint_cvt): Likewise.
> > > > >         (check_effective_target_vect_floatuint_cvt): Likewise.
> > > > >         (check_effective_target_vect_shift): Likewise.
> > > > >         (check_effective_target_whole_vector_shift): Likewise.
> > > > >         (check_effective_target_vect_bswap): Likewise.
> > > > >         (check_effective_target_vect_shift_char): Likewise.
> > > > >         (check_effective_target_vect_long): Likewise.
> > > > >         (check_effective_target_vect_float): Likewise.
> > > > >         (check_effective_target_vect_perm): Likewise.
> > > > >         (check_effective_target_vect_perm_byte): Likewise.
> > > > >         (check_effective_target_vect_perm_short): Likewise.
> > > > >         (check_effective_target_vect_widen_sum_hi_to_si_pattern):
> > > > > Likewise.
> > > > >         (check_effective_target_vect_widen_sum_qi_to_hi): Likewise.
> > > > >         (check_effective_target_vect_widen_mult_qi_to_hi): Likewise.
> > > > >         (check_effective_target_vect_widen_mult_hi_to_si): Likewise.
> > > > >         (check_effective_target_vect_widen_mult_qi_to_hi_pattern):
> > > > > Likewise.
> > > > >         (check_effective_target_vect_widen_mult_hi_to_si_pattern):
> > > > > Likewise.
> > > > >         (check_effective_target_vect_widen_shift): Likewise.
> > > > >         (check_effective_target_vect_extract_even_odd): Likewise.
> > > > >         (check_effective_target_vect_interleave): Likewise.
> > > > >         (check_effective_target_vect_multiple_sizes): Likewise.
> > > > >         (check_effective_target_vect64): Likewise.
> > > > >         (check_effective_target_vect_max_reduc): Likewise.
> > > > >
> > > > > Testing: Testsuite shows no regression when targeting ARMv7-A with
> > > > > -mfpu=neon-fpv4 and -mfloat-abi=hard or when targeting Cortex-M3 with
> > > > > default FPU and float ABI (soft). Testing was done with both
> > > > > compare_tests
> > > > > and the updated dg-cmp-results proposed in
> > > > > https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01030.html
> > > > >
> > > > > Is this ok for trunk?
> > > >
> > > > I applied your patch on top of r249233, and noticed quite a few changes:
> > > >
> > > > http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/249233-consistent_neon_check.patch/report-build-info.html
> > > >
> > > >
> > > > Note that "Big-Regression" cases are caused by the fact that there a
> > > > are PASS->XPASS and XFAILs disappear with your patch, and many
> > > > (3000-4000) PASS disappear.
> > > > In that intended?
> > >
> > >
> > > It certainly is not. I'd like to investigate this but the link to results
> > > for
> > > rev 249233 is broken. Could you provide me with the results you have for
> > > that so
> > > that I can compare manually?
> >
> >
> > Actually yes it is, at least for the configurations with default (which
> > still uses -mfpu=vfp in r249233) or VFP (whatever version) FPU. I've checked
> > all the ->NA and ->UNSUPPORTED for the arm-none-linux-gnueabi configuration
> > and none of them has a dg directive to select the neon unit (such as
> > dg-additional-options <something that would add -mfpu on the command line>).
> > I've also looked at arm-none-linux-gnueabihf configuration with neon FPU and
> > there is no regression there.
> >
> > I therefore think this is all normal and expected. Note that under current
> > trunk this should be different because neon-fp16 would be selected instead
> > of vfp for default FPU with Cortex-A9.
>
> OK, thanks for checking. So the version you sent on June 15th is OK?

Yes.

> I can start a validation against current trunk, after Richard's series,
> it probably makes sense, doesn't it?

I think it'll give cleaner results yes. Note that the one with an explicit 
-mfpu=vfp* without neon will still have a lot of changes but at least the one 
with default FPU should be more readable.
> Thanks,
>
> Christophe
>
> > Best regards,
> >
> > Thomas