Re: [PATCH] x86: Check TARGET_AVX512VL when enabling FMA

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Jul 9, 2020 at 3:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Jul 9, 2020 at 6:35 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Thu, Jul 9, 2020 at 5:04 AM Kirill Yukhin <kirill.yukhin@gmail.com> wrote:
> > >
> > > On 07 июл 09:06, H.J. Lu wrote:
> > > > On Tue, Jul 7, 2020 at 8:56 AM Kirill Yukhin <kirill.yukhin@gmail.com> wrote:
> > > > >
> > > > > Hello HJ,
> > > > >
> > > > > On 28 июн 07:19, H.J. Lu via Gcc-patches wrote:
> > > > > > Enable FMA in rsqrt<mode>2 expander and fold rsqrtv16sf2 expander into
> > > > > > rsqrt<mode>2 expander which expands to UNSPEC_RSQRT28 for TARGET_AVX512ER.
> > > > > > Although it doesn't show performance change in our workloads, FMA can
> > > > > > improve other workloads.
> > > > > >
> > > > > > gcc/
> > > > > >
> > > > > >       PR target/88713
> > > > > >       * config/i386/i386-expand.c (ix86_emit_swsqrtsf): Enable FMA.
> > > > > >       * config/i386/sse.md (VF_AVX512VL_VF1_128_256): New.
> > > > > >       (rsqrt<mode>2): Replace VF1_128_256 with VF_AVX512VL_VF1_128_256.
> > > > > >       (rsqrtv16sf2): Removed.
> > > > > >
> > > > > > gcc/testsuite/
> > > > > >
> > > > > >       PR target/88713
> > > > > >       * gcc.target/i386/pr88713-1.c: New test.
> > > > > >       * gcc.target/i386/pr88713-2.c: Likewise.
> > > > >
> > > > > So, you've introduced new rsqrt expanders for DF vectors and relaxed
> > > > > condition for V16SF. What I didn't get is why did you change unspec
> > > > > type from RSQRT to RSQRT28 for V16SF expander?
> > > > >
> > > >
> > > > UNSPEC in define_expand is meaningless when the pattern is fully
> > > > expanded by ix86_emit_swsqrtsf.  I believe that UNSPEC in rsqrt<mode>2
> > > > expander can be removed.
> > >
> > > Agree.
> >
> > I will leave UNSPEC alone here.
> >
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/pr88713-1.c
> > > @@ -0,0 +1,13 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2 -Ofast -mno-avx512f -mfma" } */
> > >
> > > I gues -O2 is useless here (and in -2.c test).
> >
> > Fixed.
> >
> > > Othwerwise LGTM.
> > >
> >
> > This is the patch I am checking in.
> >
>
> This patch is needed for
>
> FAIL: gcc.target/i386/avx512er-vrsqrt28ps-3.c (internal compiler error)
> FAIL: gcc.target/i386/avx512er-vrsqrt28ps-3.c (test for excess errors)
> FAIL: gcc.target/i386/avx512er-vrsqrt28ps-4.c (internal compiler error)
> FAIL: gcc.target/i386/avx512er-vrsqrt28ps-4.c (test for excess errors)
> FAIL: gcc.target/i386/avx512er-vrsqrt28ps-5.c (internal compiler error)
> FAIL: gcc.target/i386/avx512er-vrsqrt28ps-5.c (test for excess errors)
> FAIL: gcc.target/i386/avx512er-vrsqrt28ps-6.c (internal compiler error)
> FAIL: gcc.target/i386/avx512er-vrsqrt28ps-6.c (test for excess errors)
>

This fixed:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96144

-- 
H.J.