[27/42] i386: Make _mm_empty () as NOP when MMX is disabled

Message ID 20190216003408.23761-28-hjl.tools@gmail.com
State Superseded
Headers show
Series
  • V7: Emulate MMX intrinsics with SSE
Related show

Commit Message

H.J. Lu Feb. 16, 2019, 12:33 a.m.
With SSE emulation of MMX intrinsics, we should make _mm_empty () as NOP
when MMX is disabled.

	PR target/89021
	* config/i386/mmx.md (mmx_<emms>): Renamed to ...
	(mmx_<emms>_1): This.
	(mmx_<emms>): New expander.
---
 gcc/config/i386/mmx.md | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

-- 
2.20.1

Comments

Uros Bizjak Feb. 16, 2019, 8:58 a.m. | #1
On 2/16/19, H.J. Lu <hjl.tools@gmail.com> wrote:
> With SSE emulation of MMX intrinsics, we should make _mm_empty () as NOP

> when MMX is disabled.

>

> 	PR target/89021

> 	* config/i386/mmx.md (mmx_<emms>): Renamed to ...

> 	(mmx_<emms>_1): This.

> 	(mmx_<emms>): New expander.

> ---

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

>  1 file changed, 28 insertions(+), 1 deletion(-)

>

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

> index 9cf0251293a..0f925c0b1ea 100644

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

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

> @@ -1848,7 +1848,34 @@

>    [(UNSPECV_EMMS "emms")

>     (UNSPECV_FEMMS "femms")])

>

> -(define_insn "mmx_<emms>"

> +(define_expand "mmx_<emms>"

> +  [(unspec_volatile [(const_int 0)] EMMS)

> +   (clobber (reg:XF ST0_REG))

> +   (clobber (reg:XF ST1_REG))

> +   (clobber (reg:XF ST2_REG))

> +   (clobber (reg:XF ST3_REG))

> +   (clobber (reg:XF ST4_REG))

> +   (clobber (reg:XF ST5_REG))

> +   (clobber (reg:XF ST6_REG))

> +   (clobber (reg:XF ST7_REG))

> +   (clobber (reg:DI MM0_REG))

> +   (clobber (reg:DI MM1_REG))

> +   (clobber (reg:DI MM2_REG))

> +   (clobber (reg:DI MM3_REG))

> +   (clobber (reg:DI MM4_REG))

> +   (clobber (reg:DI MM5_REG))

> +   (clobber (reg:DI MM6_REG))

> +   (clobber (reg:DI MM7_REG))]

> +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

> +{

> +   if (TARGET_MMX)

> +     emit_insn (gen_mmx_<emms>_1 ());

> +   else

> +     emit_insn (gen_nop ());

> +   DONE;


The above should be written as:

if (!TARGET_MMX)
  {
    emit_insn (gen_nop ()));
    DONE;
  }

> +})

> +

> +(define_insn "mmx_<emms>_1"


The old insn should be renamed to "*mmx_<emms>".

Uros.

>    [(unspec_volatile [(const_int 0)] EMMS)

>     (clobber (reg:XF ST0_REG))

>     (clobber (reg:XF ST1_REG))

> --

> 2.20.1

>

>
H.J. Lu Feb. 16, 2019, 2:56 p.m. | #2
On Sat, Feb 16, 2019 at 12:58 AM Uros Bizjak <ubizjak@gmail.com> wrote:
>

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

> > With SSE emulation of MMX intrinsics, we should make _mm_empty () as NOP

> > when MMX is disabled.

> >

> >       PR target/89021

> >       * config/i386/mmx.md (mmx_<emms>): Renamed to ...

> >       (mmx_<emms>_1): This.

> >       (mmx_<emms>): New expander.

> > ---

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

> >  1 file changed, 28 insertions(+), 1 deletion(-)

> >

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

> > index 9cf0251293a..0f925c0b1ea 100644

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

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

> > @@ -1848,7 +1848,34 @@

> >    [(UNSPECV_EMMS "emms")

> >     (UNSPECV_FEMMS "femms")])

> >

> > -(define_insn "mmx_<emms>"

> > +(define_expand "mmx_<emms>"

> > +  [(unspec_volatile [(const_int 0)] EMMS)

> > +   (clobber (reg:XF ST0_REG))

> > +   (clobber (reg:XF ST1_REG))

> > +   (clobber (reg:XF ST2_REG))

> > +   (clobber (reg:XF ST3_REG))

> > +   (clobber (reg:XF ST4_REG))

> > +   (clobber (reg:XF ST5_REG))

> > +   (clobber (reg:XF ST6_REG))

> > +   (clobber (reg:XF ST7_REG))

> > +   (clobber (reg:DI MM0_REG))

> > +   (clobber (reg:DI MM1_REG))

> > +   (clobber (reg:DI MM2_REG))

> > +   (clobber (reg:DI MM3_REG))

> > +   (clobber (reg:DI MM4_REG))

> > +   (clobber (reg:DI MM5_REG))

> > +   (clobber (reg:DI MM6_REG))

> > +   (clobber (reg:DI MM7_REG))]

> > +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

> > +{

> > +   if (TARGET_MMX)

> > +     emit_insn (gen_mmx_<emms>_1 ());

> > +   else

> > +     emit_insn (gen_nop ());

> > +   DONE;

>

> The above should be written as:

>

> if (!TARGET_MMX)

>   {

>     emit_insn (gen_nop ()));

>     DONE;

>   }

>

> > +})

> > +

> > +(define_insn "mmx_<emms>_1"

>

> The old insn should be renamed to "*mmx_<emms>".

>

> Uros.


Tried and got

[hjl@gnu-cfl-2 gcc]$ cat x.c
#include <mmintrin.h>

void
foo (void)
{
  _mm_empty ();
}
[hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S x.c -da
x.c: In function ‘foo’:
x.c:7:1: error: unrecognizable insn:
    7 | }
      | ^
(insn 5 2 6 2 (unspec_volatile [
            (const_int 0 [0])
        ] UNSPECV_EMMS) "./include/mmintrin.h":60:3 -1
     (nil))
during RTL pass: vregs
dump file: x.c.234r.vregs
x.c:7:1: internal compiler error: in extract_insn, at recog.c:2310
0x10ad84d _fatal_insn(char const*, rtx_def const*, char const*, int,
char const*)
/export/gnu/import/git/gitlab/x86-gcc/gcc/rtl-error.c:108
0x10ad88e _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
/export/gnu/import/git/gitlab/x86-gcc/gcc/rtl-error.c:116
0x1042abb extract_insn(rtx_insn*)
/export/gnu/import/git/gitlab/x86-gcc/gcc/recog.c:2310
0xc95912 instantiate_virtual_regs_in_insn
/export/gnu/import/git/gitlab/x86-gcc/gcc/function.c:1654
0xc96d44 instantiate_virtual_regs
/export/gnu/import/git/gitlab/x86-gcc/gcc/function.c:1975
0xc96e0e execute
/export/gnu/import/git/gitlab/x86-gcc/gcc/function.c:2024
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
[hjl@gnu-cfl-2 gcc]$

;;
;; Full RTL generated for this function:
;;
(note 1 0 3 NOTE_INSN_DELETED)
;; basic block 2, loop depth 0, maybe hot
;;  prev block 0, next block 1, flags: (NEW, REACHABLE, RTL)
;;  pred:       ENTRY (FALLTHRU)
(note 3 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(note 2 3 5 2 NOTE_INSN_FUNCTION_BEG)
(insn 5 2 6 2 (unspec_volatile [
            (const_int 0 [0])
        ] UNSPECV_EMMS) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 6 5 7 2 (clobber (reg:XF 8 st)) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 7 6 8 2 (clobber (reg:XF 9 st(1))) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 8 7 9 2 (clobber (reg:XF 10 st(2))) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 9 8 10 2 (clobber (reg:XF 11 st(3))) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 10 9 11 2 (clobber (reg:XF 12 st(4))) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 11 10 12 2 (clobber (reg:XF 13 st(5))) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 12 11 13 2 (clobber (reg:XF 14 st(6))) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 13 12 14 2 (clobber (reg:XF 15 st(7))) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 14 13 15 2 (clobber (reg:DI 28 mm0)) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 15 14 16 2 (clobber (reg:DI 29 mm1)) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 16 15 17 2 (clobber (reg:DI 30 mm2)) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 17 16 18 2 (clobber (reg:DI 31 mm3)) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 18 17 19 2 (clobber (reg:DI 32 mm4)) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 19 18 20 2 (clobber (reg:DI 33 mm5)) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 20 19 21 2 (clobber (reg:DI 34 mm6)) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 21 20 25 2 (clobber (reg:DI 35 mm7)) "./include/mmintrin.h":60:3 -1
     (nil))
(insn 25 21 0 2 (const_int 0 [0]) "./include/mmintrin.h":61:1 -1
     (nil))
;;  succ:       EXIT [always]  (FALLTHRU)



-- 
H.J.
Uros Bizjak Feb. 16, 2019, 7:02 p.m. | #3
On 2/16/19, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, Feb 16, 2019 at 12:58 AM Uros Bizjak <ubizjak@gmail.com> wrote:

>>

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

>> > With SSE emulation of MMX intrinsics, we should make _mm_empty () as

>> > NOP

>> > when MMX is disabled.

>> >

>> >       PR target/89021

>> >       * config/i386/mmx.md (mmx_<emms>): Renamed to ...

>> >       (mmx_<emms>_1): This.

>> >       (mmx_<emms>): New expander.

>> > ---

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

>> >  1 file changed, 28 insertions(+), 1 deletion(-)

>> >

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

>> > index 9cf0251293a..0f925c0b1ea 100644

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

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

>> > @@ -1848,7 +1848,34 @@

>> >    [(UNSPECV_EMMS "emms")

>> >     (UNSPECV_FEMMS "femms")])

>> >

>> > -(define_insn "mmx_<emms>"

>> > +(define_expand "mmx_<emms>"

>> > +  [(unspec_volatile [(const_int 0)] EMMS)

>> > +   (clobber (reg:XF ST0_REG))

>> > +   (clobber (reg:XF ST1_REG))

>> > +   (clobber (reg:XF ST2_REG))

>> > +   (clobber (reg:XF ST3_REG))

>> > +   (clobber (reg:XF ST4_REG))

>> > +   (clobber (reg:XF ST5_REG))

>> > +   (clobber (reg:XF ST6_REG))

>> > +   (clobber (reg:XF ST7_REG))

>> > +   (clobber (reg:DI MM0_REG))

>> > +   (clobber (reg:DI MM1_REG))

>> > +   (clobber (reg:DI MM2_REG))

>> > +   (clobber (reg:DI MM3_REG))

>> > +   (clobber (reg:DI MM4_REG))

>> > +   (clobber (reg:DI MM5_REG))

>> > +   (clobber (reg:DI MM6_REG))

>> > +   (clobber (reg:DI MM7_REG))]

>> > +  "TARGET_MMX || TARGET_MMX_WITH_SSE"

>> > +{

>> > +   if (TARGET_MMX)

>> > +     emit_insn (gen_mmx_<emms>_1 ());

>> > +   else

>> > +     emit_insn (gen_nop ());

>> > +   DONE;

>>

>> The above should be written as:

>>

>> if (!TARGET_MMX)

>>   {

>>     emit_insn (gen_nop ()));

>>     DONE;

>>   }

>>

>> > +})

>> > +

>> > +(define_insn "mmx_<emms>_1"

>>

>> The old insn should be renamed to "*mmx_<emms>".

>>

>> Uros.

>

> Tried and got


You have to wrap the pattern in a parallel in the expander.

Uros.

>

> [hjl@gnu-cfl-2 gcc]$ cat x.c

> #include <mmintrin.h>

>

> void

> foo (void)

> {

>   _mm_empty ();

> }

> [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S x.c -da

> x.c: In function ‘foo’:

> x.c:7:1: error: unrecognizable insn:

>     7 | }

>       | ^

> (insn 5 2 6 2 (unspec_volatile [

>             (const_int 0 [0])

>         ] UNSPECV_EMMS) "./include/mmintrin.h":60:3 -1

>      (nil))

> during RTL pass: vregs

> dump file: x.c.234r.vregs

> x.c:7:1: internal compiler error: in extract_insn, at recog.c:2310

> 0x10ad84d _fatal_insn(char const*, rtx_def const*, char const*, int,

> char const*)

> /export/gnu/import/git/gitlab/x86-gcc/gcc/rtl-error.c:108

> 0x10ad88e _fatal_insn_not_found(rtx_def const*, char const*, int, char

> const*)

> /export/gnu/import/git/gitlab/x86-gcc/gcc/rtl-error.c:116

> 0x1042abb extract_insn(rtx_insn*)

> /export/gnu/import/git/gitlab/x86-gcc/gcc/recog.c:2310

> 0xc95912 instantiate_virtual_regs_in_insn

> /export/gnu/import/git/gitlab/x86-gcc/gcc/function.c:1654

> 0xc96d44 instantiate_virtual_regs

> /export/gnu/import/git/gitlab/x86-gcc/gcc/function.c:1975

> 0xc96e0e execute

> /export/gnu/import/git/gitlab/x86-gcc/gcc/function.c:2024

> Please submit a full bug report,

> with preprocessed source if appropriate.

> Please include the complete backtrace with any bug report.

> See <https://gcc.gnu.org/bugs/> for instructions.

> [hjl@gnu-cfl-2 gcc]$

>

> ;;

> ;; Full RTL generated for this function:

> ;;

> (note 1 0 3 NOTE_INSN_DELETED)

> ;; basic block 2, loop depth 0, maybe hot

> ;;  prev block 0, next block 1, flags: (NEW, REACHABLE, RTL)

> ;;  pred:       ENTRY (FALLTHRU)

> (note 3 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)

> (note 2 3 5 2 NOTE_INSN_FUNCTION_BEG)

> (insn 5 2 6 2 (unspec_volatile [

>             (const_int 0 [0])

>         ] UNSPECV_EMMS) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 6 5 7 2 (clobber (reg:XF 8 st)) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 7 6 8 2 (clobber (reg:XF 9 st(1))) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 8 7 9 2 (clobber (reg:XF 10 st(2))) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 9 8 10 2 (clobber (reg:XF 11 st(3))) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 10 9 11 2 (clobber (reg:XF 12 st(4))) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 11 10 12 2 (clobber (reg:XF 13 st(5))) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 12 11 13 2 (clobber (reg:XF 14 st(6))) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 13 12 14 2 (clobber (reg:XF 15 st(7))) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 14 13 15 2 (clobber (reg:DI 28 mm0)) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 15 14 16 2 (clobber (reg:DI 29 mm1)) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 16 15 17 2 (clobber (reg:DI 30 mm2)) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 17 16 18 2 (clobber (reg:DI 31 mm3)) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 18 17 19 2 (clobber (reg:DI 32 mm4)) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 19 18 20 2 (clobber (reg:DI 33 mm5)) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 20 19 21 2 (clobber (reg:DI 34 mm6)) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 21 20 25 2 (clobber (reg:DI 35 mm7)) "./include/mmintrin.h":60:3 -1

>      (nil))

> (insn 25 21 0 2 (const_int 0 [0]) "./include/mmintrin.h":61:1 -1

>      (nil))

> ;;  succ:       EXIT [always]  (FALLTHRU)

>

>

>

> --

> H.J.

>

Patch

diff --git a/gcc/config/i386/mmx.md b/gcc/config/i386/mmx.md
index 9cf0251293a..0f925c0b1ea 100644
--- a/gcc/config/i386/mmx.md
+++ b/gcc/config/i386/mmx.md
@@ -1848,7 +1848,34 @@ 
   [(UNSPECV_EMMS "emms")
    (UNSPECV_FEMMS "femms")])
 
-(define_insn "mmx_<emms>"
+(define_expand "mmx_<emms>"
+  [(unspec_volatile [(const_int 0)] EMMS)
+   (clobber (reg:XF ST0_REG))
+   (clobber (reg:XF ST1_REG))
+   (clobber (reg:XF ST2_REG))
+   (clobber (reg:XF ST3_REG))
+   (clobber (reg:XF ST4_REG))
+   (clobber (reg:XF ST5_REG))
+   (clobber (reg:XF ST6_REG))
+   (clobber (reg:XF ST7_REG))
+   (clobber (reg:DI MM0_REG))
+   (clobber (reg:DI MM1_REG))
+   (clobber (reg:DI MM2_REG))
+   (clobber (reg:DI MM3_REG))
+   (clobber (reg:DI MM4_REG))
+   (clobber (reg:DI MM5_REG))
+   (clobber (reg:DI MM6_REG))
+   (clobber (reg:DI MM7_REG))]
+  "TARGET_MMX || TARGET_MMX_WITH_SSE"
+{
+   if (TARGET_MMX)
+     emit_insn (gen_mmx_<emms>_1 ());
+   else
+     emit_insn (gen_nop ());
+   DONE;
+})
+
+(define_insn "mmx_<emms>_1"
   [(unspec_volatile [(const_int 0)] EMMS)
    (clobber (reg:XF ST0_REG))
    (clobber (reg:XF ST1_REG))