This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH target/89071] Fix false dependence of scalar operations vrcp/vsqrt/vrsqrt/vrndscale


On Fri, Oct 25, 2019 at 2:39 AM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Wed, Oct 23, 2019 at 7:48 AM Hongtao Liu <crazylht@gmail.com> wrote:
> >
> > Update patch:
> > Add m constraint to define_insn (sse_1_round<ssescalaemodesuffix,
> > *sse_1_round<ssescalaemodesuffix)  to fix some unrecognized insn error
> > when under sse4 but not avx512f.
>
> It looks to me that the original insn is incompletely defined. It
> should use nonimmediate_operand, "m" constraint and <iptr> pointer
> size modifier. Something like:
>
> (define_insn "sse4_1_round<ssescalarmodesuffix>"
>   [(set (match_operand:VF_128 0 "register_operand" "=Yr,*x,x,v")
>     (vec_merge:VF_128
>       (unspec:VF_128
>         [(match_operand:VF_128 2 "nonimmediate_operand" "Yrm,*xm,xm,vm")
>          (match_operand:SI 3 "const_0_to_15_operand" "n,n,n,n")]
>         UNSPEC_ROUND)
>       (match_operand:VF_128 1 "register_operand" "0,0,x,v")
>       (const_int 1)))]
>   "TARGET_SSE4_1"
>   "@
>    round<ssescalarmodesuffix>\t{%3, %2, %0|%0, %<iptr>2, %3}
>    round<ssescalarmodesuffix>\t{%3, %2, %0|%0, %<iptr>2, %3}
>    vround<ssescalarmodesuffix>\t{%3, %2, %1, %0|%0, %1, %<iptr>2, %3}
>    vrndscale<ssescalarmodesuffix>\t{%3, %2, %1, %0|%0, %1, %<iptr>2, %3}"
>
> >
> > Changelog:
> > gcc/
> >         * config/i386/sse.md:  (sse4_1_round<ssescalarmodesuffix>):
> > Change constraint x to xm
> >         since vround support memory operand.
> >         * (*sse4_1_round<ssescalarmodesuffix>): Ditto.
> >
> > Bootstrap and regression test ok.
> >
> > On Wed, Oct 23, 2019 at 9:56 AM Hongtao Liu <crazylht@gmail.com> wrote:
> > >
> > > Hi uros:
> > >   This patch fixes false dependence of scalar operations
> > > vrcp/vsqrt/vrsqrt/vrndscale.
> > >   Bootstrap ok, regression test on i386/x86 ok.
> > >
> > >   It does something like this:
> > > -----
> > > For scalar instructions with both xmm operands:
> > >
> > > op %xmmN,%xmmQ,%xmmQ ----> op %xmmN, %xmmN, %xmmQ
> > >
> > > for scalar instructions with one mem  or gpr operand:
> > >
> > > op mem/gpr, %xmmQ, %xmmQ
> > >
> > > --->  using pass rpad ---->
> > >
> > > xorps %xmmN, %xmmN, %xxN
> > > op mem/gpr, %xmmN, %xmmQ
> > >
> > > Performance influence of SPEC2017 fprate which is tested on SKX
> > >
> > > 503.bwaves_r -0.03%
> > > 507.cactuBSSN_r -0.22%
> > > 508.namd_r -0.02%
> > > 510.parest_r 0.37%
> > > 511.povray_r 0.74%
> > > 519.lbm_r 0.24%
> > > 521.wrf_r 2.35%
> > > 526.blender_r 0.71%
> > > 527.cam4_r 0.65%
> > > 538.imagick_r 0.95%
> > > 544.nab_r -0.37
> > > 549.fotonik3d_r 0.24%
> > > 554.roms_r 0.90%
> > > fprate geomean 0.50%
> > > -----
> > >
> > > Changelog
> > > gcc/
> > >         * config/i386/i386.md (*rcpsf2_sse): Add
> > >         avx_partial_xmm_update, prefer m constraint for TARGET_AVX.
> > >         (*rsqrtsf2_sse): Ditto.
> > >         (*sqrt<mode>2_sse): Ditto.
> > >         (sse4_1_round<mode>2): separate constraint vm, add
> > >         avx_partail_xmm_update, prefer m constraint for TARGET_AVX.
> > >         * config/i386/sse.md (*sse_vmrcpv4sf2"): New define_insn used
> > >         by pass rpad.
> > >         (*<sse>_vmsqrt<mode>2<mask_scalar_name><round_scalar_name>*):
> > >         Ditto.
> > >         (*sse_vmrsqrtv4sf2): Ditto.
> > >         (*avx512f_rndscale<mode><round_saeonly_name>): Ditto.
> > >         (*sse4_1_round<ssescalarmodesuffix>): Ditto.
> > >
> > > gcc/testsuite
> > >         * gcc.target/i386/pr87007-4.c: New test.
> > >         * gcc.target/i386/pr87007-5.c: Ditto.
> > >
> > >
> > > --
> > > BR,
> > > Hongtao
>
>     (set (attr "preferred_for_speed")
>       (cond [(eq_attr "alternative" "1")
>            (symbol_ref "TARGET_AVX || !TARGET_SSE_PARTIAL_REG_DEPENDENCY")
>         (eq_attr "alternative" "2")
> -          (symbol_ref "!TARGET_SSE_PARTIAL_REG_DEPENDENCY")
> +          (symbol_ref "TARGET_AVX || !TARGET_SSE_PARTIAL_REG_DEPENDENCY")
>         ]
>         (symbol_ref "true")))])
>
> This can be written as:
>
>     (set (attr "preferred_for_speed")
>       (cond [(match_test "TARGET_AVX")
>            (symbol_ref "true")
>         (eq_attr "alternative" "1,2")
>           (symbol_ref "!TARGET_SSE_PARTIAL_REG_DEPENDENCY")
>         ]
>         (symbol_ref "true")))])
>
> Uros.

Yes, after these fixed, i'll upstream to trunk, ok?
-- 
BR,
Hongtao


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]