diff mbox

Fix cselim ICE (PR tree-optimization/56635)

Message ID 20130318120012.GY12913@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek March 18, 2013, noon UTC 
Hi!

On the attached testcase we ICE, because cselim uses
   if (!is_gimple_reg_type (TREE_TYPE (lhs))
       || !operand_equal_p (lhs, gimple_assign_lhs (else_assign), 0))
to guard an optimization, and we have two MEM_REFs where operand_equal_p
is true, but they don't have compatible types (the first one is _Complex
double, i.e. is_gimple_reg_type), the second has aggregate type of the
same size/same mode (structure containing the _Complex double).

The first patch is what I've bootstrapped/regtested on i686-linux so far
(x86_64-linux regtest still pending), it seems to trigger different return
value from operand_equal_p in a couple of TUs:
c52008b.adb
/usr/src/gcc-4.8/libstdc++-v3/src/c++98/complex_io.cc
/usr/src/gcc-4.8/lto-plugin/lto-plugin.c
/usr/src/gcc-4.8/gcc/testsuite/g++.dg/opt/pr47366.C
/usr/src/gcc-4.8/gcc/testsuite/g++.dg/torture/pr56635.C
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/actual_array_constructor_1.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/alloc_comp_assign_2.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/alloc_comp_assign_3.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/argument_checking_1.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/array_memcpy_2.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/array_temporaries_3.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/auto_char_len_3.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/char_pointer_func.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/proc_ptr_23.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/pure_byref_1.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/realloc_on_assign_17.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/reshape.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/string_ctor_1.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/unlimited_polymorphic_1.f03
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/vector_subscript_2.f90
/usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/widechar_8.f90
/usr/src/gcc-4.8/libstdc++-v3/testsuite/23_containers/unordered_map/cons/56112.cc
/usr/src/gcc-4.8/libstdc++-v3/testsuite/25_algorithms/remove_if/moveable.cc
/usr/src/gcc-4.8/libstdc++-v3/testsuite/25_algorithms/remove/moveable.cc
/usr/src/gcc-4.8/libstdc++-v3/testsuite/25_algorithms/stable_sort/49559.cc
but from quick look at some of them it didn't look like something very
undesirable.  In any case, no new FAILs.  Ok for trunk if it passes regtest
on x86_64-linux too?

The second patch is a safer version intended for 4.8.0, to be
bootstrapped/regtested afterwards.

	Jakub
2013-03-18  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/56635
	* fold-const.c (operand_equal_p): For MEM_REF and TARGET_MEM_REF,
	require types_compatible_p types.

	* g++.dg/torture/pr56635.C: New test.
2013-03-18  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/56635
	* tree-ssa-phiopt.c (cond_if_else_store_replacement_1): Give up
	if lhs of then_assign and else_assign don't have compatible types.

	* g++.dg/torture/pr56635.C: New test.

--- gcc/tree-ssa-phiopt.c.jj	2013-02-13 21:47:17.000000000 +0100
+++ gcc/tree-ssa-phiopt.c	2013-03-18 09:54:03.047377144 +0100
@@ -1528,7 +1528,7 @@ cond_if_else_store_replacement_1 (basic_
 				  basic_block join_bb, gimple then_assign,
 				  gimple else_assign)
 {
-  tree lhs_base, lhs, then_rhs, else_rhs, name;
+  tree lhs_base, lhs, else_lhs, then_rhs, else_rhs, name;
   source_location then_locus, else_locus;
   gimple_stmt_iterator gsi;
   gimple newphi, new_stmt;
@@ -1544,8 +1544,10 @@ cond_if_else_store_replacement_1 (basic_
     return false;
 
   lhs = gimple_assign_lhs (then_assign);
+  else_lhs = gimple_assign_lhs (else_assign);
   if (!is_gimple_reg_type (TREE_TYPE (lhs))
-      || !operand_equal_p (lhs, gimple_assign_lhs (else_assign), 0))
+      || !operand_equal_p (lhs, else_lhs, 0)
+      || !types_compatible_p (TREE_TYPE (lhs), TREE_TYPE (else_lhs)))
     return false;
 
   lhs_base = get_base_address (lhs);
--- gcc/testsuite/g++.dg/torture/pr56635.C.jj	2013-03-18 09:56:28.089526657 +0100
+++ gcc/testsuite/g++.dg/torture/pr56635.C	2013-03-18 09:56:36.111478322 +0100
@@ -0,0 +1,17 @@
+// PR tree-optimization/56635
+// { dg-do compile }
+
+struct A { _Complex double a; };
+
+void
+foo (A **x, A **y)
+{
+  A r;
+  if (__real__ x[0]->a)
+    {
+      r.a = y[0]->a / x[0]->a;
+      **x = r;
+    }
+  else
+    **x = **y;
+}

Comments

Richard Biener March 18, 2013, 12:40 p.m. UTC | #1 
On Mon, Mar 18, 2013 at 1:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On the attached testcase we ICE, because cselim uses
>    if (!is_gimple_reg_type (TREE_TYPE (lhs))
>        || !operand_equal_p (lhs, gimple_assign_lhs (else_assign), 0))
> to guard an optimization, and we have two MEM_REFs where operand_equal_p
> is true, but they don't have compatible types (the first one is _Complex
> double, i.e. is_gimple_reg_type), the second has aggregate type of the
> same size/same mode (structure containing the _Complex double).
>
> The first patch is what I've bootstrapped/regtested on i686-linux so far
> (x86_64-linux regtest still pending), it seems to trigger different return
> value from operand_equal_p in a couple of TUs:
> c52008b.adb
> /usr/src/gcc-4.8/libstdc++-v3/src/c++98/complex_io.cc
> /usr/src/gcc-4.8/lto-plugin/lto-plugin.c
> /usr/src/gcc-4.8/gcc/testsuite/g++.dg/opt/pr47366.C
> /usr/src/gcc-4.8/gcc/testsuite/g++.dg/torture/pr56635.C
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/actual_array_constructor_1.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/alloc_comp_assign_2.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/alloc_comp_assign_3.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/argument_checking_1.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/array_memcpy_2.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/array_temporaries_3.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/auto_char_len_3.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/char_pointer_func.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/proc_ptr_23.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/pure_byref_1.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/realloc_on_assign_17.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/reshape.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/string_ctor_1.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/unlimited_polymorphic_1.f03
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/vector_subscript_2.f90
> /usr/src/gcc-4.8/gcc/testsuite/gfortran.dg/widechar_8.f90
> /usr/src/gcc-4.8/libstdc++-v3/testsuite/23_containers/unordered_map/cons/56112.cc
> /usr/src/gcc-4.8/libstdc++-v3/testsuite/25_algorithms/remove_if/moveable.cc
> /usr/src/gcc-4.8/libstdc++-v3/testsuite/25_algorithms/remove/moveable.cc
> /usr/src/gcc-4.8/libstdc++-v3/testsuite/25_algorithms/stable_sort/49559.cc
> but from quick look at some of them it didn't look like something very
> undesirable.  In any case, no new FAILs.  Ok for trunk if it passes regtest
> on x86_64-linux too?

Ok.

Thanks,
Richard.

> The second patch is a safer version intended for 4.8.0, to be
> bootstrapped/regtested afterwards.
>
>         Jakub
diff mbox

Patch

--- gcc/fold-const.c.jj	2013-02-26 10:59:53.000000000 +0100
+++ gcc/fold-const.c	2013-03-18 10:20:41.368932746 +0100
@@ -2572,13 +2572,14 @@  operand_equal_p (const_tree arg0, const_
 	  flags &= ~OEP_CONSTANT_ADDRESS_OF;
 	  /* Require equal access sizes, and similar pointer types.
 	     We can have incomplete types for array references of
-	     variable-sized arrays from the Fortran frontent
-	     though.  */
+	     variable-sized arrays from the Fortran frontend
+	     though.  Also verify the types are compatible.  */
 	  return ((TYPE_SIZE (TREE_TYPE (arg0)) == TYPE_SIZE (TREE_TYPE (arg1))
 		   || (TYPE_SIZE (TREE_TYPE (arg0))
 		       && TYPE_SIZE (TREE_TYPE (arg1))
 		       && operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)),
 					   TYPE_SIZE (TREE_TYPE (arg1)), flags)))
+		  && types_compatible_p (TREE_TYPE (arg0), TREE_TYPE (arg1))
 		  && (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (arg0, 1)))
 		      == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (arg1, 1))))
 		  && OP_SAME (0) && OP_SAME (1));
--- gcc/testsuite/g++.dg/torture/pr56635.C.jj	2013-03-18 09:56:28.089526657 +0100
+++ gcc/testsuite/g++.dg/torture/pr56635.C	2013-03-18 09:56:36.111478322 +0100
@@ -0,0 +1,17 @@ 
+// PR tree-optimization/56635
+// { dg-do compile }
+
+struct A { _Complex double a; };
+
+void
+foo (A **x, A **y)
+{
+  A r;
+  if (__real__ x[0]->a)
+    {
+      r.a = y[0]->a / x[0]->a;
+      **x = r;
+    }
+  else
+    **x = **y;
+}