Re: [PING][Patch 3/3][Arm] Add support for IEEE-conformant versions of scalar fmin* and fmax*

[Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>]

Hi David,

On 22/12/15 17:07, David Sherwood wrote:
> Hi Kyrill,
>
> Thanks for the reply, I think this latest patch addresses your
> comments. I have added a test for the scalar forms of fmax/fmin.
>
> Is this ok now?
>
> Tested:
>
> arm-none-eabi: no regressions

I've also bootstrapped this patch on arm-none-linux-gnueabihf

> Cheers,
> Dave.
>
> ChangeLog:
>
> 2015-12-22  David Sherwood  <david.sherwood@arm.com>
>
>      gcc/
>          * config/arm/iterators.md (VMAXMINFNM): New int iterator.
>          (fmaxmin): New int attribute.
>          (fmaxmin_op): Likewise.
>          * config/arm/unspecs.md (UNSPEC_VMAXNM): New unspec.
>          (UNSPEC_VMINNM): Likewise.
>          * config/arm/neon.md (fmaxmin): New pattern.
>          * config/arm/vfp.md (fmaxmin): Likewise.
>      gcc/testsuite
>          * gcc.target/arm/fmaxmin.x: New file used by tests below.
>          * gcc.target/arm/fmaxmin.c: New test.
>          * gcc.target/arm/vect-fmaxmin.c: Likewise.

Yes, this is ok now.
Thanks,
Kyrill

> ________________________________________
> From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
> Sent: Wednesday, December 16, 2015 9:31 AM
> To: David Sherwood; GCC Patches
> Subject: Re: [PING][Patch 3/3][Arm] Add support for IEEE-conformant versions of scalar fmin* and fmax*
>
> Hi David,
>
> On 16/12/15 08:53, David Sherwood wrote:
> > Hi,
> >
> > Here is the last patch of the fmin/fmax change, which adds the optabs
> > to the arm backend.
> >
> > Tested:
> >
> > arm-none-eabi: no regressions
> >
> > Good to go?
> > David Sherwood.
> >
> > ChangeLog:
> >
> > 2015-12-08  David Sherwood  <david.sherwood@arm.com>
> >
> >       gcc/
> >           * config/arm/iterators.md: New iterators.
> >           * config/arm/unspecs.md: New unspecs.
> >           * config/arm/neon.md: New pattern.
> >           * config/arm/vfp.md: Likewise.
> Please list the new entities you add in parentheses.
> For example:
>       * config/arm/iterators.md (VMAXMINFNM): New int iterator.
>       (fmaxmin): New int attribute.
>       (fmaxmin): Likewise.
>
> Same for the other files. That way one can grep through the ChangeLogs to
> find when any particular pattern/iterator/whatever was modified.
>
> +;; Auto-vectorized forms for the IEEE-754 fmax()/fmin() functions
> +(define_insn "<fmaxmin><mode>3"
> +  [(set (match_operand:VCVTF 0 "s_register_operand" "=w")
> +       (unspec:VCVTF [(match_operand:VCVTF 1 "s_register_operand" "w")
> +                      (match_operand:VCVTF 2 "s_register_operand" "w")]
> +                      VMAXMINFNM))]
> +  "TARGET_NEON && TARGET_FPU_ARMV8"
> +  "<fmaxmin_op>.<V_s_elem>\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
> +  [(set_attr "type" "neon_fp_minmax_s<q>")]
> +)
>
> I would just say "Vector forms" rather than "Auto-vectorized".
> In principle we can get vector types through other means besides
> auto-vectorisation.
>
> +;; Scalar forms for the IEEE-754 fmax()/fmin() functions
> +(define_insn "<fmaxmin><mode>3"
> +  [(set (match_operand:SDF 0 "s_register_operand" "=<F_constraint>")
> +       (unspec:SDF [(match_operand:SDF 1 "s_register_operand" "<F_constraint>")
> +                    (match_operand:SDF 2 "s_register_operand" "<F_constraint>")]
> +                    VMAXMINFNM))]
> +  "TARGET_HARD_FLOAT && TARGET_VFP5 <vfp_double_cond>"
> +  "<fmaxmin_op>.<V_if_elem>\\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
> +  [(set_attr "type" "f_minmax<vfp_type>")
> +   (set_attr "conds" "unconditional")]
> +)
> +
>
> I notice your new test doesn't test the SF variant of this.
> Could you please add something to test it?
> Looks good to me otherwise.
>
> Thanks,
> Kyrill