diff mbox series

[C] Outside of initializers partially restore c_fully_fold 7.x behavior (PR c/83801)

Message ID 20180112160310.GD2063@tucnak
State New
Headers show
Series [C] Outside of initializers partially restore c_fully_fold 7.x behavior (PR c/83801) | expand

Commit Message

Jakub Jelinek Jan. 12, 2018, 4:03 p.m. UTC 
Hi!

In 7.x and earlier, decl_constant_value_for_optimization punted if
the initializer has ARRAY_TYPE or BLKmode type, and the following patch
shows that is reasonable thing to do, otherwise we might grab very large
initializers out of constant variable initializers and if e.g. ARRAY_REF
on it doesn't fold into a small constant, this may result in the large
constants being duplicated, because the IL before gimplification will
no longer refer to portions of the original constant var decl.
E.g. look at the testcase below, with current trunk we fold the first
function to:
return (char) "01234567890123456789012345678901234567890123456789012345678901234567890123456789"[i];
and because it is first an initializer of a variable, without
-fmerge-all-constants we end up with 2 copies of that string.
Haven't changed for this decl_constant_value, because it is used in the
-Wformat code and needs to grab there the format strings (i.e.
ARRAY_TYPE/BLKmode initializers).
On the other side, when folding expressions in static variable initializers,
we can try harder, because if e.g. the ARRAY_REF has index that doesn't fold
into constant, it will be invalid anyway, so it is useful to pick there even
CONSTRUCTORs and handle COMPONENT_REFs on them.

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

2018-01-12  Jakub Jelinek  <jakub@redhat.com>

	PR c/83801
	* c-tree.h (decl_constant_value_1): Add a bool argument.
	* c-typeck.c (decl_constant_value_1): Add IN_INIT argument, allow
	returning a CONSTRUCTOR if it is true.  Use error_operand_p.
	(decl_constant_value): Adjust caller.
	* c-fold.c (c_fully_fold_internal): If in_init, pass true to
	decl_constant_value_1 as IN_INIT.  Otherwise, punt if
	decl_constant_value returns initializer that has BLKmode or
	array type.
	(c_fully_fold_internal) <case COMPONENT_REF>: Fold if !lval.

	* gcc.dg/pr83801.c: New test.


	Jakub

Comments

Jeff Law Jan. 13, 2018, 4:23 p.m. UTC | #1 
On 01/12/2018 09:03 AM, Jakub Jelinek wrote:
> Hi!
> 
> In 7.x and earlier, decl_constant_value_for_optimization punted if
> the initializer has ARRAY_TYPE or BLKmode type, and the following patch
> shows that is reasonable thing to do, otherwise we might grab very large
> initializers out of constant variable initializers and if e.g. ARRAY_REF
> on it doesn't fold into a small constant, this may result in the large
> constants being duplicated, because the IL before gimplification will
> no longer refer to portions of the original constant var decl.
> E.g. look at the testcase below, with current trunk we fold the first
> function to:
> return (char) "01234567890123456789012345678901234567890123456789012345678901234567890123456789"[i];
> and because it is first an initializer of a variable, without
> -fmerge-all-constants we end up with 2 copies of that string.
> Haven't changed for this decl_constant_value, because it is used in the
> -Wformat code and needs to grab there the format strings (i.e.
> ARRAY_TYPE/BLKmode initializers).
> On the other side, when folding expressions in static variable initializers,
> we can try harder, because if e.g. the ARRAY_REF has index that doesn't fold
> into constant, it will be invalid anyway, so it is useful to pick there even
> CONSTRUCTORs and handle COMPONENT_REFs on them.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2018-01-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/83801
> 	* c-tree.h (decl_constant_value_1): Add a bool argument.
> 	* c-typeck.c (decl_constant_value_1): Add IN_INIT argument, allow
> 	returning a CONSTRUCTOR if it is true.  Use error_operand_p.
> 	(decl_constant_value): Adjust caller.
> 	* c-fold.c (c_fully_fold_internal): If in_init, pass true to
> 	decl_constant_value_1 as IN_INIT.  Otherwise, punt if
> 	decl_constant_value returns initializer that has BLKmode or
> 	array type.
> 	(c_fully_fold_internal) <case COMPONENT_REF>: Fold if !lval.
> 
> 	* gcc.dg/pr83801.c: New test.
OK.
jeff
diff mbox series

Patch

--- gcc/c/c-tree.h.jj	2018-01-03 10:20:20.122537951 +0100
+++ gcc/c/c-tree.h	2018-01-12 12:30:32.206356337 +0100
@@ -640,7 +640,7 @@  extern struct c_expr default_function_ar
 							     struct c_expr);
 extern struct c_expr convert_lvalue_to_rvalue (location_t, struct c_expr,
 					       bool, bool);
-extern tree decl_constant_value_1 (tree);
+extern tree decl_constant_value_1 (tree, bool);
 extern void mark_exp_read (tree);
 extern tree composite_type (tree, tree);
 extern tree build_component_ref (location_t, tree, tree, location_t);
--- gcc/c/c-typeck.c.jj	2018-01-04 00:43:15.524702590 +0100
+++ gcc/c/c-typeck.c	2018-01-12 13:28:07.512304545 +0100
@@ -1832,20 +1832,20 @@  c_size_in_bytes (const_tree type)
 /* Return either DECL or its known constant value (if it has one).  */
 
 tree
-decl_constant_value_1 (tree decl)
+decl_constant_value_1 (tree decl, bool in_init)
 {
   if (/* Note that DECL_INITIAL isn't valid for a PARM_DECL.  */
       TREE_CODE (decl) != PARM_DECL
       && !TREE_THIS_VOLATILE (decl)
       && TREE_READONLY (decl)
       && DECL_INITIAL (decl) != NULL_TREE
-      && TREE_CODE (DECL_INITIAL (decl)) != ERROR_MARK
+      && !error_operand_p (DECL_INITIAL (decl))
       /* This is invalid if initial value is not constant.
 	 If it has either a function call, a memory reference,
 	 or a variable, then re-evaluating it could give different results.  */
       && TREE_CONSTANT (DECL_INITIAL (decl))
       /* Check for cases where this is sub-optimal, even though valid.  */
-      && TREE_CODE (DECL_INITIAL (decl)) != CONSTRUCTOR)
+      && (in_init || TREE_CODE (DECL_INITIAL (decl)) != CONSTRUCTOR))
     return DECL_INITIAL (decl);
   return decl;
 }
@@ -1858,7 +1858,7 @@  decl_constant_value (tree decl)
 {
   /* Don't change a variable array bound or initial value to a constant
      in a place where a variable is invalid.  */
-  return current_function_decl ? decl_constant_value_1 (decl) : decl;
+  return current_function_decl ? decl_constant_value_1 (decl, false) : decl;
 }
 
 /* Convert the array expression EXP to a pointer.  */
--- gcc/c/c-fold.c.jj	2018-01-03 10:20:20.129537952 +0100
+++ gcc/c/c-fold.c	2018-01-12 13:39:35.189290965 +0100
@@ -168,9 +168,15 @@  c_fully_fold_internal (tree expr, bool i
       if (VAR_P (expr) && !lval && (optimize || in_init))
 	{
 	  if (in_init)
-	    ret = decl_constant_value_1 (expr);
+	    ret = decl_constant_value_1 (expr, true);
 	  else
-	    ret = decl_constant_value (expr);
+	    {
+	      ret = decl_constant_value (expr);
+	      if (ret != expr
+		  && (TYPE_MODE (TREE_TYPE (ret)) == BLKmode
+		      || TREE_CODE (TREE_TYPE (ret)) == ARRAY_TYPE))
+		return expr;
+	    }
 	  /* Avoid unwanted tree sharing between the initializer and current
 	     function's body where the tree can be modified e.g. by the
 	     gimplifier.  */
@@ -264,6 +270,8 @@  c_fully_fold_internal (tree expr, bool i
 	  TREE_READONLY (ret) = TREE_READONLY (expr);
 	  TREE_THIS_VOLATILE (ret) = TREE_THIS_VOLATILE (expr);
 	}
+      if (!lval)
+	ret = fold (ret);
       goto out;
 
     case ARRAY_REF:
--- gcc/testsuite/gcc.dg/pr83801.c.jj	2018-01-12 13:32:31.977304544 +0100
+++ gcc/testsuite/gcc.dg/pr83801.c	2018-01-12 13:37:12.271295546 +0100
@@ -0,0 +1,27 @@ 
+/* PR c/83801 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-original" } */
+
+static const char a[] = "01234567890123456789012345678901234567890123456789012345678901234567890123456789";
+static const char b = a[27];
+struct S { const char c[30]; const char d[30]; };
+static const struct S e[] = { { "01234567890123456789012345678", "90123456789012345678901234567" },
+                              { "89012345678901234567890123456", "78901234567890123456789012345" } };
+static const char f = e[1].c[4];
+
+char
+foo (int i)
+{
+  return a[i];
+}
+
+char
+bar (int i)
+{
+  return e[0].d[i];
+}
+
+/* { dg-final { scan-tree-dump {a\[i]} "original" } } */
+/* { dg-final { scan-tree-dump-not {"01234567890123456789012345678901234567890123456789012345678901234567890123456789"\[i]} "original" } } */
+/* { dg-final { scan-tree-dump {e\[0]\.d\[i]} "original" } } */
+/* { dg-final { scan-tree-dump-not {"90123456789012345678901234567"\[i]} "original" } } */