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

Message ID 20190211225553.32050-37-hjl.tools@gmail.com
State New
Headers show
Series
  • V4: Emulate MMX intrinsics with SSE
Related show

Commit Message

H.J. Lu Feb. 11, 2019, 10:55 p.m.
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.
---
 gcc/config/i386/i386.c |   8 +++
 gcc/config/i386/mmx.md | 129 +++++++++++++++++++++++++++++++++--------
 2 files changed, 113 insertions(+), 24 deletions(-)

-- 
2.20.1

Comments

Uros Bizjak Feb. 12, 2019, 1:42 p.m. | #1
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.

> ---

>  gcc/config/i386/i386.c |   8 +++

>  gcc/config/i386/mmx.md | 129 +++++++++++++++++++++++++++++++++--------

>  2 files changed, 113 insertions(+), 24 deletions(-)

>

> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c

> index 7d65192c1cd..4e776b8c3ea 100644

> --- a/gcc/config/i386/i386.c

> +++ b/gcc/config/i386/i386.c

> @@ -42365,6 +42365,7 @@ ix86_expand_vector_init_duplicate (bool mmx_ok, machine_mode mode,

>  {

>    bool ok;

>

> +  mmx_ok |= TARGET_MMX_WITH_SSE;

>    switch (mode)

>      {

>      case E_V2SImode:

> @@ -42524,6 +42525,7 @@ ix86_expand_vector_init_one_nonzero (bool mmx_ok, machine_mode mode,

>    bool use_vector_set = false;

>    rtx (*gen_vec_set_0) (rtx, rtx, rtx) = NULL;

>

> +  mmx_ok |= TARGET_MMX_WITH_SSE;

>    switch (mode)

>      {

>      case E_V2DImode:

> @@ -42717,6 +42719,7 @@ ix86_expand_vector_init_one_var (bool mmx_ok, machine_mode mode,

>    XVECEXP (const_vec, 0, one_var) = CONST0_RTX (GET_MODE_INNER (mode));

>    const_vec = gen_rtx_CONST_VECTOR (mode, XVEC (const_vec, 0));

>

> +  mmx_ok |= TARGET_MMX_WITH_SSE;

>    switch (mode)

>      {

>      case E_V2DFmode:

> @@ -43102,6 +43105,7 @@ ix86_expand_vector_init_general (bool mmx_ok, machine_mode mode,

>    machine_mode quarter_mode = VOIDmode;

>    int n, i;

>

> +  mmx_ok |= TARGET_MMX_WITH_SSE;

>    switch (mode)

>      {

>      case E_V2SFmode:

> @@ -43301,6 +43305,8 @@ ix86_expand_vector_init (bool mmx_ok, rtx target, rtx vals)

>    int i;

>    rtx x;

>

> +  mmx_ok |= TARGET_MMX_WITH_SSE;

> +

>    /* Handle first initialization from vector elts.  */

>    if (n_elts != XVECLEN (vals, 0))

>      {

> @@ -43400,6 +43406,7 @@ ix86_expand_vector_set (bool mmx_ok, rtx target, rtx val, int elt)

>    machine_mode mmode = VOIDmode;

>    rtx (*gen_blendm) (rtx, rtx, rtx, rtx);

>

> +  mmx_ok |= TARGET_MMX_WITH_SSE;

>    switch (mode)

>      {

>      case E_V2SFmode:

> @@ -43755,6 +43762,7 @@ ix86_expand_vector_extract (bool mmx_ok, rtx target, rtx vec, int elt)

>    bool use_vec_extr = false;

>    rtx tmp;

>

> +  mmx_ok |= TARGET_MMX_WITH_SSE;

>    switch (mode)

>      {

>      case E_V2SImode:

> diff --git a/gcc/config/i386/mmx.md b/gcc/config/i386/mmx.md

> index c8bd544dc9e..4e8b6e54b4c 100644

> --- a/gcc/config/i386/mmx.md

> +++ b/gcc/config/i386/mmx.md

> @@ -591,14 +591,23 @@

>     (set_attr "prefix_extra" "1")

>     (set_attr "mode" "V2SF")])

>

> -(define_insn "*vec_dupv2sf"

> -  [(set (match_operand:V2SF 0 "register_operand" "=y")

> +(define_insn_and_split "*vec_dupv2sf"

> +  [(set (match_operand:V2SF 0 "register_operand" "=y,x,Yv")

>         (vec_duplicate:V2SF

> -         (match_operand:SF 1 "register_operand" "0")))]

> -  "TARGET_MMX"

> -  "punpckldq\t%0, %0"

> -  [(set_attr "type" "mmxcvt")

> -   (set_attr "mode" "DI")])

> +         (match_operand:SF 1 "register_operand" "0,0,Yv")))]

> +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

> +  "@

> +   punpckldq\t%0, %0

> +   #

> +   #"

> +  "TARGET_MMX_WITH_SSE && reload_completed"

> +  [(set (match_dup 0)

> +       (vec_duplicate:V4SF (match_dup 1)))]

> +  "operands[0] = lowpart_subreg (V4SFmode, operands[0],

> +                                GET_MODE (operands[0]));"

> +  [(set_attr "mmx_isa" "native,x64_noavx,x64_avx")

> +   (set_attr "type" "mmxcvt,ssemov,ssemov")

> +   (set_attr "mode" "DI,TI,TI")])

>

>  (define_insn "*mmx_concatv2sf"

>    [(set (match_operand:V2SF 0 "register_operand"     "=y,y")

> @@ -616,7 +625,7 @@

>    [(match_operand:V2SF 0 "register_operand")

>     (match_operand:SF 1 "register_operand")

>     (match_operand 2 "const_int_operand")]

> -  "TARGET_MMX"

> +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

>  {

>    ix86_expand_vector_set (false, operands[0], operands[1],

>                           INTVAL (operands[2]));

> @@ -630,7 +639,20 @@

>         (vec_select:SF

>           (match_operand:V2SF 1 "nonimmediate_operand" " xm,x,ym,y,m,m")

>           (parallel [(const_int 0)])))]

> -  "TARGET_MMX && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

> +  "TARGET_MMX

> +   && !TARGET_MMX_WITH_SSE

> +   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

> +  "#"

> +  "&& reload_completed"

> +  [(set (match_dup 0) (match_dup 1))]

> +  "operands[1] = gen_lowpart (SFmode, operands[1]);")

> +

> +(define_insn_and_split "*vec_extractv2sf_0_sse"

> +  [(set (match_operand:SF 0 "nonimmediate_operand"     "=x, m,f,r")

> +       (vec_select:SF

> +         (match_operand:V2SF 1 "nonimmediate_operand" " xm,x,m,m")

> +         (parallel [(const_int 0)])))]

> +  "TARGET_MMX_WITH_SSE && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

>    "#"

>    "&& reload_completed"

>    [(set (match_dup 0) (match_dup 1))]

> @@ -643,7 +665,9 @@

>         (vec_select:SF

>           (match_operand:V2SF 1 "nonimmediate_operand" " 0,x,x,o,o,o,o")

>           (parallel [(const_int 1)])))]

> -  "TARGET_MMX && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

> +  "TARGET_MMX

> +   && !TARGET_MMX_WITH_SSE

> +   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

>    "@

>     punpckhdq\t%0, %0

>     %vmovshdup\t{%1, %0|%0, %1}

> @@ -665,12 +689,33 @@

>     (set_attr "prefix" "orig,maybe_vex,orig,orig,orig,orig,orig")

>     (set_attr "mode" "DI,V4SF,V4SF,SF,SF,SF,SF")])

>

> +(define_insn "*vec_extractv2sf_1_sse"

> +  [(set (match_operand:SF 0 "nonimmediate_operand"     "=x,x,x,f,r")

> +       (vec_select:SF

> +         (match_operand:V2SF 1 "nonimmediate_operand" " x,x,o,o,o")

> +         (parallel [(const_int 1)])))]

> +  "TARGET_MMX_WITH_SSE && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

> +  "@

> +   %vmovshdup\t{%1, %0|%0, %1}

> +   shufps\t{$0xe5, %1, %0|%0, %1, 0xe5}

> +   #

> +   #

> +   #"

> +  [(set_attr "isa" "sse3,noavx,*,*,*")

> +   (set_attr "type" "sse,sseshuf1,ssemov,fmov,imov")

> +   (set (attr "length_immediate")

> +     (if_then_else (eq_attr "alternative" "1")

> +                  (const_string "1")

> +                  (const_string "*")))

> +   (set_attr "prefix" "maybe_vex,orig,orig,orig,orig")

> +   (set_attr "mode" "V4SF,V4SF,SF,SF,SF")])

> +

>  (define_split

>    [(set (match_operand:SF 0 "register_operand")

>         (vec_select:SF

>           (match_operand:V2SF 1 "memory_operand")

>           (parallel [(const_int 1)])))]

> -  "TARGET_MMX && reload_completed"

> +  "(TARGET_MMX || TARGET_MMX_WITH_SSE) && reload_completed"

>    [(set (match_dup 0) (match_dup 1))]

>    "operands[1] = adjust_address (operands[1], SFmode, 4);")

>

> @@ -678,7 +723,7 @@

>    [(match_operand:SF 0 "register_operand")

>     (match_operand:V2SF 1 "register_operand")

>     (match_operand 2 "const_int_operand")]

> -  "TARGET_MMX"

> +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

>  {

>    ix86_expand_vector_extract (false, operands[0], operands[1],

>                               INTVAL (operands[2]));

> @@ -1574,7 +1619,7 @@

>    [(match_operand:V2SI 0 "register_operand")

>     (match_operand:SI 1 "register_operand")

>     (match_operand 2 "const_int_operand")]

> -  "TARGET_MMX"

> +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

>  {

>    ix86_expand_vector_set (false, operands[0], operands[1],

>                           INTVAL (operands[2]));

> @@ -1588,7 +1633,20 @@

>         (vec_select:SI

>           (match_operand:V2SI 1 "nonimmediate_operand" "xm,x,ym,y,m")

>           (parallel [(const_int 0)])))]

> -  "TARGET_MMX && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

> +  "TARGET_MMX

> +   && !TARGET_MMX_WITH_SSE

> +   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

> +  "#"

> +  "&& reload_completed"

> +  [(set (match_dup 0) (match_dup 1))]

> +  "operands[1] = gen_lowpart (SImode, operands[1]);")

> +

> +(define_insn_and_split "*vec_extractv2si_0_sse"

> +  [(set (match_operand:SI 0 "nonimmediate_operand"     "=x,m,r")

> +       (vec_select:SI

> +         (match_operand:V2SI 1 "nonimmediate_operand" "xm,x,m")

> +         (parallel [(const_int 0)])))]

> +  "TARGET_MMX_WITH_SSE && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

>    "#"

>    "&& reload_completed"

>    [(set (match_dup 0) (match_dup 1))]

> @@ -1601,7 +1659,9 @@

>         (vec_select:SI

>           (match_operand:V2SI 1 "nonimmediate_operand" " 0,x,x,o,o,o")

>           (parallel [(const_int 1)])))]

> -  "TARGET_MMX && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

> +  "TARGET_MMX

> +   && !TARGET_MMX_WITH_SSE

> +   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

>    "@

>     punpckhdq\t%0, %0

>     %vpshufd\t{$0xe5, %1, %0|%0, %1, 0xe5}

> @@ -1618,22 +1678,43 @@

>     (set_attr "prefix" "orig,maybe_vex,orig,orig,orig,orig")

>     (set_attr "mode" "DI,TI,V4SF,SI,SI,SI")])

>

> +(define_insn "*vec_extractv2si_1_sse"

> +  [(set (match_operand:SI 0 "nonimmediate_operand"     "=x,x,x,r")

> +       (vec_select:SI

> +         (match_operand:V2SI 1 "nonimmediate_operand" " x,x,o,o")

> +         (parallel [(const_int 1)])))]

> +  "TARGET_MMX_WITH_SSE

> +   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

> +  "@

> +   %vpshufd\t{$0xe5, %1, %0|%0, %1, 0xe5}

> +   shufps\t{$0xe5, %1, %0|%0, %1, 0xe5}

> +   #

> +   #"

> +  [(set_attr "isa" "sse2,noavx,*,*")

> +   (set_attr "type" "sseshuf1,sseshuf1,ssemov,imov")

> +   (set (attr "length_immediate")

> +     (if_then_else (eq_attr "alternative" "0,1")

> +                  (const_string "1")

> +                  (const_string "*")))

> +   (set_attr "prefix" "maybe_vex,orig,orig,orig")

> +   (set_attr "mode" "TI,V4SF,SI,SI")])

> +

>  (define_split

>    [(set (match_operand:SI 0 "register_operand")

>         (vec_select:SI

>           (match_operand:V2SI 1 "memory_operand")

>           (parallel [(const_int 1)])))]

> -  "TARGET_MMX && reload_completed"

> +  "(TARGET_MMX || TARGET_MMX_WITH_SSE) && reload_completed"

>    [(set (match_dup 0) (match_dup 1))]

>    "operands[1] = adjust_address (operands[1], SImode, 4);")

>

>  (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.

Uros.
>    "#"

>    "&& reload_completed"

>    [(set (match_dup 0) (zero_extend:DI (match_dup 1)))]

> @@ -1645,7 +1726,7 @@

>    [(match_operand:SI 0 "register_operand")

>     (match_operand:V2SI 1 "register_operand")

>     (match_operand 2 "const_int_operand")]

> -  "TARGET_MMX"

> +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

>  {

>    ix86_expand_vector_extract (false, operands[0], operands[1],

>                               INTVAL (operands[2]));

> @@ -1665,7 +1746,7 @@

>    [(match_operand:V4HI 0 "register_operand")

>     (match_operand:HI 1 "register_operand")

>     (match_operand 2 "const_int_operand")]

> -  "TARGET_MMX"

> +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

>  {

>    ix86_expand_vector_set (false, operands[0], operands[1],

>                           INTVAL (operands[2]));

> @@ -1676,7 +1757,7 @@

>    [(match_operand:HI 0 "register_operand")

>     (match_operand:V4HI 1 "register_operand")

>     (match_operand 2 "const_int_operand")]

> -  "TARGET_MMX"

> +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

>  {

>    ix86_expand_vector_extract (false, operands[0], operands[1],

>                               INTVAL (operands[2]));

> @@ -1696,7 +1777,7 @@

>    [(match_operand:V8QI 0 "register_operand")

>     (match_operand:QI 1 "register_operand")

>     (match_operand 2 "const_int_operand")]

> -  "TARGET_MMX"

> +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

>  {

>    ix86_expand_vector_set (false, operands[0], operands[1],

>                           INTVAL (operands[2]));

> @@ -1707,7 +1788,7 @@

>    [(match_operand:QI 0 "register_operand")

>     (match_operand:V8QI 1 "register_operand")

>     (match_operand 2 "const_int_operand")]

> -  "TARGET_MMX"

> +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

>  {

>    ix86_expand_vector_extract (false, operands[0], operands[1],

>                               INTVAL (operands[2]));

> --

> 2.20.1

>
H.J. Lu Feb. 12, 2019, 7:35 p.m. | #2
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:

(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")])

-- 
H.J.
Uros Bizjak Feb. 12, 2019, 7:44 p.m. | #3
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,*,*".

This way, it is clear that we enable alternative 0 only for native
mmx. It looks to me that we need to add similar treatment to a couple
of other patterns in sse.md, where we allow "y" constraint, e.g.
*vec_concatv2sf_sse, *vec_concatv2si_sse4_1, etc.

Uros.
H.J. Lu Feb. 12, 2019, 8:13 p.m. | #4
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:

MOVD (when destination operand is XMM register)
DEST[31:0] ← SRC;
DEST[127:32] ← 000000000000000000000000H;
DEST[MAXVL-1:128] (Unmodified)

> This way, it is clear that we enable alternative 0 only for native

> mmx. It looks to me that we need to add similar treatment to a couple

> of other patterns in sse.md, where we allow "y" constraint, e.g.

> *vec_concatv2sf_sse, *vec_concatv2si_sse4_1, etc.

>


I will take a look.

Thanks.

-- 
H.J.
Uros Bizjak Feb. 12, 2019, 8:24 p.m. | #5
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.

BTW: sse2 is not a member of mmx_isa. attribute.

Uros.

>> This way, it is clear that we enable alternative 0 only for native

>> mmx. It looks to me that we need to add similar treatment to a couple

>> of other patterns in sse.md, where we allow "y" constraint, e.g.

>> *vec_concatv2sf_sse, *vec_concatv2si_sse4_1, etc.

>>

>

> I will take a look.

>

> Thanks.

>

> --

> H.J.

>
Uros Bizjak Feb. 12, 2019, 8:29 p.m. | #6
On 2/12/19, H.J. Lu <hjl.tools@gmail.com> wrote:

>> This way, it is clear that we enable alternative 0 only for native

>> mmx. It looks to me that we need to add similar treatment to a couple

>> of other patterns in sse.md, where we allow "y" constraint, e.g.

>> *vec_concatv2sf_sse, *vec_concatv2si_sse4_1, etc.

>>

>

> I will take a look.


From a quick look to mentioned pattern, I have see that a couple of
movd with XMM reg are wrongly marked as sse2 isa. movd/movq with MMX
regs are base MMX instructions.

Uros.
H.J. Lu Feb. 12, 2019, 8:44 p.m. | #7
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?

> BTW: sse2 is not a member of mmx_isa. attribute.


If not, I can add sse2 to  mmx_isa.

> Uros.

>

> >> This way, it is clear that we enable alternative 0 only for native

> >> mmx. It looks to me that we need to add similar treatment to a couple

> >> of other patterns in sse.md, where we allow "y" constraint, e.g.

> >> *vec_concatv2sf_sse, *vec_concatv2si_sse4_1, etc.

> >>

> >

> > I will take a look.

> >

> > Thanks.

> >

> > --

> > H.J.

> >




-- 
H.J.
Uros Bizjak Feb. 13, 2019, 8:26 a.m. | #8
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.
H.J. Lu Feb. 13, 2019, 6:58 p.m. | #9
On Tue, Feb 12, 2019 at 12:29 PM Uros Bizjak <ubizjak@gmail.com> wrote:
>

> On 2/12/19, H.J. Lu <hjl.tools@gmail.com> wrote:

>

> >> This way, it is clear that we enable alternative 0 only for native

> >> mmx. It looks to me that we need to add similar treatment to a couple

> >> of other patterns in sse.md, where we allow "y" constraint, e.g.

> >> *vec_concatv2sf_sse, *vec_concatv2si_sse4_1, etc.

> >>

> >

> > I will take a look.

>

> From a quick look to mentioned pattern, I have see that a couple of

> movd with XMM reg are wrongly marked as sse2 isa. movd/movq with MMX

> regs are base MMX instructions.


I opened:

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

-- 
H.J.

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 7d65192c1cd..4e776b8c3ea 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -42365,6 +42365,7 @@  ix86_expand_vector_init_duplicate (bool mmx_ok, machine_mode mode,
 {
   bool ok;
 
+  mmx_ok |= TARGET_MMX_WITH_SSE;
   switch (mode)
     {
     case E_V2SImode:
@@ -42524,6 +42525,7 @@  ix86_expand_vector_init_one_nonzero (bool mmx_ok, machine_mode mode,
   bool use_vector_set = false;
   rtx (*gen_vec_set_0) (rtx, rtx, rtx) = NULL;
 
+  mmx_ok |= TARGET_MMX_WITH_SSE;
   switch (mode)
     {
     case E_V2DImode:
@@ -42717,6 +42719,7 @@  ix86_expand_vector_init_one_var (bool mmx_ok, machine_mode mode,
   XVECEXP (const_vec, 0, one_var) = CONST0_RTX (GET_MODE_INNER (mode));
   const_vec = gen_rtx_CONST_VECTOR (mode, XVEC (const_vec, 0));
 
+  mmx_ok |= TARGET_MMX_WITH_SSE;
   switch (mode)
     {
     case E_V2DFmode:
@@ -43102,6 +43105,7 @@  ix86_expand_vector_init_general (bool mmx_ok, machine_mode mode,
   machine_mode quarter_mode = VOIDmode;
   int n, i;
 
+  mmx_ok |= TARGET_MMX_WITH_SSE;
   switch (mode)
     {
     case E_V2SFmode:
@@ -43301,6 +43305,8 @@  ix86_expand_vector_init (bool mmx_ok, rtx target, rtx vals)
   int i;
   rtx x;
 
+  mmx_ok |= TARGET_MMX_WITH_SSE;
+
   /* Handle first initialization from vector elts.  */
   if (n_elts != XVECLEN (vals, 0))
     {
@@ -43400,6 +43406,7 @@  ix86_expand_vector_set (bool mmx_ok, rtx target, rtx val, int elt)
   machine_mode mmode = VOIDmode;
   rtx (*gen_blendm) (rtx, rtx, rtx, rtx);
 
+  mmx_ok |= TARGET_MMX_WITH_SSE;
   switch (mode)
     {
     case E_V2SFmode:
@@ -43755,6 +43762,7 @@  ix86_expand_vector_extract (bool mmx_ok, rtx target, rtx vec, int elt)
   bool use_vec_extr = false;
   rtx tmp;
 
+  mmx_ok |= TARGET_MMX_WITH_SSE;
   switch (mode)
     {
     case E_V2SImode:
diff --git a/gcc/config/i386/mmx.md b/gcc/config/i386/mmx.md
index c8bd544dc9e..4e8b6e54b4c 100644
--- a/gcc/config/i386/mmx.md
+++ b/gcc/config/i386/mmx.md
@@ -591,14 +591,23 @@ 
    (set_attr "prefix_extra" "1")
    (set_attr "mode" "V2SF")])
 
-(define_insn "*vec_dupv2sf"
-  [(set (match_operand:V2SF 0 "register_operand" "=y")
+(define_insn_and_split "*vec_dupv2sf"
+  [(set (match_operand:V2SF 0 "register_operand" "=y,x,Yv")
 	(vec_duplicate:V2SF
-	  (match_operand:SF 1 "register_operand" "0")))]
-  "TARGET_MMX"
-  "punpckldq\t%0, %0"
-  [(set_attr "type" "mmxcvt")
-   (set_attr "mode" "DI")])
+	  (match_operand:SF 1 "register_operand" "0,0,Yv")))]
+  "TARGET_MMX || TARGET_MMX_WITH_SSE"
+  "@
+   punpckldq\t%0, %0
+   #
+   #"
+  "TARGET_MMX_WITH_SSE && reload_completed"
+  [(set (match_dup 0)
+	(vec_duplicate:V4SF (match_dup 1)))]
+  "operands[0] = lowpart_subreg (V4SFmode, operands[0],
+				 GET_MODE (operands[0]));"
+  [(set_attr "mmx_isa" "native,x64_noavx,x64_avx")
+   (set_attr "type" "mmxcvt,ssemov,ssemov")
+   (set_attr "mode" "DI,TI,TI")])
 
 (define_insn "*mmx_concatv2sf"
   [(set (match_operand:V2SF 0 "register_operand"     "=y,y")
@@ -616,7 +625,7 @@ 
   [(match_operand:V2SF 0 "register_operand")
    (match_operand:SF 1 "register_operand")
    (match_operand 2 "const_int_operand")]
-  "TARGET_MMX"
+  "TARGET_MMX || TARGET_MMX_WITH_SSE"
 {
   ix86_expand_vector_set (false, operands[0], operands[1],
 			  INTVAL (operands[2]));
@@ -630,7 +639,20 @@ 
 	(vec_select:SF
 	  (match_operand:V2SF 1 "nonimmediate_operand" " xm,x,ym,y,m,m")
 	  (parallel [(const_int 0)])))]
-  "TARGET_MMX && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+  "TARGET_MMX
+   && !TARGET_MMX_WITH_SSE
+   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+  "#"
+  "&& reload_completed"
+  [(set (match_dup 0) (match_dup 1))]
+  "operands[1] = gen_lowpart (SFmode, operands[1]);")
+
+(define_insn_and_split "*vec_extractv2sf_0_sse"
+  [(set (match_operand:SF 0 "nonimmediate_operand"     "=x, m,f,r")
+	(vec_select:SF
+	  (match_operand:V2SF 1 "nonimmediate_operand" " xm,x,m,m")
+	  (parallel [(const_int 0)])))]
+  "TARGET_MMX_WITH_SSE && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
   "#"
   "&& reload_completed"
   [(set (match_dup 0) (match_dup 1))]
@@ -643,7 +665,9 @@ 
 	(vec_select:SF
 	  (match_operand:V2SF 1 "nonimmediate_operand" " 0,x,x,o,o,o,o")
 	  (parallel [(const_int 1)])))]
-  "TARGET_MMX && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+  "TARGET_MMX
+   && !TARGET_MMX_WITH_SSE
+   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
   "@
    punpckhdq\t%0, %0
    %vmovshdup\t{%1, %0|%0, %1}
@@ -665,12 +689,33 @@ 
    (set_attr "prefix" "orig,maybe_vex,orig,orig,orig,orig,orig")
    (set_attr "mode" "DI,V4SF,V4SF,SF,SF,SF,SF")])
 
+(define_insn "*vec_extractv2sf_1_sse"
+  [(set (match_operand:SF 0 "nonimmediate_operand"     "=x,x,x,f,r")
+	(vec_select:SF
+	  (match_operand:V2SF 1 "nonimmediate_operand" " x,x,o,o,o")
+	  (parallel [(const_int 1)])))]
+  "TARGET_MMX_WITH_SSE && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+  "@
+   %vmovshdup\t{%1, %0|%0, %1}
+   shufps\t{$0xe5, %1, %0|%0, %1, 0xe5}
+   #
+   #
+   #"
+  [(set_attr "isa" "sse3,noavx,*,*,*")
+   (set_attr "type" "sse,sseshuf1,ssemov,fmov,imov")
+   (set (attr "length_immediate")
+     (if_then_else (eq_attr "alternative" "1")
+		   (const_string "1")
+		   (const_string "*")))
+   (set_attr "prefix" "maybe_vex,orig,orig,orig,orig")
+   (set_attr "mode" "V4SF,V4SF,SF,SF,SF")])
+
 (define_split
   [(set (match_operand:SF 0 "register_operand")
 	(vec_select:SF
 	  (match_operand:V2SF 1 "memory_operand")
 	  (parallel [(const_int 1)])))]
-  "TARGET_MMX && reload_completed"
+  "(TARGET_MMX || TARGET_MMX_WITH_SSE) && reload_completed"
   [(set (match_dup 0) (match_dup 1))]
   "operands[1] = adjust_address (operands[1], SFmode, 4);")
 
@@ -678,7 +723,7 @@ 
   [(match_operand:SF 0 "register_operand")
    (match_operand:V2SF 1 "register_operand")
    (match_operand 2 "const_int_operand")]
-  "TARGET_MMX"
+  "TARGET_MMX || TARGET_MMX_WITH_SSE"
 {
   ix86_expand_vector_extract (false, operands[0], operands[1],
 			      INTVAL (operands[2]));
@@ -1574,7 +1619,7 @@ 
   [(match_operand:V2SI 0 "register_operand")
    (match_operand:SI 1 "register_operand")
    (match_operand 2 "const_int_operand")]
-  "TARGET_MMX"
+  "TARGET_MMX || TARGET_MMX_WITH_SSE"
 {
   ix86_expand_vector_set (false, operands[0], operands[1],
 			  INTVAL (operands[2]));
@@ -1588,7 +1633,20 @@ 
 	(vec_select:SI
 	  (match_operand:V2SI 1 "nonimmediate_operand" "xm,x,ym,y,m")
 	  (parallel [(const_int 0)])))]
-  "TARGET_MMX && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+  "TARGET_MMX
+   && !TARGET_MMX_WITH_SSE
+   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+  "#"
+  "&& reload_completed"
+  [(set (match_dup 0) (match_dup 1))]
+  "operands[1] = gen_lowpart (SImode, operands[1]);")
+
+(define_insn_and_split "*vec_extractv2si_0_sse"
+  [(set (match_operand:SI 0 "nonimmediate_operand"     "=x,m,r")
+	(vec_select:SI
+	  (match_operand:V2SI 1 "nonimmediate_operand" "xm,x,m")
+	  (parallel [(const_int 0)])))]
+  "TARGET_MMX_WITH_SSE && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
   "#"
   "&& reload_completed"
   [(set (match_dup 0) (match_dup 1))]
@@ -1601,7 +1659,9 @@ 
 	(vec_select:SI
 	  (match_operand:V2SI 1 "nonimmediate_operand" " 0,x,x,o,o,o")
 	  (parallel [(const_int 1)])))]
-  "TARGET_MMX && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+  "TARGET_MMX
+   && !TARGET_MMX_WITH_SSE
+   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
   "@
    punpckhdq\t%0, %0
    %vpshufd\t{$0xe5, %1, %0|%0, %1, 0xe5}
@@ -1618,22 +1678,43 @@ 
    (set_attr "prefix" "orig,maybe_vex,orig,orig,orig,orig")
    (set_attr "mode" "DI,TI,V4SF,SI,SI,SI")])
 
+(define_insn "*vec_extractv2si_1_sse"
+  [(set (match_operand:SI 0 "nonimmediate_operand"     "=x,x,x,r")
+	(vec_select:SI
+	  (match_operand:V2SI 1 "nonimmediate_operand" " x,x,o,o")
+	  (parallel [(const_int 1)])))]
+  "TARGET_MMX_WITH_SSE
+   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+  "@
+   %vpshufd\t{$0xe5, %1, %0|%0, %1, 0xe5}
+   shufps\t{$0xe5, %1, %0|%0, %1, 0xe5}
+   #
+   #"
+  [(set_attr "isa" "sse2,noavx,*,*")
+   (set_attr "type" "sseshuf1,sseshuf1,ssemov,imov")
+   (set (attr "length_immediate")
+     (if_then_else (eq_attr "alternative" "0,1")
+		   (const_string "1")
+		   (const_string "*")))
+   (set_attr "prefix" "maybe_vex,orig,orig,orig")
+   (set_attr "mode" "TI,V4SF,SI,SI")])
+
 (define_split
   [(set (match_operand:SI 0 "register_operand")
 	(vec_select:SI
 	  (match_operand:V2SI 1 "memory_operand")
 	  (parallel [(const_int 1)])))]
-  "TARGET_MMX && reload_completed"
+  "(TARGET_MMX || TARGET_MMX_WITH_SSE) && reload_completed"
   [(set (match_dup 0) (match_dup 1))]
   "operands[1] = adjust_address (operands[1], SImode, 4);")
 
 (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"
   "#"
   "&& reload_completed"
   [(set (match_dup 0) (zero_extend:DI (match_dup 1)))]
@@ -1645,7 +1726,7 @@ 
   [(match_operand:SI 0 "register_operand")
    (match_operand:V2SI 1 "register_operand")
    (match_operand 2 "const_int_operand")]
-  "TARGET_MMX"
+  "TARGET_MMX || TARGET_MMX_WITH_SSE"
 {
   ix86_expand_vector_extract (false, operands[0], operands[1],
 			      INTVAL (operands[2]));
@@ -1665,7 +1746,7 @@ 
   [(match_operand:V4HI 0 "register_operand")
    (match_operand:HI 1 "register_operand")
    (match_operand 2 "const_int_operand")]
-  "TARGET_MMX"
+  "TARGET_MMX || TARGET_MMX_WITH_SSE"
 {
   ix86_expand_vector_set (false, operands[0], operands[1],
 			  INTVAL (operands[2]));
@@ -1676,7 +1757,7 @@ 
   [(match_operand:HI 0 "register_operand")
    (match_operand:V4HI 1 "register_operand")
    (match_operand 2 "const_int_operand")]
-  "TARGET_MMX"
+  "TARGET_MMX || TARGET_MMX_WITH_SSE"
 {
   ix86_expand_vector_extract (false, operands[0], operands[1],
 			      INTVAL (operands[2]));
@@ -1696,7 +1777,7 @@ 
   [(match_operand:V8QI 0 "register_operand")
    (match_operand:QI 1 "register_operand")
    (match_operand 2 "const_int_operand")]
-  "TARGET_MMX"
+  "TARGET_MMX || TARGET_MMX_WITH_SSE"
 {
   ix86_expand_vector_set (false, operands[0], operands[1],
 			  INTVAL (operands[2]));
@@ -1707,7 +1788,7 @@ 
   [(match_operand:QI 0 "register_operand")
    (match_operand:V8QI 1 "register_operand")
    (match_operand 2 "const_int_operand")]
-  "TARGET_MMX"
+  "TARGET_MMX || TARGET_MMX_WITH_SSE"
 {
   ix86_expand_vector_extract (false, operands[0], operands[1],
 			      INTVAL (operands[2]));