[v2] Extend subst to simplify CONST_INT inside SIGN_EXTEND.

Message ID 20191010221636.22199-1-jimw@sifive.com
State New
Headers show
Series
  • [v2] Extend subst to simplify CONST_INT inside SIGN_EXTEND.
Related show

Commit Message

Jim Wilson Oct. 10, 2019, 10:16 p.m.
This addresses PR 91860 which has four testcases triggering internal errors.
The problem here is that in combine when handling debug insns, we are trying
to substitute
(sign_extend:DI (const_int 8160 [0x1fe0]))
as the value for
(reg:DI 78 [ _9 ])
in the debug insn
(debug_insn 29 28 30 2 (var_location:QI d (subreg:QI (reg:DI 78 [ _9 ]) 0)) "tmp4.c":11:5 -1
     (nil))
This eventually triggers an abort because 8160 is not a sign-extended
QImode value.

We should avoid creating the invalid RTL in the first place.  In subst there
is already code to avoid putting a CONST_INT inside a ZERO_EXTEND.  This
needs to be extended to also handle a SIGN_EXTEND the same way.

This was tested with rv32/newlib and rv64/linux cross builds and make checks.
There were no regressions.  The new tests all fail for rv64 without the patch,
and work with the patch.

OK?

Jim

	gcc/
	PR rtl-optimization/91860
	* combine.c (subst): If new_rtx is a constant, also check for
	SIGN_EXTEND when deciding whether to call simplify_unary_operation.

	gcc/testsuite/
	PR rtl-optimization/91860
	* gcc.dg/pr91860-1.c: New testcase.
	* gcc.dg/pr91860-2.c: New testcase.
	* gcc.dg/pr91860-3.c: New testcase.
	* gcc.dg/pr91860-4.c: New testcase.
---
 gcc/combine.c                    |  1 +
 gcc/testsuite/gcc.dg/pr91860-1.c | 18 ++++++++++++++++++
 gcc/testsuite/gcc.dg/pr91860-2.c | 13 +++++++++++++
 gcc/testsuite/gcc.dg/pr91860-3.c | 15 +++++++++++++++
 gcc/testsuite/gcc.dg/pr91860-4.c | 24 ++++++++++++++++++++++++
 5 files changed, 71 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/pr91860-1.c
 create mode 100644 gcc/testsuite/gcc.dg/pr91860-2.c
 create mode 100644 gcc/testsuite/gcc.dg/pr91860-3.c
 create mode 100644 gcc/testsuite/gcc.dg/pr91860-4.c

-- 
2.17.1

Comments

Segher Boessenkool Oct. 11, 2019, 9:02 a.m. | #1
Hi!

On Thu, Oct 10, 2019 at 03:16:36PM -0700, Jim Wilson wrote:
> This addresses PR 91860 which has four testcases triggering internal errors.

> The problem here is that in combine when handling debug insns, we are trying

> to substitute

> (sign_extend:DI (const_int 8160 [0x1fe0]))

> as the value for

> (reg:DI 78 [ _9 ])

> in the debug insn

> (debug_insn 29 28 30 2 (var_location:QI d (subreg:QI (reg:DI 78 [ _9 ]) 0)) "tmp4.c":11:5 -1

>      (nil))

> This eventually triggers an abort because 8160 is not a sign-extended

> QImode value.

> 

> We should avoid creating the invalid RTL in the first place.


It is *normal* for combine to create invalid RTL.  It first creates it,
and it checks if it is valid later.

However:

> In subst there

> is already code to avoid putting a CONST_INT inside a ZERO_EXTEND.  This

> needs to be extended to also handle a SIGN_EXTEND the same way.


That works, sure.  Approved for trunk.  Thanks!


Segher

Patch

diff --git a/gcc/combine.c b/gcc/combine.c
index d295a81abf9..92e4e5e6898 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -5680,6 +5680,7 @@  subst (rtx x, rtx from, rtx to, int in_dest, int in_cond, int unique_copy)
 		}
 	      else if (CONST_SCALAR_INT_P (new_rtx)
 		       && (GET_CODE (x) == ZERO_EXTEND
+			   || GET_CODE (x) == SIGN_EXTEND
 			   || GET_CODE (x) == FLOAT
 			   || GET_CODE (x) == UNSIGNED_FLOAT))
 		{
diff --git a/gcc/testsuite/gcc.dg/pr91860-1.c b/gcc/testsuite/gcc.dg/pr91860-1.c
new file mode 100644
index 00000000000..e715040e33d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr91860-1.c
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Og -fipa-cp -g --param=max-combine-insns=3" } */
+
+char a;
+int b;
+
+static void
+bar (short d)
+{
+  d <<= __builtin_sub_overflow (0, d, &a);
+  b = __builtin_bswap16 (~d);
+}
+
+void
+foo (void)
+{
+  bar (21043);
+}
diff --git a/gcc/testsuite/gcc.dg/pr91860-2.c b/gcc/testsuite/gcc.dg/pr91860-2.c
new file mode 100644
index 00000000000..7b44e648ca6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr91860-2.c
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Og -fexpensive-optimizations -fno-tree-fre -g --param=max-combine-insns=4" } */
+
+unsigned a, b, c;
+void
+foo (void)
+{
+  unsigned short e;
+  __builtin_mul_overflow (0, b, &a);
+  __builtin_sub_overflow (59347, 9, &e);
+  e <<= a & 5;
+  c = e;
+}
diff --git a/gcc/testsuite/gcc.dg/pr91860-3.c b/gcc/testsuite/gcc.dg/pr91860-3.c
new file mode 100644
index 00000000000..2b488cc9048
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr91860-3.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Og -g2 --param=max-combine-insns=3" } */
+
+int a, b;
+
+void
+foo (void)
+{
+  unsigned short d = 46067;
+  int e = e;
+  d <<= __builtin_mul_overflow (~0, e, &a);
+  d |= -68719476735;
+  b = d;
+}
+
diff --git a/gcc/testsuite/gcc.dg/pr91860-4.c b/gcc/testsuite/gcc.dg/pr91860-4.c
new file mode 100644
index 00000000000..36f2bd55c64
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr91860-4.c
@@ -0,0 +1,24 @@ 
+/* { dg-do compile } */
+/* { dg-require-effective-target int128 } */
+/* { dg-options "-O2 -g" } */
+
+typedef unsigned char u8;
+typedef unsigned int u32;
+typedef unsigned __int128 u128;
+
+u32 b, c;
+
+static inline
+u128 bar (u8 d, u128 e)
+{
+  __builtin_memset (11 + (char *) &e, b, 1);
+  d <<= e & 7;
+  d = d | d > 0;
+  return d + e;
+}
+
+void
+foo (void)
+{
+  c = bar (~0, 5);
+}