On Tue, Feb 12, 2019 at 10:02 AM Uros Bizjak <ubiz...@gmail.com> wrote:
>
> On Fri, Feb 8, 2019 at 12:29 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> >
> > On Fri, Feb 8, 2019 at 1:51 AM Uros Bizjak <ubiz...@gmail.com> wrote:
> > >
> > > On Thu, Feb 7, 2019 at 10:11 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> > > >
> > > > OImode and TImode moves must be done in XImode to access upper 16
> > > > vector registers without AVX512VL.  With AVX512VL, we can access
> > > > upper 16 vector registers in OImode and TImode.
> > > >
> > > >         PR target/89229
> > > >         * config/i386/i386.md (*movoi_internal_avx): Set mode to XI for
> > > >         upper 16 vector registers without TARGET_AVX512VL.
> > > >         (*movti_internal): Likewise.
> > >
> > > Please use (not (match_test "...")) instead of (match_test "!...") and
> > > put the new test as the first argument of the AND rtx.
> > >
> > > LGTM with the above change.
> >
> > This is the patch I am checking in.
>
> HJ,
>
> please revert two PR89229 patches as they introduce a regression.
>

This is what I checked in.

-- 
H.J.
From 8b572f6aae417645bb8caabc05d761474155d406 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Fri, 8 Feb 2019 16:20:49 -0800
Subject: [PATCH] i386: Revert revision 268678 and revision 268657

i386 backend has

INT_MODE (OI, 32);
INT_MODE (XI, 64);

So, XI_MODE represents 64 INTEGER bytes = 64 * 8 = 512 bit operation,
in case of const_1, all 512 bits set.

We can load zeros with narrower instruction, (e.g. 256 bit by inherent
zeroing of highpart in case of 128 bit xor), so TImode in this case.

Some targets prefer V4SF mode, so they will emit float xorps for zeroing

Then the introduction of AVX512F fubared everything by overloading the
meaning of insn mode.

How should we use INSN mode,  MODE_XI, in standard_sse_constant_opcode
and patterns which use standard_sse_constant_opcode? 2 options:

1.  MODE_XI should only used to check if EXT_REX_SSE_REG_P is true
in any register operand.  The operand size must be determined by operand
itself , not by MODE_XI.  The operand encoding size should be determined
by the operand size, EXT_REX_SSE_REG_P and AVX512VL.
2. MODE_XI should be used to determine the operand encoding size.
EXT_REX_SSE_REG_P and AVX512VL should be checked for encoding
instructions.

gcc/

	PR target/89229
	* config/i386/i386.md (*movoi_internal_avx): Revert revision
	268678 and revision 268657.
	(*movti_internal): Likewise.

gcc/testsuite/

	PR target/89229
	* gcc.target/i386/pr89229-1.c: New test.
---
 gcc/config/i386/i386.md                   | 14 +++----
 gcc/testsuite/gcc.target/i386/pr89229-1.c | 47 +++++++++++++++++++++++
 2 files changed, 53 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr89229-1.c

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 3d9141ae450..9948f77fca5 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -1933,13 +1933,12 @@
    (set_attr "type" "sselog1,sselog1,ssemov,ssemov")
    (set_attr "prefix" "vex")
    (set (attr "mode")
-	(cond [(and (not (match_test "TARGET_AVX512VL"))
-		    (ior (match_operand 0 "ext_sse_reg_operand")
-			 (match_operand 1 "ext_sse_reg_operand")))
+	(cond [(ior (match_operand 0 "ext_sse_reg_operand")
+		    (match_operand 1 "ext_sse_reg_operand"))
 		 (const_string "XI")
 	       (and (eq_attr "alternative" "1")
 		    (match_test "TARGET_AVX512VL"))
-		 (const_string "OI")
+		 (const_string "XI")
 	       (ior (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL")
 		    (and (eq_attr "alternative" "3")
 			 (match_test "TARGET_SSE_TYPELESS_STORES")))
@@ -2013,13 +2012,12 @@
    (set (attr "mode")
 	(cond [(eq_attr "alternative" "0,1")
 		 (const_string "DI")
-	       (and (not (match_test "TARGET_AVX512VL"))
-		    (ior (match_operand 0 "ext_sse_reg_operand")
-			 (match_operand 1 "ext_sse_reg_operand")))
+	       (ior (match_operand 0 "ext_sse_reg_operand")
+		    (match_operand 1 "ext_sse_reg_operand"))
 		 (const_string "XI")
 	       (and (eq_attr "alternative" "3")
 		    (match_test "TARGET_AVX512VL"))
-		 (const_string "TI")
+		 (const_string "XI")
 	       (ior (not (match_test "TARGET_SSE2"))
 		    (ior (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL")
 			 (and (eq_attr "alternative" "5")
diff --git a/gcc/testsuite/gcc.target/i386/pr89229-1.c b/gcc/testsuite/gcc.target/i386/pr89229-1.c
new file mode 100644
index 00000000000..cce95350bf2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr89229-1.c
@@ -0,0 +1,47 @@
+/* { dg-do assemble { target { avx512bw && avx512vl } } } */
+/* { dg-options "-O1 -mavx512bw -mavx512vl -mtune=skylake-avx512" } */
+
+extern void abort (void);
+extern void exit (int);
+struct s { unsigned char a[256]; };
+union u { struct { struct s b; int c; } d; struct { int c; struct s b; } e; };
+static union u v;
+static union u v0;
+static struct s *p = &v.d.b;
+static struct s *q = &v.e.b;
+
+static inline struct s rp (void) { return *p; }
+static inline struct s rq (void) { return *q; }
+static void pq (void) { *p = rq(); }
+static void qp (void) { *q = rp(); }
+
+static void
+init (struct s *sp)
+{
+  int i;
+  for (i = 0; i < 256; i++)
+    sp->a[i] = i;
+}
+
+static void
+check (struct s *sp)
+{
+  int i;
+  for (i = 0; i < 256; i++)
+    if (sp->a[i] != i)
+      abort ();
+}
+
+void
+main_test (void)
+{
+  v = v0;
+  init (p);
+  qp ();
+  check (q);
+  v = v0;
+  init (q);
+  pq ();
+  check (p);
+  exit (0);
+}
-- 
2.20.1

Reply via email to