Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)

[Jakub Jelinek <jakub at redhat dot com>]

On Fri, Mar 11, 2016 at 09:27:54AM -0500, Jason Merrill wrote:
> On 03/10/2016 01:39 PM, Jakub Jelinek wrote:
> >+      /* Don't reuse the result of cxx_eval_constant_expression
> >+	 call if it isn't a constant initializer or if it requires
> >+	 relocations.  */
> 
> Let's phrase this positively ("Reuse the result if...").
> 
> >+	  if (new_ctx.ctor != ctx->ctor)
> >+	    eltinit = new_ctx.ctor;
> >+	  for (i = 1; i < max; ++i)
> >+	    {
> >+	      idx = build_int_cst (size_type_node, i);
> >+	      CONSTRUCTOR_APPEND_ELT (*p, idx, eltinit);
> >+	    }
> 
> This is going to use the same CONSTRUCTOR for all the elements, which will
> lead to problems if we then store into a subobject of one of the elements
> and see that reflected in all the others as well.  We need to unshare_expr
> when reusing the initializer.

Ok, here is a new version with unshare_expr and reworded comment.
I haven't been successful with writing a testcase where the unshare_expr
would matter, but have included what I've tried in the patch anyway.
Compile-time wise the unshare_expr is not very costly, and e.g. for the
other testcase in this patch gives a nice compile time speedup.

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

2016-03-11  Jakub Jelinek  <jakub@redhat.com>

	PR c++/70001
	* constexpr.c (cxx_eval_vec_init_1): Reuse CONSTRUCTOR initializers
	for 1..max even for multi-dimensional arrays.  Call unshare_expr
	on it.

	* g++.dg/cpp0x/constexpr-70001-4.C: New test.
	* g++.dg/cpp1y/pr70001.C: New test.

--- gcc/cp/constexpr.c.jj	2016-03-10 12:52:04.000000000 +0100
+++ gcc/cp/constexpr.c	2016-03-11 19:24:28.435537864 +0100
@@ -2340,7 +2340,6 @@ cxx_eval_vec_init_1 (const constexpr_ctx
   vec<constructor_elt, va_gc> **p = &CONSTRUCTOR_ELTS (ctx->ctor);
   vec_alloc (*p, max + 1);
   bool pre_init = false;
-  tree pre_init_elt = NULL_TREE;
   unsigned HOST_WIDE_INT i;
 
   /* For the default constructor, build up a call to the default
@@ -2370,6 +2369,7 @@ cxx_eval_vec_init_1 (const constexpr_ctx
     {
       tree idx = build_int_cst (size_type_node, i);
       tree eltinit;
+      bool reuse = false;
       constexpr_ctx new_ctx;
       init_subob_ctx (ctx, new_ctx, idx, pre_init ? init : elttype);
       if (new_ctx.ctor != ctx->ctor)
@@ -2378,7 +2378,10 @@ cxx_eval_vec_init_1 (const constexpr_ctx
 	{
 	  /* A multidimensional array; recurse.  */
 	  if (value_init || init == NULL_TREE)
-	    eltinit = NULL_TREE;
+	    {
+	      eltinit = NULL_TREE;
+	      reuse = i == 0;
+	    }
 	  else
 	    eltinit = cp_build_array_ref (input_location, init, idx,
 					  tf_warning_or_error);
@@ -2390,18 +2393,9 @@ cxx_eval_vec_init_1 (const constexpr_ctx
 	{
 	  /* Initializing an element using value or default initialization
 	     we just pre-built above.  */
-	  if (pre_init_elt == NULL_TREE)
-	    pre_init_elt
-	      = cxx_eval_constant_expression (&new_ctx, init, lval,
-					      non_constant_p, overflow_p);
-	  eltinit = pre_init_elt;
-	  /* Don't reuse the result of cxx_eval_constant_expression
-	     call if it isn't a constant initializer or if it requires
-	     relocations.  */
-	  if (initializer_constant_valid_p (pre_init_elt,
-					    TREE_TYPE (pre_init_elt))
-	      != null_pointer_node)
-	    pre_init_elt = NULL_TREE;
+	  eltinit = cxx_eval_constant_expression (&new_ctx, init, lval,
+						  non_constant_p, overflow_p);
+	  reuse = i == 0;
 	}
       else
 	{
@@ -2427,6 +2421,23 @@ cxx_eval_vec_init_1 (const constexpr_ctx
 	}
       else
 	CONSTRUCTOR_APPEND_ELT (*p, idx, eltinit);
+      /* Reuse the result of cxx_eval_constant_expression call
+	  from the first iteration to all others if it is a constant
+	  initializer that doesn't require relocations.  */
+      if (reuse
+	  && max > 1
+	  && (initializer_constant_valid_p (eltinit, TREE_TYPE (eltinit))
+	      == null_pointer_node))
+	{
+	  if (new_ctx.ctor != ctx->ctor)
+	    eltinit = new_ctx.ctor;
+	  for (i = 1; i < max; ++i)
+	    {
+	      idx = build_int_cst (size_type_node, i);
+	      CONSTRUCTOR_APPEND_ELT (*p, idx, unshare_expr (eltinit));
+	    }
+	  break;
+	}
     }
 
   if (!*non_constant_p)
--- gcc/testsuite/g++.dg/cpp0x/constexpr-70001-4.C.jj	2016-03-10 19:28:13.386481311 +0100
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-70001-4.C	2016-03-10 19:28:43.295074924 +0100
@@ -0,0 +1,13 @@
+// PR c++/70001
+// { dg-do compile { target c++11 } }
+
+struct B
+{
+  int a;
+  constexpr B () : a (0) { }
+};
+
+struct A
+{
+  B b[1 << 19][1][1][1];
+} c;
--- gcc/testsuite/g++.dg/cpp1y/pr70001.C.jj	2016-03-11 18:22:15.526046513 +0100
+++ gcc/testsuite/g++.dg/cpp1y/pr70001.C	2016-03-11 18:21:43.000000000 +0100
@@ -0,0 +1,49 @@
+// PR c++/70001
+// { dg-do compile { target c++14 } }
+
+struct B
+{
+  int a;
+  constexpr B () : a (0) { }
+  constexpr B (int x) : a (x) { }
+};
+struct C
+{
+  B c;
+  constexpr C () : c (0) { }
+};
+struct A
+{
+  B b[1 << 4];
+};
+struct D
+{
+  C d[1 << 4];
+};
+
+constexpr int
+foo (int a, int b)
+{
+  A c;
+  c.b[a].a += b;
+  c.b[b].a += a;
+  return c.b[0].a + c.b[a].a + c.b[b].a;
+}
+
+constexpr int
+bar (int a, int b)
+{
+  D c;
+  c.d[a].c.a += b;
+  c.d[b].c.a += a;
+  return c.d[0].c.a + c.d[a].c.a + c.d[b].c.a;
+}
+
+constexpr int d = foo (1, 2);
+constexpr int e = foo (0, 3);
+constexpr int f = foo (2, 4);
+constexpr int g = bar (1, 2);
+constexpr int h = bar (0, 3);
+constexpr int i = bar (2, 4);
+static_assert (d == 3 && e == 6 && f == 6, "");
+static_assert (g == 3 && h == 6 && i == 6, "");


	Jakub