[PATCH] i386: Use EXT_REX_SSE_REG_P in *movoi_internal_avx/movti_internal
H.J. Lu
hjl.tools@gmail.com
Mon Feb 11 13:11:00 GMT 2019
On Sat, Feb 09, 2019 at 02:39:30PM +0100, Jakub Jelinek wrote:
> On Sat, Feb 09, 2019 at 01:22:30PM +0100, Jakub Jelinek wrote:
> > On Sat, Feb 09, 2019 at 04:11:43AM -0800, H.J. Lu wrote:
> > > I believe all usages of
> > >
> > > (ior (match_operand 0 "ext_sse_reg_operand")
> > > (match_operand 1 "ext_sse_reg_operand"))
> > >
> > > should be checked. I am not sure if they should be there at all.
> >
> > E.g. in i386.md all the other spots look fine, because {DI,SI,DF,SF}mode
> > is allowed in ext sse regs even with -mavx512f. And sse.md doesn't use this
> > at all. What I'm wondering is if we need the sse.md (*mov<mode>_internal)
> > code I've cited earlier, doing bootstrap/regtest now with gcc_unreachable in
> > there (and in *mov{o,x}i_internal* for MODE_XI too) too see if it ever
> > triggers.
>
> The following didn't ICE on anything, which is not a proof, but given that
> hard_regno_mode_ok should return false for ext_sse_reg_operand regs for
> avx512f && !avx512vl, it matches my expectations, on the other hand, it was
> a normal defaults bootstrap, don't have a knl which might be best for this
> to test -mavx512f -mno-avx512vl on everything.
> So perhaps we can also nuke the large if from mov<mode>_internal.
>
> --- gcc/config/i386/i386.md.jj 2019-02-09 12:35:57.971475641 +0100
> +++ gcc/config/i386/i386.md 2019-02-09 12:37:40.776802962 +0100
> @@ -1905,6 +1905,7 @@ (define_insn "*movoi_internal_avx"
> return standard_sse_constant_opcode (insn, operands);
>
> case TYPE_SSEMOV:
> + gcc_assert (get_attr_mode (insn) != MODE_XI);
> if (misaligned_operand (operands[0], OImode)
> || misaligned_operand (operands[1], OImode))
> {
> @@ -1970,6 +1971,7 @@ (define_insn "*movti_internal"
> case TYPE_SSEMOV:
> /* TDmode values are passed as TImode on the stack. Moving them
> to stack may result in unaligned memory access. */
> + gcc_assert (get_attr_mode (insn) != MODE_XI);
> if (misaligned_operand (operands[0], TImode)
> || misaligned_operand (operands[1], TImode))
> {
> --- gcc/config/i386/sse.md.jj 2019-01-28 21:57:39.301110220 +0100
> +++ gcc/config/i386/sse.md 2019-02-09 12:36:45.863696416 +0100
> @@ -989,6 +989,7 @@ (define_insn "mov<mode>_internal"
> && (EXT_REX_SSE_REG_P (operands[0])
> || EXT_REX_SSE_REG_P (operands[1])))
> {
> + gcc_unreachable ();
> if (memory_operand (operands[0], <MODE>mode))
> {
> if (<MODE_SIZE> == 32)
>
Here is the updated patch to remove ext_sse_reg_operand check with a
testcase.
OK for trunk?
Thanks.
H.J.
---
Since hard_regno_mode_ok only allows xmm16-xmm31 in OImode or TImode
with TARGET_AVX512VL:
/* AVX512VL allows sse regs16+ for 128/256 bit modes. */
if (TARGET_AVX512VL
&& (mode == OImode
|| mode == TImode
|| VALID_AVX256_REG_MODE (mode)
|| VALID_AVX512VL_128_REG_MODE (mode)))
return true;
/* xmm16-xmm31 are only available for AVX-512. */
if (EXT_REX_SSE_REGNO_P (regno))
return false;
there is no need to check ext_sse_reg_operand in *movoi_internal_avx nor
*movti_internal. Instead, we should check EXT_REX_SSE_REG_P for upper 16
vector registers.
2019-02-11 H.J. Lu <hongjiu.lu@intel.com>
Jakub Jelinek <jakub@redhat.com>
gcc/
PR target/89229
* config/i386/i386.md (*movoi_internal_avx): Check
EXT_REX_SSE_REG_P instead of MODE_XI for upper 16 vector
registers.
(*movti_internal): Likewise.
gcc/testsuite/
PR target/89229
* gcc.target/i386/pr89229-1.c: New test.
---
gcc/config/i386/i386.md | 22 +++++------
gcc/testsuite/gcc.target/i386/pr89229-1.c | 47 +++++++++++++++++++++++
2 files changed, 56 insertions(+), 13 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..5b89e52493e 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -1910,7 +1910,8 @@
{
if (get_attr_mode (insn) == MODE_V8SF)
return "vmovups\t{%1, %0|%0, %1}";
- else if (get_attr_mode (insn) == MODE_XI)
+ else if (EXT_REX_SSE_REG_P (operands[0])
+ || EXT_REX_SSE_REG_P (operands[1]))
return "vmovdqu32\t{%1, %0|%0, %1}";
else
return "vmovdqu\t{%1, %0|%0, %1}";
@@ -1919,7 +1920,8 @@
{
if (get_attr_mode (insn) == MODE_V8SF)
return "vmovaps\t{%1, %0|%0, %1}";
- else if (get_attr_mode (insn) == MODE_XI)
+ else if (EXT_REX_SSE_REG_P (operands[0])
+ || EXT_REX_SSE_REG_P (operands[1]))
return "vmovdqa32\t{%1, %0|%0, %1}";
else
return "vmovdqa\t{%1, %0|%0, %1}";
@@ -1933,11 +1935,7 @@
(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")))
- (const_string "XI")
- (and (eq_attr "alternative" "1")
+ (cond [(and (eq_attr "alternative" "1")
(match_test "TARGET_AVX512VL"))
(const_string "OI")
(ior (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL")
@@ -1973,7 +1971,8 @@
{
if (get_attr_mode (insn) == MODE_V4SF)
return "%vmovups\t{%1, %0|%0, %1}";
- else if (get_attr_mode (insn) == MODE_XI)
+ else if (EXT_REX_SSE_REG_P (operands[0])
+ || EXT_REX_SSE_REG_P (operands[1]))
return "vmovdqu32\t{%1, %0|%0, %1}";
else
return "%vmovdqu\t{%1, %0|%0, %1}";
@@ -1982,7 +1981,8 @@
{
if (get_attr_mode (insn) == MODE_V4SF)
return "%vmovaps\t{%1, %0|%0, %1}";
- else if (get_attr_mode (insn) == MODE_XI)
+ else if (EXT_REX_SSE_REG_P (operands[0])
+ || EXT_REX_SSE_REG_P (operands[1]))
return "vmovdqa32\t{%1, %0|%0, %1}";
else
return "%vmovdqa\t{%1, %0|%0, %1}";
@@ -2013,10 +2013,6 @@
(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")))
- (const_string "XI")
(and (eq_attr "alternative" "3")
(match_test "TARGET_AVX512VL"))
(const_string "TI")
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
More information about the Gcc-patches
mailing list