[PATCH 36/40] i386: Allow MMX vector expanders with TARGET_MMX_WITH_SSE

Uros Bizjak ubizjak@gmail.com
Wed Feb 13 08:26:00 GMT 2019


On Tue, Feb 12, 2019 at 9:44 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Feb 12, 2019 at 12:24 PM Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > On 2/12/19, H.J. Lu <hjl.tools@gmail.com> wrote:
> > > On Tue, Feb 12, 2019 at 11:44 AM Uros Bizjak <ubizjak@gmail.com> wrote:
> > >>
> > >> On Tue, Feb 12, 2019 at 8:35 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >> >
> > >> > On Tue, Feb 12, 2019 at 5:43 AM Uros Bizjak <ubizjak@gmail.com> wrote:
> > >> > >
> > >> > > On Mon, Feb 11, 2019 at 11:55 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >> > > >
> > >> > > >         PR target/89021
> > >> > > >         * config/i386/i386.c (ix86_expand_vector_init_duplicate):
> > >> > > > Set
> > >> > > >         mmx_ok to true if TARGET_MMX_WITH_SSE is true.
> > >> > > >         (ix86_expand_vector_init_one_nonzero): Likewise.
> > >> > > >         (ix86_expand_vector_init_one_var): Likewise.
> > >> > > >         (ix86_expand_vector_init_general): Likewise.
> > >> > > >         (ix86_expand_vector_init): Likewise.
> > >> > > >         (ix86_expand_vector_set): Likewise.
> > >> > > >         (ix86_expand_vector_extract): Likewise.
> > >> > > >         * config/i386/mmx.md (*vec_dupv2sf): Changed to
> > >> > > >         define_insn_and_split to support SSE emulation.
> > >> > > >         (vec_setv2sf): Also allow TARGET_MMX_WITH_SSE.
> > >> > > >         (vec_extractv2sf_1 splitter): Likewise.
> > >> > > >         (vec_extractv2sfsf): Likewise.
> > >> > > >         (vec_setv2si): Likewise.
> > >> > > >         (vec_extractv2si_1 splitter): Likewise.
> > >> > > >         (vec_extractv2sisi): Likewise.
> > >> > > >         (vec_setv4hi): Likewise.
> > >> > > >         (vec_extractv4hihi): Likewise.
> > >> > > >         (vec_setv8qi): Likewise.
> > >> > > >         (vec_extractv8qiqi): Likewise.
> > >> > > >         (*vec_extractv2sf_0): Don't allow TARGET_MMX_WITH_SSE.
> > >> > > >         (*vec_extractv2sf_1): Likewise.
> > >> > > >         (*vec_extractv2si_0): Likewise.
> > >> > > >         (*vec_extractv2si_1): Likewise.
> > >> > > >         (*vec_extractv2sf_0_sse): New.
> > >> > > >         (*vec_extractv2sf_1_sse): Likewise.
> > >> > > >         (*vec_extractv2si_0_sse): Likewise.
> > >> > > >         (*vec_extractv2si_1_sse): Likewise.
> > >> > >
> > >> > > Please do not introduce new *_sse patterns, use mmx_isa attribute to
> > >> > > disable unwanted alternatives.
> > >> >
> > >> > Will do.
> > >> >
> > >> > > >  (define_insn_and_split "*vec_extractv2si_zext_mem"
> > >> > > > -  [(set (match_operand:DI 0 "register_operand" "=y,x,r")
> > >> > > > +  [(set (match_operand:DI 0 "register_operand" "=x,r")
> > >> > > >         (zero_extend:DI
> > >> > > >           (vec_select:SI
> > >> > > > -           (match_operand:V2SI 1 "memory_operand" "o,o,o")
> > >> > > > +           (match_operand:V2SI 1 "memory_operand" "o,o")
> > >> > > >             (parallel [(match_operand:SI 2
> > >> > > > "const_0_to_1_operand")]))))]
> > >> > > > -  "TARGET_64BIT && TARGET_MMX"
> > >> > > > +  "TARGET_64BIT"
> > >> > >
> > >> > > Here you need TARGET_64BIT && (TARGET_MMX || TARGET_MMX_WITH_SSE) and
> > >> > > mmx_isa attribute.
> > >> > >
> > >> >
> > >> > Why is && (TARGET_MMX || TARGET_MMX_WITH_SSE) needed?  The 3rd
> > >> > alternative doesn't need MMX nor SSE2:
> > >>
> > >> Ah, I didn't notice that. LGTM then.
> > >>
> > >> > (define_insn_and_split "*vec_extractv2si_zext_mem"
> > >> >   [(set (match_operand:DI 0 "register_operand" "=y,x,r")
> > >> >         (zero_extend:DI
> > >> >           (vec_select:SI
> > >> >             (match_operand:V2SI 1 "memory_operand" "o,o,o")
> > >> >             (parallel [(match_operand:SI 2
> > >> > "const_0_to_1_operand")]))))]
> > >> >   "TARGET_64BIT"
> > >> >   "#"
> > >> >   "&& reload_completed"
> > >> >   [(set (match_dup 0) (zero_extend:DI (match_dup 1)))]
> > >> > {
> > >> >   operands[1] = adjust_address (operands[1], SImode, INTVAL
> > >> > (operands[2]) * 4);
> > >> > }
> > >> >   [(set_attr "mmx_isa" "native,sse2,base")])
> > >>
> > >> Please write this as "native,*,*".
> > >
> > > Did you mean "native,sse2,*"?  The second alternative is SSE2 MOVD:
> >
> > No, my proposed definition is OK, see below.
> >
> > > MOVD (when destination operand is XMM register)
> > > DEST[31:0] ← SRC;
> > > DEST[127:32] ← 000000000000000000000000H;
> > > DEST[MAXVL-1:128] (Unmodified)
> >
> > You should also add "isa" attribute with "*,sse2,*", which should be
> > there from the beginning.
>
> Can we have both isa and mmx_isa attributes in the same pattern?

Sure, but some limitations apply: "isa" should be first and it has to
have "*" where mmx_isa is specified (and vice versa). When "enabled"
is calculated, calculation first scans "isa" attribute, until it hits
some known value, otherwise the calculation proceeds to "mmx_isa"
attribute, and if it doesn't hit anything, proceeds to default 1.

> > BTW: sse2 is not a member of mmx_isa. attribute.
>
> If not, I can add sse2 to  mmx_isa.

Maybe we indeed need it, but let's try without.

Uros.



More information about the Gcc-patches mailing list