This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix inliner INDIRECT_EXPR <ADDR_EXPR> handling (PR c/34668)


Hi!

This patch fixes an inlining issue with IMA.  &optab_table[0] is parsed
as ADDR_EXPR with struct optab * type of optab_table which has
struct optab [1] ARRAY_TYPE.  Later on IMA changes the type of optab_table
to ARRAY_TYPE of a different struct optab (from the other CU), but
the type of ADDR_EXPR stays.  When inlining creates INDIRECT_REF
<ADDR_EXPR>, copy_body_r doesn't try to build_fold_indirect_ref,
but calls fold_indirect_ref_1 and if that didn't optimize something,
forcefully removes both INDIRECT_REF and ADDR_EXPR, which is IMHO a bad
thing, because it doesn't care about the types.
Without IMA this works, because fold_indirect_ref_1 folds the address
of the *&optab_table into optable_table[0]:
      /* *(foo *)&fooarray => fooarray[0] */
      else if (TREE_CODE (optype) == ARRAY_TYPE
               && type == TREE_TYPE (optype))
        {
          tree type_domain = TYPE_DOMAIN (optype);
          tree min_val = size_zero_node;
          if (type_domain && TYPE_MIN_VALUE (type_domain))
            min_val = TYPE_MIN_VALUE (type_domain);
          return build4 (ARRAY_REF, type, op, min_val, NULL_TREE, NULL_TREE);
        }
but on this testcase it unfortunately doesn't trigger, because the
types are just compatible, not identical.  copy_body_r is the only
direct user of fold_indirect_ref_1, in all other cases this folding is
just an optimization, as when it is not folded, INDIRECT_REF is created
or the original INDIRECT_REF is kept.  But in tree-inline.c if
fold_indirect_ref_1 returns NULL, it will
	if (TREE_CODE (new) == ADDR_EXPR)
	  *tp = TREE_OPERAND (new, 0);
forcefully, not bothering to check types.  Changing fold_indirect_ref_1
to work with types_compatible_p would be probably too expensive.

The following patch fixes this by checking if types_compatible_p in
copy_body_r before the removal, or checks similarly to fold_indirect_ref_1
for *(foo *)&fooarray with just compatible types rather than identical
and even if those aren't compatible (e.g. from fold_indirect_ref_1
the *&complex -> __real__ complex and *&vector -> BIT_FIELD_REF) just casts
and forces regimplification.
For the attached testcase actually either the else if, or else is
sufficient, but I believe we want both.  If just the else is present,
then we later segfault in get_addr_dereference_operands because the just
created temporary doesn't have var_ann yet (and calling there get_var_ann
doesn't seem to work), that's why it also adds a non-NULL check for v_ann.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2008-01-10  Jakub Jelinek  <jakub@redhat.com>

	PR c/34668
	* tree-inline.c (copy_body_r): For INDIRECT_REF <ADDR_EXPR> store
	ADDR_EXPR operand only if the types are compatible, otherwise
	either change ADDR_EXPR of array into its first entry or fold_convert
	the pointer and regimplify.
	(copy_phis_for_bb): Regimplify also if id->regimplify.
	* tree-ssa-operands.c (get_addr_dereference_operands): Don't dereference
	v_ann if NULL.

	* gcc.dg/pr34668-1.c: New test.
	* gcc.dg/pr34668-2.c: New test.

--- gcc/tree-inline.c.jj	2008-01-04 00:53:03.000000000 +0100
+++ gcc/tree-inline.c	2008-01-10 00:48:07.000000000 +0100
@@ -700,7 +700,30 @@ copy_body_r (tree *tp, int *walk_subtree
 	      if (! *tp)
 	        {
 		  if (TREE_CODE (new) == ADDR_EXPR)
-		    *tp = TREE_OPERAND (new, 0);
+		    {
+		      tree t = TREE_OPERAND (new, 0);
+		      if (types_compatible_p (type, TREE_TYPE (t)))
+			*tp = t;
+		      else if (TREE_CODE (TREE_TYPE (t)) == ARRAY_TYPE
+			       && types_compatible_p (TREE_TYPE (TREE_TYPE (t)),
+						      type))
+			{
+			  tree type_domain = TYPE_DOMAIN (TREE_TYPE (t));
+			  tree min_val = size_zero_node;
+			  if (type_domain && TYPE_MIN_VALUE (type_domain))
+			    min_val = TYPE_MIN_VALUE (type_domain);
+			  *tp = build4 (ARRAY_REF, type, t, min_val,
+					NULL_TREE, NULL_TREE);
+			}
+		      else
+			{
+			  t = build_fold_addr_expr (t);
+			  t = fold_convert (TREE_TYPE (new), t);
+			  *tp = build1 (INDIRECT_REF, type, t);
+			  TREE_THIS_VOLATILE (*tp) = TREE_THIS_VOLATILE (old);
+			  id->regimplify = true;
+			}
+		    }
 	          else
 		    {
 	              *tp = build1 (INDIRECT_REF, type, new);
@@ -1210,13 +1233,15 @@ copy_phis_for_bb (basic_block bb, copy_b
 	      tree arg = PHI_ARG_DEF_FROM_EDGE (phi, old_edge);
 	      tree new_arg = arg;
 
+	      id->regimplify = false;
 	      walk_tree (&new_arg, copy_body_r, id, NULL);
 	      gcc_assert (new_arg);
 	      /* With return slot optimization we can end up with
 	         non-gimple (foo *)&this->m, fix that here.  */
-	      if (TREE_CODE (new_arg) != SSA_NAME
-		  && TREE_CODE (new_arg) != FUNCTION_DECL
-		  && !is_gimple_val (new_arg))
+	      if ((TREE_CODE (new_arg) != SSA_NAME
+		   && TREE_CODE (new_arg) != FUNCTION_DECL
+		   && !is_gimple_val (new_arg))
+		  || id->regimplify)
 		{
 		  tree stmts = NULL_TREE;
 		  new_arg = force_gimple_operand (new_arg, &stmts,
--- gcc/tree-ssa-operands.c.jj	2008-01-04 00:53:03.000000000 +0100
+++ gcc/tree-ssa-operands.c	2008-01-10 00:38:25.000000000 +0100
@@ -1663,7 +1663,7 @@ get_addr_dereference_operands (tree stmt
 	  /* If we don't know what this pointer points to then we have
 	     to make sure to not prune virtual operands based on offset
 	     and size.  */
-	  if (v_ann->symbol_mem_tag)
+	  if (v_ann && v_ann->symbol_mem_tag)
 	    {
 	      add_virtual_operand (v_ann->symbol_mem_tag, s_ann, flags,
 				   full_ref, 0, -1, false);
--- gcc/testsuite/gcc.dg/pr34668-1.c.jj	2008-01-10 00:55:59.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr34668-1.c	2008-01-10 00:59:02.000000000 +0100
@@ -0,0 +1,19 @@
+/* PR c/34668 */
+/* { dg-do compile } */
+/* { dg-options "--combine -O2" } */
+/* { dg-additional-sources "pr34668-2.c" } */
+
+struct optab { unsigned code; };
+extern struct optab optab_table[1];
+
+void
+init_optab (struct optab *op)
+{
+  op->code = 0xdead;
+}
+
+void
+set_conv_libfunc (void)
+{
+  init_optab (&optab_table[0]);
+}
--- gcc/testsuite/gcc.dg/pr34668-2.c.jj	2008-01-10 00:56:02.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr34668-2.c	2008-01-10 00:56:36.000000000 +0100
@@ -0,0 +1,5 @@
+/* PR c/34668 */
+/* { dg-do compile } */
+
+struct optab { unsigned code; };
+extern struct optab optab_table[1];

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]