Hi All,
This and the report in PR103632 are caused by a bug in REE where it generates
incorrect code.
It's trying to eliminate the following zero extension
(insn 54 90 102 2 (set (reg:V4SI 33 v1)
(zero_extend:V4SI (reg/v:V4HI 40 v8)))
(nil))
by folding it in the definition of `v8`:
(insn 2 5 104 2 (set (reg/v:V4HI 40 v8)
(reg:V4HI 32 v0 [156]))
(nil))
which is fine, except that `v8` is also used by the extracts, e.g.:
(insn 11 10 12 2 (set (reg:SI 1 x1)
(zero_extend:SI (vec_select:HI (reg/v:V4HI 40 v8)
(parallel [
(const_int 3)
]))))
(nil))
REE replaces insn 2 by folding insn 54 and placing it at the definition site of
insn 2, so before insn 11.
Trying to eliminate extension:
(insn 54 90 102 2 (set (reg:V4SI 33 v1)
(zero_extend:V4SI (reg/v:V4HI 40 v8)))
(nil))
Tentatively merged extension with definition (copy needed):
(insn 2 5 104 2 (set (reg:V4SI 33 v1)
(zero_extend:V4SI (reg:V4HI 32 v0)))
(nil))
to produce
(insn 2 5 110 2 (set (reg:V4SI 33 v1)
(zero_extend:V4SI (reg:V4HI 32 v0)))
(nil))
(insn 110 2 104 2 (set (reg:V4SI 40 v8)
(reg:V4SI 33 v1))
(nil))
The new insn 2 using v0 directly is correct, but the insn 110 it creates is
wrong, `v8` should still be V4HI.
or it also needs to eliminate the zero extension from the extracts, so instead
of
(insn 11 10 12 2 (set (reg:SI 1 x1)
(zero_extend:SI (vec_select:HI (reg/v:V4HI 40 v8)
(parallel [
(const_int 3)
]))))
(nil))
it should be
(insn 11 10 12 2 (set (reg:SI 1 x1)
(vec_select:SI (reg/v:V4SI 40 v8)
(parallel [
(const_int 3)
])))
(nil))
without doing so the indices have been remapped in the extension and so we
extract the wrong elements
At any other optimization level but -Os ree seems to abort so this doesn't
trigger:
Trying to eliminate extension:
(insn 54 90 101 2 (set (reg:V4SI 32 v0)
(zero_extend:V4SI (reg/v:V4HI 40 v8)))
(nil))
Elimination opportunities = 2 realized = 0
purely due to the ordering of instructions. REE doesn't check uses of `v8`
because it assumes that with a zero extended value, you still have access to the
lower bits by using the the bottom part of the register.
This is true for scalar but not for vector. This would have been fine as well
if REE had eliminated the zero_extend on insn 11 and the rest but it doesn't do
so since REE can only handle cases where the SRC value are REG_P.
It does try to do this in add_removable_extension:
1160 /* For vector mode extensions, ensure that all uses of the
1161 XEXP (src, 0) register are in insn or debug insns, as unlike
1162 integral extensions lowpart subreg of the sign/zero extended
1163 register are not equal to the original register, so we have
1164 to change all uses or none and the current code isn't able
1165 to change them all at once in one transaction. */
However this code doesn't trigger for the example because REE doesn't check the
uses if the defining instruction doesn't feed into another extension..
Which is bogus. For vectors it should always check all usages.
r12-2288-g8695bf78dad1a42636775843ca832a2f4dba4da3 simply exposed this as it now
lowers VEC_SELECT 0 into the RTL canonical form subreg 0 which causes REE to run
more often.
Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
Ok for master?
Thanks,
Tamar
gcc/ChangeLog:
PR rtl-optimization/103350
* ree.c (add_removable_extension): Don't stop at first definition but
inspect all.
gcc/testsuite/ChangeLog:
PR rtl-optimization/103350
* gcc.target/aarch64/pr103350-1.c: New test.
* gcc.target/aarch64/pr103350-2.c: New test.
--- inline copy of patch --
diff --git a/gcc/ree.c b/gcc/ree.c
index
e31ca2fa1a8073c09b054602c2fa19cfe0cb23c4..13debe8a4af1e8abe666d88b6694163172894030
100644
--- a/gcc/ree.c
+++ b/gcc/ree.c
@@ -1165,31 +1165,28 @@ add_removable_extension (const_rtx expr, rtx_insn *insn,
to change them all at once in one transaction. */
else if (VECTOR_MODE_P (GET_MODE (XEXP (src, 0))))
{
- if (idx == 0)
- {
- struct df_link *ref_chain, *ref_link;
+ struct df_link *ref_chain, *ref_link;
- ref_chain = DF_REF_CHAIN (def->ref);
- for (ref_link = ref_chain; ref_link; ref_link = ref_link->next)
+ ref_chain = DF_REF_CHAIN (def->ref);
+ for (ref_link = ref_chain; ref_link; ref_link = ref_link->next)
+ {
+ if (ref_link->ref == NULL
+ || DF_REF_INSN_INFO (ref_link->ref) == NULL)
{
- if (ref_link->ref == NULL
- || DF_REF_INSN_INFO (ref_link->ref) == NULL)
- {
- idx = -1U;
- break;
- }
- rtx_insn *use_insn = DF_REF_INSN (ref_link->ref);
- if (use_insn != insn && !DEBUG_INSN_P (use_insn))
- {
- idx = -1U;
- break;
- }
+ idx = -1U;
+ break;
+ }
+ rtx_insn *use_insn = DF_REF_INSN (ref_link->ref);
+ if (use_insn != insn && !DEBUG_INSN_P (use_insn))
+ {
+ idx = -1U;
+ break;
}
- if (idx == -1U)
- def_map[INSN_UID (DF_REF_INSN (def->ref))] = idx;
}
+
if (idx == -1U)
{
+ def_map[INSN_UID (DF_REF_INSN (def->ref))] = idx;
if (dump_file)
{
fprintf (dump_file, "Cannot eliminate extension:\n");
diff --git a/gcc/testsuite/gcc.target/aarch64/pr103350-1.c
b/gcc/testsuite/gcc.target/aarch64/pr103350-1.c
new file mode 100644
index
0000000000000000000000000000000000000000..61c796dc6e8e3733a9446c89189135b83ecb4f3e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr103350-1.c
@@ -0,0 +1,48 @@
+/* { dg-do run } */
+/* { dg-additional-options "-Os -fno-tree-ter -save-temps -fdump-rtl-ree-all
-free -std=c99 -w" } */
+
+typedef unsigned char u8;
+typedef unsigned char __attribute__((__vector_size__ (8))) v64u8;
+typedef unsigned char __attribute__((__vector_size__ (16))) v128u8;
+typedef unsigned char __attribute__((__vector_size__ (32))) v256u8;
+typedef unsigned short __attribute__((__vector_size__ (8))) v64u16;
+typedef unsigned int __attribute__((__vector_size__ (16))) v128u32;
+typedef unsigned long long u64;
+typedef unsigned __int128 u128;
+
+v64u16 foo0_v32u16_0 = { 4, 5, 6, 7 };
+u64 foo0_u64_0 = 0x30;
+
+__attribute__((__noipa__))
+v64u8 foo0 (v64u16 v64u16_0, u128 u128_0)
+{
+ /* 03 00 05 00 03 00 02 00 ... */
+ v256u8 v256u16_1 = (v256u8)__builtin_shufflevector (v64u16_0, foo0_v32u16_0,
+ 3, 5, 3, 2, 3, 5, 1, 0,
+ 3, 5, 3, 2, 2, 0, 2, 0);
+ /* 00 00 00 00 01 00 00 00 ... */
+ v128u8 v128u8_1 = (v128u8) __builtin_convertvector (v64u16_0, v128u32);
+ /* 10 */
+ u8 u8_1 = foo0_u64_0 % u128_0;
+ /* 03 00 05 00 04 00 02 00 ... */
+ v128u8 v128u8_r = ((union {v256u8 a; v128u8 b[2];}) v256u16_1).b[0] +
v128u8_1;
+ /* 00 00 01 00 02 00 03 00 */
+ v64u8 v64u8_0 = (v64u8)v64u16_0;
+ /* 03 00 06 00 06 00 05 00 */
+ v64u8 v64u8_r = ((union {v128u8 a; v64u8 b[2];}) v128u8_r).b[0] + v64u8_0;
+ /* 13 10 16 10 16 10 15 10 */
+ return v64u8_r + u8_1;
+}
+
+int
+main (void)
+{
+ v64u8 x = foo0 ((v64u16){ 0, 1, 2, 3 }, 0x20);
+ v64u8 exp = { 0x13, 0x10, 0x16, 0x10, 0x16, 0x10, 0x15, 0x10 };
+ for (unsigned i = 0; i < sizeof(x); i++)
+ if (x[i] != exp[i])
+ __builtin_abort();
+ return 0;
+}
+
+/* { dg-final { scan-rtl-dump {because some vector uses aren't extension} ree
} } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr103350-2.c
b/gcc/testsuite/gcc.target/aarch64/pr103350-2.c
new file mode 100644
index
0000000000000000000000000000000000000000..2696212710b9da1c065cd26d26dd9f09719cf9e9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr103350-2.c
@@ -0,0 +1,53 @@
+/* { dg-do run } */
+/* { dg-additional-options "-O2 -save-temps -fdump-rtl-ree-all -free -std=c99
-w" } */
+
+typedef unsigned char __attribute__((__vector_size__ (8))) v64u8;
+typedef unsigned char __attribute__((__vector_size__ (16))) v128u8;
+typedef unsigned short __attribute__((__vector_size__ (8))) v64u16;
+typedef unsigned short __attribute__((__vector_size__ (64))) v512u16;
+typedef unsigned int __attribute__((__vector_size__ (16))) v128u32;
+typedef unsigned long long u64;
+typedef unsigned long long __attribute__((__vector_size__ (8))) v64u64;
+typedef unsigned long long __attribute__((__vector_size__ (16))) v128u64;
+typedef unsigned __int128 u128;
+typedef unsigned __int128 __attribute__((__vector_size__ (64))) v512u128;
+v512u16 foo0_v512u16_0;
+u64 foo0_u64_0;
+u64 foo0_u16_0;
+
+void
+foo0 (v64u16 v64u16_0, v64u64 v64u64_0, u128 u128_0, v64u8 * ret)
+{
+ /* { 0, 4, 0, 0 } */
+ v128u32 v128u32_2 = __builtin_convertvector (v64u16_0, v128u32);
+ /* 0 */
+ foo0_u16_0 ^= foo0_u64_0 % u128_0;
+ /* { 0, ... } */
+ foo0_v512u16_0 *=
+ __builtin_shufflevector (v64u16_0, v64u16_0, 7, 7, 2, 1, 0, 0, 3, 6, 2, 3,
+ 1, 0, 7, 5, 6, 7, 4, 3, 2, 3, 0, 6, 1, 2, 3, 3,
+ 6, 7, 6, 2, 4, 3);
+ /* { 0, 0, 0, 0, 4, 0, ... } */
+ v128u8 v128u8_r = (v128u8) ((v512u128) foo0_v512u16_0)[0] +
+ (v128u8) v128u32_2;
+ /* { 0, 0, 4, 0, 4, 0, 0, 0 } */
+ v64u8 v64u8_r = (v64u8) ((v128u64) v128u8_r)[0] +
+ (v64u8) v64u16_0 + (v64u8) v64u64_0;
+ *ret = v64u8_r;
+}
+
+int
+main (void)
+{
+ v64u8 x, exp = (v64u8){ 0, 0, 4, 0, 4, 0, 0, 0 };
+ foo0 ((v64u16){0, 4}, (v64u64){}, 5, &x);
+ /*
+ for (unsigned i = 0; i < sizeof (x); i++)
+ __builtin_printf ("%02x", x[i]);
+ */
+ for (unsigned i = 0; i < sizeof (x); i++)
+ if (x[i] != exp[i]) __builtin_abort();
+ return 0;
+}
+
+/* { dg-final { scan-rtl-dump {because some vector uses aren't extension} ree
} } */
--
diff --git a/gcc/ree.c b/gcc/ree.c
index e31ca2fa1a8073c09b054602c2fa19cfe0cb23c4..13debe8a4af1e8abe666d88b6694163172894030 100644
--- a/gcc/ree.c
+++ b/gcc/ree.c
@@ -1165,31 +1165,28 @@ add_removable_extension (const_rtx expr, rtx_insn *insn,
to change them all at once in one transaction. */
else if (VECTOR_MODE_P (GET_MODE (XEXP (src, 0))))
{
- if (idx == 0)
- {
- struct df_link *ref_chain, *ref_link;
+ struct df_link *ref_chain, *ref_link;
- ref_chain = DF_REF_CHAIN (def->ref);
- for (ref_link = ref_chain; ref_link; ref_link = ref_link->next)
+ ref_chain = DF_REF_CHAIN (def->ref);
+ for (ref_link = ref_chain; ref_link; ref_link = ref_link->next)
+ {
+ if (ref_link->ref == NULL
+ || DF_REF_INSN_INFO (ref_link->ref) == NULL)
{
- if (ref_link->ref == NULL
- || DF_REF_INSN_INFO (ref_link->ref) == NULL)
- {
- idx = -1U;
- break;
- }
- rtx_insn *use_insn = DF_REF_INSN (ref_link->ref);
- if (use_insn != insn && !DEBUG_INSN_P (use_insn))
- {
- idx = -1U;
- break;
- }
+ idx = -1U;
+ break;
+ }
+ rtx_insn *use_insn = DF_REF_INSN (ref_link->ref);
+ if (use_insn != insn && !DEBUG_INSN_P (use_insn))
+ {
+ idx = -1U;
+ break;
}
- if (idx == -1U)
- def_map[INSN_UID (DF_REF_INSN (def->ref))] = idx;
}
+
if (idx == -1U)
{
+ def_map[INSN_UID (DF_REF_INSN (def->ref))] = idx;
if (dump_file)
{
fprintf (dump_file, "Cannot eliminate extension:\n");
diff --git a/gcc/testsuite/gcc.target/aarch64/pr103350-1.c b/gcc/testsuite/gcc.target/aarch64/pr103350-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..61c796dc6e8e3733a9446c89189135b83ecb4f3e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr103350-1.c
@@ -0,0 +1,48 @@
+/* { dg-do run } */
+/* { dg-additional-options "-Os -fno-tree-ter -save-temps -fdump-rtl-ree-all -free -std=c99 -w" } */
+
+typedef unsigned char u8;
+typedef unsigned char __attribute__((__vector_size__ (8))) v64u8;
+typedef unsigned char __attribute__((__vector_size__ (16))) v128u8;
+typedef unsigned char __attribute__((__vector_size__ (32))) v256u8;
+typedef unsigned short __attribute__((__vector_size__ (8))) v64u16;
+typedef unsigned int __attribute__((__vector_size__ (16))) v128u32;
+typedef unsigned long long u64;
+typedef unsigned __int128 u128;
+
+v64u16 foo0_v32u16_0 = { 4, 5, 6, 7 };
+u64 foo0_u64_0 = 0x30;
+
+__attribute__((__noipa__))
+v64u8 foo0 (v64u16 v64u16_0, u128 u128_0)
+{
+ /* 03 00 05 00 03 00 02 00 ... */
+ v256u8 v256u16_1 = (v256u8)__builtin_shufflevector (v64u16_0, foo0_v32u16_0,
+ 3, 5, 3, 2, 3, 5, 1, 0,
+ 3, 5, 3, 2, 2, 0, 2, 0);
+ /* 00 00 00 00 01 00 00 00 ... */
+ v128u8 v128u8_1 = (v128u8) __builtin_convertvector (v64u16_0, v128u32);
+ /* 10 */
+ u8 u8_1 = foo0_u64_0 % u128_0;
+ /* 03 00 05 00 04 00 02 00 ... */
+ v128u8 v128u8_r = ((union {v256u8 a; v128u8 b[2];}) v256u16_1).b[0] + v128u8_1;
+ /* 00 00 01 00 02 00 03 00 */
+ v64u8 v64u8_0 = (v64u8)v64u16_0;
+ /* 03 00 06 00 06 00 05 00 */
+ v64u8 v64u8_r = ((union {v128u8 a; v64u8 b[2];}) v128u8_r).b[0] + v64u8_0;
+ /* 13 10 16 10 16 10 15 10 */
+ return v64u8_r + u8_1;
+}
+
+int
+main (void)
+{
+ v64u8 x = foo0 ((v64u16){ 0, 1, 2, 3 }, 0x20);
+ v64u8 exp = { 0x13, 0x10, 0x16, 0x10, 0x16, 0x10, 0x15, 0x10 };
+ for (unsigned i = 0; i < sizeof(x); i++)
+ if (x[i] != exp[i])
+ __builtin_abort();
+ return 0;
+}
+
+/* { dg-final { scan-rtl-dump {because some vector uses aren't extension} ree } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr103350-2.c b/gcc/testsuite/gcc.target/aarch64/pr103350-2.c
new file mode 100644
index 0000000000000000000000000000000000000000..2696212710b9da1c065cd26d26dd9f09719cf9e9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr103350-2.c
@@ -0,0 +1,53 @@
+/* { dg-do run } */
+/* { dg-additional-options "-O2 -save-temps -fdump-rtl-ree-all -free -std=c99 -w" } */
+
+typedef unsigned char __attribute__((__vector_size__ (8))) v64u8;
+typedef unsigned char __attribute__((__vector_size__ (16))) v128u8;
+typedef unsigned short __attribute__((__vector_size__ (8))) v64u16;
+typedef unsigned short __attribute__((__vector_size__ (64))) v512u16;
+typedef unsigned int __attribute__((__vector_size__ (16))) v128u32;
+typedef unsigned long long u64;
+typedef unsigned long long __attribute__((__vector_size__ (8))) v64u64;
+typedef unsigned long long __attribute__((__vector_size__ (16))) v128u64;
+typedef unsigned __int128 u128;
+typedef unsigned __int128 __attribute__((__vector_size__ (64))) v512u128;
+v512u16 foo0_v512u16_0;
+u64 foo0_u64_0;
+u64 foo0_u16_0;
+
+void
+foo0 (v64u16 v64u16_0, v64u64 v64u64_0, u128 u128_0, v64u8 * ret)
+{
+ /* { 0, 4, 0, 0 } */
+ v128u32 v128u32_2 = __builtin_convertvector (v64u16_0, v128u32);
+ /* 0 */
+ foo0_u16_0 ^= foo0_u64_0 % u128_0;
+ /* { 0, ... } */
+ foo0_v512u16_0 *=
+ __builtin_shufflevector (v64u16_0, v64u16_0, 7, 7, 2, 1, 0, 0, 3, 6, 2, 3,
+ 1, 0, 7, 5, 6, 7, 4, 3, 2, 3, 0, 6, 1, 2, 3, 3,
+ 6, 7, 6, 2, 4, 3);
+ /* { 0, 0, 0, 0, 4, 0, ... } */
+ v128u8 v128u8_r = (v128u8) ((v512u128) foo0_v512u16_0)[0] +
+ (v128u8) v128u32_2;
+ /* { 0, 0, 4, 0, 4, 0, 0, 0 } */
+ v64u8 v64u8_r = (v64u8) ((v128u64) v128u8_r)[0] +
+ (v64u8) v64u16_0 + (v64u8) v64u64_0;
+ *ret = v64u8_r;
+}
+
+int
+main (void)
+{
+ v64u8 x, exp = (v64u8){ 0, 0, 4, 0, 4, 0, 0, 0 };
+ foo0 ((v64u16){0, 4}, (v64u64){}, 5, &x);
+ /*
+ for (unsigned i = 0; i < sizeof (x); i++)
+ __builtin_printf ("%02x", x[i]);
+ */
+ for (unsigned i = 0; i < sizeof (x); i++)
+ if (x[i] != exp[i]) __builtin_abort();
+ return 0;
+}
+
+/* { dg-final { scan-rtl-dump {because some vector uses aren't extension} ree } } */
