Hi, 1 is replace 8 spaces with tab suggested by ./check_GNU_style.sh, should I still fix it back? 2,3,4 Done
CSE is working, spec 2k6 on skylake-avx512 has same score.
PR target/73350,80862
gcc/
* config/i386/subst.md (round): Fix round pattern.
* config/i386/i386.c (ix86_erase_embedded_rounding):
Fix erasing rounding for the fixed pattern.
gcc/testsuite
* gcc.target/i386/pr73350.c: New test.
Ok for trunk?
Thanks,
Julia
> -----Original Message-----
> From: Kirill Yukhin [mailto:kirill.yuk...@gmail.com]
> Sent: Wednesday, May 31, 2017 12:34 PM
> To: Koval, Julia <julia.ko...@intel.com>
> Cc: GCC Patches <gcc-patches@gcc.gnu.org>; Uros Bizjak
> <ubiz...@gmail.com>; Peryt, Sebastian <sebastian.pe...@intel.com>;
> ja...@redhat.com; richard.guent...@gmail.com
> Subject: Re: [PATCH][x86][PR73350][PR80862]
>
> On 31 May 11:38, Kirill Yukhin wrote:
> > Hello Julia,
> > On 26 May 09:13, Koval, Julia wrote:
> > > Hi,
> > > This patch fixes these PR's. Ok for trunk?
> > >
> > > gcc/
> > > * config/i386/subst.md (round): Fix round pattern.
> > > * config/i386/i386.c (ix86_erase_embedded_rounding):
> > > Fix erasing rounding for the fixed pattern.
> > >
> > > Thanks,
> > > Julia
> >
> > Let me copy-paste parts of your patch here.
> > diff --git a/gcc/config/i386/subst.md b/gcc/config/i386/subst.md
> > index 0bc22fd..2e632b9 100644
> > --- a/gcc/config/i386/subst.md
> > +++ b/gcc/config/i386/subst.md
> > @@ -137,12 +137,12 @@
> >
> > (define_subst "round"
> > [(set (match_operand:SUBST_A 0)
> > - (match_operand:SUBST_A 1))]
> > + (match_operand:SUBST_A 1))]
> > "TARGET_AVX512F"
> > - [(parallel[
> > - (set (match_dup 0)
> > - (match_dup 1))
> > - (unspec [(match_operand:SI 2 "const_4_or_8_to_11_operand")]
> UNSPEC_EMBEDDED_ROUNDING)])])
> > + [(set (match_dup 0)
> > + (unspec:SUBST_A [(match_dup 1)
> > + (match_operand:SI 2 "const_4_or_8_to_11_operand")]
> > + UNSPEC_EMBEDDED_ROUNDING))])
> >
> > (define_subst_attr "round_saeonly_name" "round_saeonly" "" "_round")
> > (define_subst_attr "round_saeonly_mask_operand2" "mask" "%r2" "%r4")
> > --
> > 2.5.5
> >
> > So, you propose to put RC as third argument to the set expression.
> > I am not sure that we might use (set ...) in such a way.
> Whoops, I was wrong. You are setting w/ SET_SRC as UNSPEC which
> which is eliminated conditionally, which is much better to me.
>
> Few nits:
> 1.
> diff --git a/gcc/config/i386/subst.md b/gcc/config/i386/subst.md
> index 0bc22fd..2e632b9 100644
> --- a/gcc/config/i386/subst.md
> +++ b/gcc/config/i386/subst.md
> @@ -137,12 +137,12 @@
>
> (define_subst "round"
> [(set (match_operand:SUBST_A 0)
> - (match_operand:SUBST_A 1))]
> + (match_operand:SUBST_A 1))]
> "TARGET_AVX512F"
> Junk.
>
> 2. Check identation pls
>
> 3. Mention PR in ChangeLog entry
>
> 4. Add reg test please
>
> Overall I like this approach. We must somehow set explicit dependency
> between RC and actual op, why not this way?
>
> Could you pls make sure that CSE is still working for ops w/ identical RC?
>
> To be paranoid: is it possible to check skylake-avx512 w/ and w/o the patch
> on Spec2k6?
>
> --
> Thanks, K
0001-test_rounding.patch
Description: 0001-test_rounding.patch
