* [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
@ 2016-03-10 17:04 Jakub Jelinek
2016-03-10 17:19 ` Jason Merrill
2016-03-10 17:35 ` Patrick Palka
0 siblings, 2 replies; 12+ messages in thread
From: Jakub Jelinek @ 2016-03-10 17:04 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
As mentioned in the PR, the compile time and compile memory are wasted
if a large array is is using value or default initialization, and
if the resulting initializer value is simple enough, we can just share
it by all the elements.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-03-10 Patrick Palka <ppalka@gcc.gnu.org>
Jakub Jelinek <jakub@redhat.com>
PR c++/70001
* constexpr.c (cxx_eval_vec_init_1): For pre_init case, reuse
return value from cxx_eval_constant_expression from earlier
elements if it is valid constant initializer requiring no
relocations.
* g++.dg/cpp0x/constexpr-70001-1.C: New test.
* g++.dg/cpp0x/constexpr-70001-2.C: New test.
* g++.dg/cpp0x/constexpr-70001-3.C: New test.
--- gcc/cp/constexpr.c.jj 2016-03-08 21:04:43.050564671 +0100
+++ gcc/cp/constexpr.c 2016-03-10 12:52:04.016852313 +0100
@@ -2340,6 +2340,7 @@ 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
@@ -2389,9 +2390,18 @@ cxx_eval_vec_init_1 (const constexpr_ctx
{
/* Initializing an element using value or default initialization
we just pre-built above. */
- eltinit = (cxx_eval_constant_expression
- (&new_ctx, init,
- lval, non_constant_p, overflow_p));
+ 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;
}
else
{
--- gcc/testsuite/g++.dg/cpp0x/constexpr-70001-1.C.jj 2016-03-10 13:08:58.732932160 +0100
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-70001-1.C 2016-03-10 13:05:53.000000000 +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];
+} c;
--- gcc/testsuite/g++.dg/cpp0x/constexpr-70001-2.C.jj 2016-03-10 13:09:01.866889167 +0100
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-70001-2.C 2016-03-10 13:07:27.000000000 +0100
@@ -0,0 +1,19 @@
+// PR c++/70001
+// { dg-do run { target c++11 } }
+
+struct B
+{
+ struct B *a;
+ constexpr B () : a (this) { }
+};
+
+constexpr int N = 1 << 4;
+struct A { B c[N]; } d;
+
+int
+main ()
+{
+ for (int i = 0; i < N; ++i)
+ if (d.c[i].a != &d.c[i])
+ __builtin_abort ();
+}
--- gcc/testsuite/g++.dg/cpp0x/constexpr-70001-3.C.jj 2016-03-10 13:09:04.700850290 +0100
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-70001-3.C 2016-03-10 13:09:53.199184977 +0100
@@ -0,0 +1,26 @@
+// PR c++/70001
+// { dg-do compile { target c++11 } }
+
+#include <array>
+#include <complex>
+
+typedef std::complex<double> cd;
+
+const int LOG = 17;
+const int N = (1 << LOG);
+
+std::array<cd, N> a;
+std::array<cd, N> b;
+
+void
+foo (std::array<cd, N> &arr)
+{
+ std::array<std::array<cd, N>, LOG + 1> f;
+}
+
+int
+main ()
+{
+ foo (a);
+ foo (b);
+}
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
2016-03-10 17:04 [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001) Jakub Jelinek
@ 2016-03-10 17:19 ` Jason Merrill
2016-03-10 17:35 ` Patrick Palka
1 sibling, 0 replies; 12+ messages in thread
From: Jason Merrill @ 2016-03-10 17:19 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
OK.
Jason
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
2016-03-10 17:04 [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001) Jakub Jelinek
2016-03-10 17:19 ` Jason Merrill
@ 2016-03-10 17:35 ` Patrick Palka
2016-03-10 17:37 ` Jakub Jelinek
1 sibling, 1 reply; 12+ messages in thread
From: Patrick Palka @ 2016-03-10 17:35 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, GCC Patches
On Thu, Mar 10, 2016 at 12:03 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> As mentioned in the PR, the compile time and compile memory are wasted
> if a large array is is using value or default initialization, and
> if the resulting initializer value is simple enough, we can just share
> it by all the elements.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2016-03-10 Patrick Palka <ppalka@gcc.gnu.org>
> Jakub Jelinek <jakub@redhat.com>
>
> PR c++/70001
> * constexpr.c (cxx_eval_vec_init_1): For pre_init case, reuse
> return value from cxx_eval_constant_expression from earlier
> elements if it is valid constant initializer requiring no
> relocations.
>
> * g++.dg/cpp0x/constexpr-70001-1.C: New test.
> * g++.dg/cpp0x/constexpr-70001-2.C: New test.
> * g++.dg/cpp0x/constexpr-70001-3.C: New test.
>
> --- gcc/cp/constexpr.c.jj 2016-03-08 21:04:43.050564671 +0100
> +++ gcc/cp/constexpr.c 2016-03-10 12:52:04.016852313 +0100
> @@ -2340,6 +2340,7 @@ 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
> @@ -2389,9 +2390,18 @@ cxx_eval_vec_init_1 (const constexpr_ctx
> {
> /* Initializing an element using value or default initialization
> we just pre-built above. */
> - eltinit = (cxx_eval_constant_expression
> - (&new_ctx, init,
> - lval, non_constant_p, overflow_p));
> + 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;
Doesn't this mean that we call initializer_constant_valid_p at each
iteration? This would slow down the non-constant case even further.
So I wonder if the return value of initializer_constant_valid_p could
be cached or something, since it seems like a potentially expensive
predicate.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
2016-03-10 17:35 ` Patrick Palka
@ 2016-03-10 17:37 ` Jakub Jelinek
2016-03-10 17:56 ` Jakub Jelinek
0 siblings, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2016-03-10 17:37 UTC (permalink / raw)
To: Patrick Palka; +Cc: Jason Merrill, GCC Patches
On Thu, Mar 10, 2016 at 12:34:40PM -0500, Patrick Palka wrote:
> Doesn't this mean that we call initializer_constant_valid_p at each
> iteration? This would slow down the non-constant case even further.
> So I wonder if the return value of initializer_constant_valid_p could
> be cached or something, since it seems like a potentially expensive
> predicate.
You're right, but I've already committed the patch. I'll write an
incremental patch and test it.
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
2016-03-10 17:37 ` Jakub Jelinek
@ 2016-03-10 17:56 ` Jakub Jelinek
2016-03-10 18:32 ` Patrick Palka
2016-03-10 18:33 ` Jason Merrill
0 siblings, 2 replies; 12+ messages in thread
From: Jakub Jelinek @ 2016-03-10 17:56 UTC (permalink / raw)
To: Patrick Palka; +Cc: Jason Merrill, GCC Patches
On Thu, Mar 10, 2016 at 06:37:32PM +0100, Jakub Jelinek wrote:
> On Thu, Mar 10, 2016 at 12:34:40PM -0500, Patrick Palka wrote:
> > Doesn't this mean that we call initializer_constant_valid_p at each
> > iteration? This would slow down the non-constant case even further.
> > So I wonder if the return value of initializer_constant_valid_p could
> > be cached or something, since it seems like a potentially expensive
> > predicate.
>
> You're right, but I've already committed the patch. I'll write an
> incremental patch and test it.
So, like this?
2016-03-10 Jakub Jelinek <jakub@redhat.com>
PR c++/70001
* constexpr.c (cxx_eval_vec_init_1): For pre_init, only
call initializer_constant_valid_p on the first iteration.
--- gcc/cp/constexpr.c.jj 2016-03-10 12:52:04.000000000 +0100
+++ gcc/cp/constexpr.c 2016-03-10 18:45:13.416533853 +0100
@@ -2391,17 +2391,21 @@ 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);
+ /* Don't reuse the result of cxx_eval_constant_expression
+ call if it isn't a constant initializer or if it requires
+ relocations. */
+ if (i == 0
+ && (initializer_constant_valid_p (eltinit,
+ TREE_TYPE (eltinit))
+ == null_pointer_node))
+ pre_init_elt = eltinit;
+ }
+ else
+ eltinit = pre_init_elt;
}
else
{
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
2016-03-10 17:56 ` Jakub Jelinek
@ 2016-03-10 18:32 ` Patrick Palka
2016-03-10 18:40 ` Jakub Jelinek
2016-03-10 18:33 ` Jason Merrill
1 sibling, 1 reply; 12+ messages in thread
From: Patrick Palka @ 2016-03-10 18:32 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, GCC Patches
On Thu, Mar 10, 2016 at 12:56 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Mar 10, 2016 at 06:37:32PM +0100, Jakub Jelinek wrote:
>> On Thu, Mar 10, 2016 at 12:34:40PM -0500, Patrick Palka wrote:
>> > Doesn't this mean that we call initializer_constant_valid_p at each
>> > iteration? This would slow down the non-constant case even further.
>> > So I wonder if the return value of initializer_constant_valid_p could
>> > be cached or something, since it seems like a potentially expensive
>> > predicate.
>>
>> You're right, but I've already committed the patch. I'll write an
>> incremental patch and test it.
>
> So, like this?
>
> 2016-03-10 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/70001
> * constexpr.c (cxx_eval_vec_init_1): For pre_init, only
> call initializer_constant_valid_p on the first iteration.
>
> --- gcc/cp/constexpr.c.jj 2016-03-10 12:52:04.000000000 +0100
> +++ gcc/cp/constexpr.c 2016-03-10 18:45:13.416533853 +0100
> @@ -2391,17 +2391,21 @@ 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);
> + /* Don't reuse the result of cxx_eval_constant_expression
> + call if it isn't a constant initializer or if it requires
> + relocations. */
> + if (i == 0
> + && (initializer_constant_valid_p (eltinit,
> + TREE_TYPE (eltinit))
> + == null_pointer_node))
> + pre_init_elt = eltinit;
> + }
> + else
> + eltinit = pre_init_elt;
> }
> else
> {
Looks fine to me :)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
2016-03-10 17:56 ` Jakub Jelinek
2016-03-10 18:32 ` Patrick Palka
@ 2016-03-10 18:33 ` Jason Merrill
1 sibling, 0 replies; 12+ messages in thread
From: Jason Merrill @ 2016-03-10 18:33 UTC (permalink / raw)
To: Jakub Jelinek, Patrick Palka; +Cc: GCC Patches
OK.
Jason
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
2016-03-10 18:32 ` Patrick Palka
@ 2016-03-10 18:40 ` Jakub Jelinek
2016-03-10 20:33 ` Jakub Jelinek
2016-03-11 14:27 ` Jason Merrill
0 siblings, 2 replies; 12+ messages in thread
From: Jakub Jelinek @ 2016-03-10 18:40 UTC (permalink / raw)
To: Patrick Palka; +Cc: Jason Merrill, GCC Patches
On Thu, Mar 10, 2016 at 01:32:10PM -0500, Patrick Palka wrote:
> Looks fine to me :)
On a closer look, this doesn't handle the multi-dimensional array cases,
and even for single-dimensional ones will not share the CONSTRUCTOR
if init_subob_ctx created one, and call init_subob_ctx many times
and in there in some cases e.g. build new new_ctx.object every time etc.
Is this also ok, if it passes bootstrap/regtest?
2016-03-10 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, and reuse not just
eltinit itself, but surrounding subobject CONSTRUCTOR too.
* g++.dg/cpp0x/constexpr-70001-4.C: New test.
--- gcc/cp/constexpr.c.jj 2016-03-10 12:52:04.000000000 +0100
+++ gcc/cp/constexpr.c 2016-03-10 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);
+ /* Don't reuse the result of cxx_eval_constant_expression
+ call if it isn't a constant initializer or if it requires
+ 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, 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;
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
2016-03-10 18:40 ` Jakub Jelinek
@ 2016-03-10 20:33 ` Jakub Jelinek
2016-03-11 14:27 ` Jason Merrill
1 sibling, 0 replies; 12+ messages in thread
From: Jakub Jelinek @ 2016-03-10 20:33 UTC (permalink / raw)
To: Patrick Palka, Jason Merrill; +Cc: GCC Patches
On Thu, Mar 10, 2016 at 07:39:57PM +0100, Jakub Jelinek wrote:
> Is this also ok, if it passes bootstrap/regtest?
>
> 2016-03-10 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, and reuse not just
> eltinit itself, but surrounding subobject CONSTRUCTOR too.
>
> * g++.dg/cpp0x/constexpr-70001-4.C: New test.
Successfully bootstrapped/regtested on x86_64-linux and i686-linux.
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
2016-03-10 18:40 ` Jakub Jelinek
2016-03-10 20:33 ` Jakub Jelinek
@ 2016-03-11 14:27 ` Jason Merrill
2016-03-11 14:50 ` Jakub Jelinek
2016-03-11 19:38 ` Jakub Jelinek
1 sibling, 2 replies; 12+ messages in thread
From: Jason Merrill @ 2016-03-11 14:27 UTC (permalink / raw)
To: Jakub Jelinek, Patrick Palka; +Cc: GCC Patches
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.
Jason
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
2016-03-11 14:27 ` Jason Merrill
@ 2016-03-11 14:50 ` Jakub Jelinek
2016-03-11 19:38 ` Jakub Jelinek
1 sibling, 0 replies; 12+ messages in thread
From: Jakub Jelinek @ 2016-03-11 14:50 UTC (permalink / raw)
To: Jason Merrill; +Cc: Patrick Palka, GCC Patches
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.
Well, but then even what has been already committed is unsafe,
initializer_constant_valid_p can return null_pointer_node even on
CONSTRUCTOR, or CONSTRUCTOR holding CONSTRUCTORs etc.
So, either we need to unshare_expr it in every case, so
CONSTRUCTOR_APPEND_ELT (*p, idx, unshare_expr (eltinit));
or alternatively we could use some flag on CONSTRUCTOR to mark (possibly)
shared ctors and unshare them upon constexpr store to them, or
unshare whenever we store.
What would be a testcase for the unsharing?
Following still works:
// 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 d = foo (1, 2);
constexpr int e = foo (0, 3);
constexpr int f = foo (2, 4);
static_assert (d == 3 && e == 6 && f == 6, "");
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 g = bar (1, 2);
constexpr int h = bar (0, 3);
constexpr int i = bar (2, 4);
static_assert (g == 3 && h == 6 && i == 6, "");
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001)
2016-03-11 14:27 ` Jason Merrill
2016-03-11 14:50 ` Jakub Jelinek
@ 2016-03-11 19:38 ` Jakub Jelinek
1 sibling, 0 replies; 12+ messages in thread
From: Jakub Jelinek @ 2016-03-11 19:38 UTC (permalink / raw)
To: Jason Merrill; +Cc: Patrick Palka, GCC Patches
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-03-11 19:38 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-10 17:04 [C++ PATCH] Reuse certain cxx_eval_constant_expression results in cxx_eval_vec_init_1 (PR c++/70001) Jakub Jelinek
2016-03-10 17:19 ` Jason Merrill
2016-03-10 17:35 ` Patrick Palka
2016-03-10 17:37 ` Jakub Jelinek
2016-03-10 17:56 ` Jakub Jelinek
2016-03-10 18:32 ` Patrick Palka
2016-03-10 18:40 ` Jakub Jelinek
2016-03-10 20:33 ` Jakub Jelinek
2016-03-11 14:27 ` Jason Merrill
2016-03-11 14:50 ` Jakub Jelinek
2016-03-11 19:38 ` Jakub Jelinek
2016-03-10 18:33 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).