| Message ID | 20200803124356.385746-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: cxx_eval_vec_init after zero initialization [PR96282] | expand |
On Mon, 3 Aug 2020, Patrick Palka wrote: > In the first testcase below, expand_aggr_init_1 sets up t's default > constructor such that the ctor first zero-initializes the entire base b, > followed by calling b's default constructor, the latter of which just > default-initializes the array member b::m via a VEC_INIT_EXPR. > > So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is > nonempty due to the prior zero-initialization, and we proceed in > cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor > without first checking if a matching constructor_elt already exists. > This leads to ctx->ctor having two matching constructor_elts for each > index. > > This patch partially fixes this issue by making the RANGE_EXPR > optimization in cxx_eval_vec_init truncate ctx->ctor before adding the > single RANGE_EXPR constructor_elt. This isn't a complete fix because > the RANGE_EXPR optimization applies only when the constant initializer > is relocatable, so whenever it's not relocatable we can still build up > an invalid CONSTRUCTOR, e.g. if in the first testcase we add an NSDMI > such as 'e *p = this;' to struct e, then the ICE still occurs even with > this patch. A complete but more risky one-line fix would be to always truncate ctx->ctor beforehand, not just when the RANGE_EXPR optimization applies. If it's true that the initializer of a VEC_INIT_EXPR can't observe the previous elements of the target array, then it should be safe to always truncate I think? Another complete fix would be to replace the uses of CONSTRUCTOR_APPEND_ELT with get_or_insert_ctor_field, which does the right thing when a matching constructor_elt already exists. But get_or_insert_ctor_field was introduced only in GCC 10, . I'm not sure which fix we should go with in light of this being an 8/9/10/11 regression.. > > A side benefit of the approach taken by this patch is that constexpr > evaluation of a simple VEC_INIT_EXPR for a high-dimensional array no > longer scales exponentially with the number of dimensions. This is > because after the RANGE_EXPR optimization, the CONSTRUCTOR for each > array dimension now consists of a single constructor_elt (with index > 0...max-1) instead of two constructor_elts (with indexes 0 and 1...max-1 > respectively). This is verified by the second testcase below. > > Bootstrapped and regtested on x86_64-pc-linux-gnu. Does this look OK > for trunk and perhaps for backports? > > gcc/cp/ChangeLog: > > PR c++/96282 > * constexpr.c (cxx_eval_vec_init_1): Move the i == 0 test to the > if statement that guards the RANGE_EXPR optimization. Consider the > RANGE_EXPR optimization before we append the first element. > Truncate ctx->ctor when performing the RANGE_EXPR optimization. > Make the built RANGE_EXPR start at index 0 instead of 1. Don't > call unshare_constructor. > > gcc/testsuite/ChangeLog: > > PR c++/96282 > * g++.dg/cpp0x/constexpr-array26.C: New test. > * g++.dg/cpp0x/constexpr-array27.C: New test. > --- > gcc/cp/constexpr.c | 36 ++++++++++--------- > .../g++.dg/cpp0x/constexpr-array26.C | 13 +++++++ > .../g++.dg/cpp0x/constexpr-array27.C | 18 ++++++++++ > 3 files changed, 51 insertions(+), 16 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index b1c1d249c6e..3808a0713ba 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -4189,7 +4189,7 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, > if (value_init || init == NULL_TREE) > { > eltinit = NULL_TREE; > - reuse = i == 0; > + reuse = true; > } > else > eltinit = cp_build_array_ref (input_location, init, idx, complain); > @@ -4206,7 +4206,7 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, > return ctx->ctor; > eltinit = cxx_eval_constant_expression (&new_ctx, init, lval, > non_constant_p, overflow_p); > - reuse = i == 0; > + reuse = true; > } > else > { > @@ -4222,33 +4222,37 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, > } > if (*non_constant_p && !ctx->quiet) > break; > - if (new_ctx.ctor != ctx->ctor) > - { > - /* We appended this element above; update the value. */ > - gcc_assert ((*p)->last().index == idx); > - (*p)->last().value = eltinit; > - } > - 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 > + if (i == 0 > + && reuse > && (eltinit == NULL_TREE > || (initializer_constant_valid_p (eltinit, TREE_TYPE (eltinit)) > == null_pointer_node))) > { > if (new_ctx.ctor != ctx->ctor) > eltinit = new_ctx.ctor; > - tree range = build2 (RANGE_EXPR, size_type_node, > - build_int_cst (size_type_node, 1), > - build_int_cst (size_type_node, max - 1)); > - CONSTRUCTOR_APPEND_ELT (*p, range, unshare_constructor (eltinit)); > + vec_safe_truncate (*p, 0); > + if (max > 1) > + idx = build2 (RANGE_EXPR, size_type_node, > + build_int_cst (size_type_node, 0), > + build_int_cst (size_type_node, max - 1)); > + CONSTRUCTOR_APPEND_ELT (*p, idx, eltinit); > break; > } > else if (i == 0) > vec_safe_reserve (*p, max); > + > + if (new_ctx.ctor != ctx->ctor) > + { > + /* We appended this element above; update the value. */ > + gcc_assert ((*p)->last().index == idx); > + (*p)->last().value = eltinit; > + } > + else > + CONSTRUCTOR_APPEND_ELT (*p, idx, eltinit); > } > > if (!*non_constant_p) > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > new file mode 100644 > index 00000000000..274f55a88bf > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > @@ -0,0 +1,13 @@ > +// PR c++/96282 > +// { dg-do compile { target c++11 } } > + > +struct e { bool v = true; }; > + > +template<int N> > +struct b { e m[N]; }; > + > +template<int N> > +struct t : b<N> { constexpr t() : b<N>() {} }; > + > +constexpr t<1> h1; > +constexpr t<42> h2; > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > new file mode 100644 > index 00000000000..a5ce3f7be08 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > @@ -0,0 +1,18 @@ > +// Verify that default initialization an array of aggregates within an aggregate > +// does not scale exponentially with the number of dimensions. > + > +// { dg-do compile { target c++11 } } > +// Pass -fsyntax-only to stress only the performance of the frontend. > +// { dg-additional-options "-fsyntax-only" } > + > +struct A > +{ > + int a = 42; > +}; > + > +struct B > +{ > + A b[2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2]; > +}; > + > +constexpr B c; > -- > 2.28.0.89.g85b4e0a6dc > >
On 8/3/20 8:53 AM, Patrick Palka wrote: > On Mon, 3 Aug 2020, Patrick Palka wrote: > >> In the first testcase below, expand_aggr_init_1 sets up t's default >> constructor such that the ctor first zero-initializes the entire base b, >> followed by calling b's default constructor, the latter of which just >> default-initializes the array member b::m via a VEC_INIT_EXPR. >> >> So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is >> nonempty due to the prior zero-initialization, and we proceed in >> cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor >> without first checking if a matching constructor_elt already exists. >> This leads to ctx->ctor having two matching constructor_elts for each >> index. >> >> This patch partially fixes this issue by making the RANGE_EXPR >> optimization in cxx_eval_vec_init truncate ctx->ctor before adding the >> single RANGE_EXPR constructor_elt. This isn't a complete fix because >> the RANGE_EXPR optimization applies only when the constant initializer >> is relocatable, so whenever it's not relocatable we can still build up >> an invalid CONSTRUCTOR, e.g. if in the first testcase we add an NSDMI >> such as 'e *p = this;' to struct e, then the ICE still occurs even with >> this patch. > > A complete but more risky one-line fix would be to always truncate > ctx->ctor beforehand, not just when the RANGE_EXPR optimization applies. > If it's true that the initializer of a VEC_INIT_EXPR can't observe the > previous elements of the target array, then it should be safe to always > truncate I think? What if default-initialization of the array element type doesn't fully initialize the elements, e.g. if 'e' had another member without a default initializer? Does truncation first mean we lose the zero-initialization of such a member? We could probably still do the truncation, but clear the CONSTRUCTOR_NO_CLEARING flag on the element initializer. > Another complete fix would be to replace the uses of CONSTRUCTOR_APPEND_ELT > with get_or_insert_ctor_field, which does the right thing when a > matching constructor_elt already exists. But get_or_insert_ctor_field > was introduced only in GCC 10, . > > I'm not sure which fix we should go with in light of this being an > 8/9/10/11 regression.. > >> >> A side benefit of the approach taken by this patch is that constexpr >> evaluation of a simple VEC_INIT_EXPR for a high-dimensional array no >> longer scales exponentially with the number of dimensions. This is >> because after the RANGE_EXPR optimization, the CONSTRUCTOR for each >> array dimension now consists of a single constructor_elt (with index >> 0...max-1) instead of two constructor_elts (with indexes 0 and 1...max-1 >> respectively). This is verified by the second testcase below. >> >> Bootstrapped and regtested on x86_64-pc-linux-gnu. Does this look OK >> for trunk and perhaps for backports? >> >> gcc/cp/ChangeLog: >> >> PR c++/96282 >> * constexpr.c (cxx_eval_vec_init_1): Move the i == 0 test to the >> if statement that guards the RANGE_EXPR optimization. Consider the >> RANGE_EXPR optimization before we append the first element. >> Truncate ctx->ctor when performing the RANGE_EXPR optimization. >> Make the built RANGE_EXPR start at index 0 instead of 1. Don't >> call unshare_constructor. >> >> gcc/testsuite/ChangeLog: >> >> PR c++/96282 >> * g++.dg/cpp0x/constexpr-array26.C: New test. >> * g++.dg/cpp0x/constexpr-array27.C: New test. >> --- >> gcc/cp/constexpr.c | 36 ++++++++++--------- >> .../g++.dg/cpp0x/constexpr-array26.C | 13 +++++++ >> .../g++.dg/cpp0x/constexpr-array27.C | 18 ++++++++++ >> 3 files changed, 51 insertions(+), 16 deletions(-) >> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C >> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C >> >> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >> index b1c1d249c6e..3808a0713ba 100644 >> --- a/gcc/cp/constexpr.c >> +++ b/gcc/cp/constexpr.c >> @@ -4189,7 +4189,7 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, >> if (value_init || init == NULL_TREE) >> { >> eltinit = NULL_TREE; >> - reuse = i == 0; >> + reuse = true; >> } >> else >> eltinit = cp_build_array_ref (input_location, init, idx, complain); >> @@ -4206,7 +4206,7 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, >> return ctx->ctor; >> eltinit = cxx_eval_constant_expression (&new_ctx, init, lval, >> non_constant_p, overflow_p); >> - reuse = i == 0; >> + reuse = true; >> } >> else >> { >> @@ -4222,33 +4222,37 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, >> } >> if (*non_constant_p && !ctx->quiet) >> break; >> - if (new_ctx.ctor != ctx->ctor) >> - { >> - /* We appended this element above; update the value. */ >> - gcc_assert ((*p)->last().index == idx); >> - (*p)->last().value = eltinit; >> - } >> - 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 >> + if (i == 0 >> + && reuse >> && (eltinit == NULL_TREE >> || (initializer_constant_valid_p (eltinit, TREE_TYPE (eltinit)) >> == null_pointer_node))) >> { >> if (new_ctx.ctor != ctx->ctor) >> eltinit = new_ctx.ctor; >> - tree range = build2 (RANGE_EXPR, size_type_node, >> - build_int_cst (size_type_node, 1), >> - build_int_cst (size_type_node, max - 1)); >> - CONSTRUCTOR_APPEND_ELT (*p, range, unshare_constructor (eltinit)); >> + vec_safe_truncate (*p, 0); >> + if (max > 1) >> + idx = build2 (RANGE_EXPR, size_type_node, >> + build_int_cst (size_type_node, 0), >> + build_int_cst (size_type_node, max - 1)); >> + CONSTRUCTOR_APPEND_ELT (*p, idx, eltinit); >> break; >> } >> else if (i == 0) >> vec_safe_reserve (*p, max); >> + >> + if (new_ctx.ctor != ctx->ctor) >> + { >> + /* We appended this element above; update the value. */ >> + gcc_assert ((*p)->last().index == idx); >> + (*p)->last().value = eltinit; >> + } >> + else >> + CONSTRUCTOR_APPEND_ELT (*p, idx, eltinit); >> } >> >> if (!*non_constant_p) >> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C >> new file mode 100644 >> index 00000000000..274f55a88bf >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C >> @@ -0,0 +1,13 @@ >> +// PR c++/96282 >> +// { dg-do compile { target c++11 } } >> + >> +struct e { bool v = true; }; >> + >> +template<int N> >> +struct b { e m[N]; }; >> + >> +template<int N> >> +struct t : b<N> { constexpr t() : b<N>() {} }; >> + >> +constexpr t<1> h1; >> +constexpr t<42> h2; >> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C >> new file mode 100644 >> index 00000000000..a5ce3f7be08 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C >> @@ -0,0 +1,18 @@ >> +// Verify that default initialization an array of aggregates within an aggregate >> +// does not scale exponentially with the number of dimensions. >> + >> +// { dg-do compile { target c++11 } } >> +// Pass -fsyntax-only to stress only the performance of the frontend. >> +// { dg-additional-options "-fsyntax-only" } >> + >> +struct A >> +{ >> + int a = 42; >> +}; >> + >> +struct B >> +{ >> + A b[2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2]; >> +}; >> + >> +constexpr B c; >> -- >> 2.28.0.89.g85b4e0a6dc >> >> >
On Mon, 3 Aug 2020, Jason Merrill wrote: > On 8/3/20 8:53 AM, Patrick Palka wrote: > > On Mon, 3 Aug 2020, Patrick Palka wrote: > > > > > In the first testcase below, expand_aggr_init_1 sets up t's default > > > constructor such that the ctor first zero-initializes the entire base b, > > > followed by calling b's default constructor, the latter of which just > > > default-initializes the array member b::m via a VEC_INIT_EXPR. > > > > > > So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is > > > nonempty due to the prior zero-initialization, and we proceed in > > > cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor > > > without first checking if a matching constructor_elt already exists. > > > This leads to ctx->ctor having two matching constructor_elts for each > > > index. > > > > > > This patch partially fixes this issue by making the RANGE_EXPR > > > optimization in cxx_eval_vec_init truncate ctx->ctor before adding the > > > single RANGE_EXPR constructor_elt. This isn't a complete fix because > > > the RANGE_EXPR optimization applies only when the constant initializer > > > is relocatable, so whenever it's not relocatable we can still build up > > > an invalid CONSTRUCTOR, e.g. if in the first testcase we add an NSDMI > > > such as 'e *p = this;' to struct e, then the ICE still occurs even with > > > this patch. > > > > A complete but more risky one-line fix would be to always truncate > > ctx->ctor beforehand, not just when the RANGE_EXPR optimization applies. > > If it's true that the initializer of a VEC_INIT_EXPR can't observe the > > previous elements of the target array, then it should be safe to always > > truncate I think? > > What if default-initialization of the array element type doesn't fully > initialize the elements, e.g. if 'e' had another member without a default > initializer? Does truncation first mean we lose the zero-initialization of > such a member? Hmm, it looks like we would lose the zero-initialization of such a member with or without truncation first (so with any one of the three proposed fixes). I think it's because the evaluation loop in cxx_eval_vec_init disregards each element's prior (zero-initialized) state. > > We could probably still do the truncation, but clear the > CONSTRUCTOR_NO_CLEARING flag on the element initializer. Ah, this seems to work well. Like this? -- >8 -- Subject: [PATCH] c++: cxx_eval_vec_init after zero initialization [PR96282] In the first testcase below, expand_aggr_init_1 sets up t's default constructor such that the ctor first zero-initializes the entire base b, followed by calling b's default constructor, the latter of which just default-initializes the array member b::m via a VEC_INIT_EXPR. So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is nonempty due to the prior zero-initialization, and we proceed in cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor without first checking if a matching constructor_elt already exists. This leads to ctx->ctor having two matching constructor_elts for each index. This patch fixes this issue by truncating a zero-initialized array object in cxx_eval_vec_init_1 before we begin appending default-initialized array elements to it. Since default-initialization may leave parts of the element type unitialized, we also preserve the array's prior zero-initialized state by clearing CONSTRUCTOR_NO_CLEARING on each appended element initializers. gcc/cp/ChangeLog: PR c++/96282 * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and then clear CONSTRUCTOR_NO_CLEARING on each appended element initializer if we're default-initializing a previously zero-initialized array object. gcc/testsuite/ChangeLog: PR c++/96282 * g++.dg/cpp0x/constexpr-array26.C: New test. * g++.dg/cpp0x/constexpr-array27.C: New test. * g++.dg/cpp2a/constexpr-init18.C: New test. --- gcc/cp/constexpr.c | 17 ++++++++++++++++- gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 ++++++++++++++++ 4 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index b1c1d249c6e..706bef323b2 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -4171,6 +4171,17 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, pre_init = true; } + bool zero_initialized_p = false; + if ((pre_init || value_init || !init) && initializer_zerop (ctx->ctor)) + { + /* We're default-initializing an array object that had been + zero-initialized earlier. We'll preserve this prior state + by clearing CONSTRUCTOR_NO_CLEARING on each of its element + initializers. */ + zero_initialized_p = true; + vec_safe_truncate (*p, 0); + } + tree nelts = get_array_or_vector_nelts (ctx, atype, non_constant_p, overflow_p); unsigned HOST_WIDE_INT max = tree_to_uhwi (nelts); @@ -4182,7 +4193,11 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, constexpr_ctx new_ctx; init_subob_ctx (ctx, new_ctx, idx, pre_init ? init : elttype); if (new_ctx.ctor != ctx->ctor) - CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); + { + if (zero_initialized_p) + CONSTRUCTOR_NO_CLEARING (new_ctx.ctor) = false; + CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); + } if (TREE_CODE (elttype) == ARRAY_TYPE) { /* A multidimensional array; recurse. */ diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C new file mode 100644 index 00000000000..274f55a88bf --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C @@ -0,0 +1,13 @@ +// PR c++/96282 +// { dg-do compile { target c++11 } } + +struct e { bool v = true; }; + +template<int N> +struct b { e m[N]; }; + +template<int N> +struct t : b<N> { constexpr t() : b<N>() {} }; + +constexpr t<1> h1; +constexpr t<42> h2; diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C new file mode 100644 index 00000000000..bd5dd58d1ab --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C @@ -0,0 +1,13 @@ +// PR c++/96282 +// { dg-do compile { target c++11 } } + +struct e { bool v = true; e *p = this; }; + +template<int N> +struct b { e m[N]; }; + +template<int N> +struct t : b<N> { constexpr t() : b<N>() {} }; + +constexpr t<1> h1; +constexpr t<42> h2; diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C new file mode 100644 index 00000000000..47c15edfc73 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C @@ -0,0 +1,16 @@ +// PR c++/96282 +// { dg-do compile { target c++20 } } + +struct e { bool v = true; bool w; }; + +template<int N> +struct b { e m[N]; }; + +template<int N> +struct t : b<N> { constexpr t() : b<N>() {} }; + +constexpr t<1> h1; +static_assert(h1.m[0].w == false); + +constexpr t<42> h2; +static_assert(h2.m[17].w == false);
On 8/3/20 2:45 PM, Patrick Palka wrote: > On Mon, 3 Aug 2020, Jason Merrill wrote: > >> On 8/3/20 8:53 AM, Patrick Palka wrote: >>> On Mon, 3 Aug 2020, Patrick Palka wrote: >>> >>>> In the first testcase below, expand_aggr_init_1 sets up t's default >>>> constructor such that the ctor first zero-initializes the entire base b, >>>> followed by calling b's default constructor, the latter of which just >>>> default-initializes the array member b::m via a VEC_INIT_EXPR. >>>> >>>> So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is >>>> nonempty due to the prior zero-initialization, and we proceed in >>>> cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor >>>> without first checking if a matching constructor_elt already exists. >>>> This leads to ctx->ctor having two matching constructor_elts for each >>>> index. >>>> >>>> This patch partially fixes this issue by making the RANGE_EXPR >>>> optimization in cxx_eval_vec_init truncate ctx->ctor before adding the >>>> single RANGE_EXPR constructor_elt. This isn't a complete fix because >>>> the RANGE_EXPR optimization applies only when the constant initializer >>>> is relocatable, so whenever it's not relocatable we can still build up >>>> an invalid CONSTRUCTOR, e.g. if in the first testcase we add an NSDMI >>>> such as 'e *p = this;' to struct e, then the ICE still occurs even with >>>> this patch. >>> >>> A complete but more risky one-line fix would be to always truncate >>> ctx->ctor beforehand, not just when the RANGE_EXPR optimization applies. >>> If it's true that the initializer of a VEC_INIT_EXPR can't observe the >>> previous elements of the target array, then it should be safe to always >>> truncate I think? >> >> What if default-initialization of the array element type doesn't fully >> initialize the elements, e.g. if 'e' had another member without a default >> initializer? Does truncation first mean we lose the zero-initialization of >> such a member? > > Hmm, it looks like we would lose the zero-initialization of such a > member with or without truncation first (so with any one of the three > proposed fixes). I think it's because the evaluation loop in > cxx_eval_vec_init disregards each element's prior (zero-initialized) > state. > >> >> We could probably still do the truncation, but clear the >> CONSTRUCTOR_NO_CLEARING flag on the element initializer. > > Ah, this seems to work well. Like this? > > -- >8 -- > > Subject: [PATCH] c++: cxx_eval_vec_init after zero initialization [PR96282] > > In the first testcase below, expand_aggr_init_1 sets up t's default > constructor such that the ctor first zero-initializes the entire base b, > followed by calling b's default constructor, the latter of which just > default-initializes the array member b::m via a VEC_INIT_EXPR. > > So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is > nonempty due to the prior zero-initialization, and we proceed in > cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor > without first checking if a matching constructor_elt already exists. > This leads to ctx->ctor having two matching constructor_elts for each > index. > > This patch fixes this issue by truncating a zero-initialized array > object in cxx_eval_vec_init_1 before we begin appending default-initialized > array elements to it. Since default-initialization may leave parts of > the element type unitialized, we also preserve the array's prior > zero-initialized state by clearing CONSTRUCTOR_NO_CLEARING on each > appended element initializers. > > gcc/cp/ChangeLog: > > PR c++/96282 > * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and > then clear CONSTRUCTOR_NO_CLEARING on each appended element > initializer if we're default-initializing a previously > zero-initialized array object. > > gcc/testsuite/ChangeLog: > > PR c++/96282 > * g++.dg/cpp0x/constexpr-array26.C: New test. > * g++.dg/cpp0x/constexpr-array27.C: New test. > * g++.dg/cpp2a/constexpr-init18.C: New test. > --- > gcc/cp/constexpr.c | 17 ++++++++++++++++- > gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ > gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ > gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 ++++++++++++++++ > 4 files changed, 58 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index b1c1d249c6e..706bef323b2 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -4171,6 +4171,17 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, > pre_init = true; > } > > + bool zero_initialized_p = false; > + if ((pre_init || value_init || !init) && initializer_zerop (ctx->ctor)) Does initializer_zerop capture the difference between a default-initialized or value-initialized containing object? I would expect it to be true for both. Maybe check CONSTRUCTOR_NO_CLEARING (ctx->ctor) instead? This all seems parallel to the no_zero_init code in cxx_eval_store_expression. > + { > + /* We're default-initializing an array object that had been > + zero-initialized earlier. We'll preserve this prior state > + by clearing CONSTRUCTOR_NO_CLEARING on each of its element > + initializers. */ > + zero_initialized_p = true; > + vec_safe_truncate (*p, 0); > + } > + > tree nelts = get_array_or_vector_nelts (ctx, atype, non_constant_p, > overflow_p); > unsigned HOST_WIDE_INT max = tree_to_uhwi (nelts); > @@ -4182,7 +4193,11 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, > constexpr_ctx new_ctx; > init_subob_ctx (ctx, new_ctx, idx, pre_init ? init : elttype); > if (new_ctx.ctor != ctx->ctor) > - CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); > + { > + if (zero_initialized_p) > + CONSTRUCTOR_NO_CLEARING (new_ctx.ctor) = false; > + CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); > + } > if (TREE_CODE (elttype) == ARRAY_TYPE) > { > /* A multidimensional array; recurse. */ > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > new file mode 100644 > index 00000000000..274f55a88bf > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > @@ -0,0 +1,13 @@ > +// PR c++/96282 > +// { dg-do compile { target c++11 } } > + > +struct e { bool v = true; }; > + > +template<int N> > +struct b { e m[N]; }; > + > +template<int N> > +struct t : b<N> { constexpr t() : b<N>() {} }; > + > +constexpr t<1> h1; > +constexpr t<42> h2; > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > new file mode 100644 > index 00000000000..bd5dd58d1ab > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > @@ -0,0 +1,13 @@ > +// PR c++/96282 > +// { dg-do compile { target c++11 } } > + > +struct e { bool v = true; e *p = this; }; > + > +template<int N> > +struct b { e m[N]; }; > + > +template<int N> > +struct t : b<N> { constexpr t() : b<N>() {} }; > + > +constexpr t<1> h1; > +constexpr t<42> h2; > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > new file mode 100644 > index 00000000000..47c15edfc73 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > @@ -0,0 +1,16 @@ > +// PR c++/96282 > +// { dg-do compile { target c++20 } } > + > +struct e { bool v = true; bool w; }; > + > +template<int N> > +struct b { e m[N]; }; > + > +template<int N> > +struct t : b<N> { constexpr t() : b<N>() {} }; > + > +constexpr t<1> h1; > +static_assert(h1.m[0].w == false); > + > +constexpr t<42> h2; > +static_assert(h2.m[17].w == false); >
On Mon, 3 Aug 2020, Jason Merrill wrote: > On 8/3/20 2:45 PM, Patrick Palka wrote: > > On Mon, 3 Aug 2020, Jason Merrill wrote: > > > > > On 8/3/20 8:53 AM, Patrick Palka wrote: > > > > On Mon, 3 Aug 2020, Patrick Palka wrote: > > > > > > > > > In the first testcase below, expand_aggr_init_1 sets up t's default > > > > > constructor such that the ctor first zero-initializes the entire base > > > > > b, > > > > > followed by calling b's default constructor, the latter of which just > > > > > default-initializes the array member b::m via a VEC_INIT_EXPR. > > > > > > > > > > So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor > > > > > is > > > > > nonempty due to the prior zero-initialization, and we proceed in > > > > > cxx_eval_vec_init to append new constructor_elts to the end of > > > > > ctx->ctor > > > > > without first checking if a matching constructor_elt already exists. > > > > > This leads to ctx->ctor having two matching constructor_elts for each > > > > > index. > > > > > > > > > > This patch partially fixes this issue by making the RANGE_EXPR > > > > > optimization in cxx_eval_vec_init truncate ctx->ctor before adding the > > > > > single RANGE_EXPR constructor_elt. This isn't a complete fix because > > > > > the RANGE_EXPR optimization applies only when the constant initializer > > > > > is relocatable, so whenever it's not relocatable we can still build up > > > > > an invalid CONSTRUCTOR, e.g. if in the first testcase we add an NSDMI > > > > > such as 'e *p = this;' to struct e, then the ICE still occurs even > > > > > with > > > > > this patch. > > > > > > > > A complete but more risky one-line fix would be to always truncate > > > > ctx->ctor beforehand, not just when the RANGE_EXPR optimization applies. > > > > If it's true that the initializer of a VEC_INIT_EXPR can't observe the > > > > previous elements of the target array, then it should be safe to always > > > > truncate I think? > > > > > > What if default-initialization of the array element type doesn't fully > > > initialize the elements, e.g. if 'e' had another member without a default > > > initializer? Does truncation first mean we lose the zero-initialization > > > of > > > such a member? > > > > Hmm, it looks like we would lose the zero-initialization of such a > > member with or without truncation first (so with any one of the three > > proposed fixes). I think it's because the evaluation loop in > > cxx_eval_vec_init disregards each element's prior (zero-initialized) > > state. > > > > > > > > We could probably still do the truncation, but clear the > > > CONSTRUCTOR_NO_CLEARING flag on the element initializer. > > > > Ah, this seems to work well. Like this? > > > > -- >8 -- > > > > Subject: [PATCH] c++: cxx_eval_vec_init after zero initialization [PR96282] > > > > In the first testcase below, expand_aggr_init_1 sets up t's default > > constructor such that the ctor first zero-initializes the entire base b, > > followed by calling b's default constructor, the latter of which just > > default-initializes the array member b::m via a VEC_INIT_EXPR. > > > > So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is > > nonempty due to the prior zero-initialization, and we proceed in > > cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor > > without first checking if a matching constructor_elt already exists. > > This leads to ctx->ctor having two matching constructor_elts for each > > index. > > > > This patch fixes this issue by truncating a zero-initialized array > > object in cxx_eval_vec_init_1 before we begin appending default-initialized > > array elements to it. Since default-initialization may leave parts of > > the element type unitialized, we also preserve the array's prior > > zero-initialized state by clearing CONSTRUCTOR_NO_CLEARING on each > > appended element initializers. > > > > gcc/cp/ChangeLog: > > > > PR c++/96282 > > * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and > > then clear CONSTRUCTOR_NO_CLEARING on each appended element > > initializer if we're default-initializing a previously > > zero-initialized array object. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/96282 > > * g++.dg/cpp0x/constexpr-array26.C: New test. > > * g++.dg/cpp0x/constexpr-array27.C: New test. > > * g++.dg/cpp2a/constexpr-init18.C: New test. > > --- > > gcc/cp/constexpr.c | 17 ++++++++++++++++- > > gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ > > gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ > > gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 ++++++++++++++++ > > 4 files changed, 58 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > > index b1c1d249c6e..706bef323b2 100644 > > --- a/gcc/cp/constexpr.c > > +++ b/gcc/cp/constexpr.c > > @@ -4171,6 +4171,17 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree > > atype, tree init, > > pre_init = true; > > } > > + bool zero_initialized_p = false; > > + if ((pre_init || value_init || !init) && initializer_zerop (ctx->ctor)) > > Does initializer_zerop capture the difference between a default-initialized or > value-initialized containing object? I would expect it to be true for both. > Maybe check CONSTRUCTOR_NO_CLEARING (ctx->ctor) instead? D'oh, indeed it looks like initializer_zerop is not what we want here. > > This all seems parallel to the no_zero_init code in cxx_eval_store_expression. I am testing the following, which inspects CONSTRUCTOR_NO_CLEARING and also beefs up the tests to verify that we handle the multidimensional array case correctly. -- >8 -- Subject: [PATCH] c++: cxx_eval_vec_init after zero initialization [PR96282] In the first testcase below, expand_aggr_init_1 sets up t's default constructor such that the ctor first zero-initializes the entire base b, followed by calling b's default constructor, the latter of which just default-initializes the array member b::m via a VEC_INIT_EXPR. So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is nonempty due to the prior zero-initialization, and we proceed in cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor without first checking if a matching constructor_elt already exists. This leads to ctx->ctor having two matching constructor_elts for each index. This patch fixes this issue by truncating a zeroed out array CONSTRUCTOR in cxx_eval_vec_init_1 before we begin appending array elements to it. We preserve its zeroed out state during evaluation by clearing CONSTRUCTOR_NO_CLEARING on each new appended element. gcc/cp/ChangeLog: PR c++/96282 * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and then clear CONSTRUCTOR_NO_CLEARING on each appended element initializer if we're initializing a previously zeroed out array object. gcc/testsuite/ChangeLog: PR c++/96282 * g++.dg/cpp0x/constexpr-array26.C: New test. * g++.dg/cpp0x/constexpr-array27.C: New test. * g++.dg/cpp2a/constexpr-init18.C: New test. --- gcc/cp/constexpr.c | 18 +++++++++++++++++- gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 ++++++++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index b1c1d249c6e..364630f6333 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -4171,6 +4171,18 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, pre_init = true; } + bool zeroed_out = false; + if ((pre_init || value_init || !init) + && !CONSTRUCTOR_NO_CLEARING (ctx->ctor)) + { + /* We're initializing an array object that had been zeroed out + earlier. Truncate ctx->ctor and preserve its prior state by + propagating CONSTRUCTOR_NO_CLEARING to each of the element + initializers we append to it. */ + zeroed_out = true; + vec_safe_truncate (*p, 0); + } + tree nelts = get_array_or_vector_nelts (ctx, atype, non_constant_p, overflow_p); unsigned HOST_WIDE_INT max = tree_to_uhwi (nelts); @@ -4182,7 +4194,11 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, constexpr_ctx new_ctx; init_subob_ctx (ctx, new_ctx, idx, pre_init ? init : elttype); if (new_ctx.ctor != ctx->ctor) - CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); + { + if (zeroed_out) + CONSTRUCTOR_NO_CLEARING (new_ctx.ctor) = false; + CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); + } if (TREE_CODE (elttype) == ARRAY_TYPE) { /* A multidimensional array; recurse. */ diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C new file mode 100644 index 00000000000..274f55a88bf --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C @@ -0,0 +1,13 @@ +// PR c++/96282 +// { dg-do compile { target c++11 } } + +struct e { bool v = true; }; + +template<int N> +struct b { e m[N]; }; + +template<int N> +struct t : b<N> { constexpr t() : b<N>() {} }; + +constexpr t<1> h1; +constexpr t<42> h2; diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C new file mode 100644 index 00000000000..1234caef31d --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C @@ -0,0 +1,13 @@ +// PR c++/96282 +// { dg-do compile { target c++11 } } + +struct e { bool v = true; e *p = this; }; + +template<int N> +struct b { e m[N][N]; }; + +template<int N> +struct t : b<N> { constexpr t() : b<N>() {} }; + +constexpr t<1> h1; +constexpr t<42> h2; diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C new file mode 100644 index 00000000000..47ad11f2290 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C @@ -0,0 +1,16 @@ +// PR c++/96282 +// { dg-do compile { target c++20 } } + +struct e { bool v = true; bool w; }; + +template<int N> +struct b { e m[N][N]; }; + +template<int N> +struct t : b<N> { constexpr t() : b<N>() {} }; + +constexpr t<1> h1; +static_assert(h1.m[0][0].w == false); + +constexpr t<42> h2; +static_assert(h2.m[17][17].w == false);
On Mon, 3 Aug 2020, Patrick Palka wrote: > On Mon, 3 Aug 2020, Jason Merrill wrote: > > > On 8/3/20 2:45 PM, Patrick Palka wrote: > > > On Mon, 3 Aug 2020, Jason Merrill wrote: > > > > > > > On 8/3/20 8:53 AM, Patrick Palka wrote: > > > > > On Mon, 3 Aug 2020, Patrick Palka wrote: > > > > > > > > > > > In the first testcase below, expand_aggr_init_1 sets up t's default > > > > > > constructor such that the ctor first zero-initializes the entire base > > > > > > b, > > > > > > followed by calling b's default constructor, the latter of which just > > > > > > default-initializes the array member b::m via a VEC_INIT_EXPR. > > > > > > > > > > > > So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor > > > > > > is > > > > > > nonempty due to the prior zero-initialization, and we proceed in > > > > > > cxx_eval_vec_init to append new constructor_elts to the end of > > > > > > ctx->ctor > > > > > > without first checking if a matching constructor_elt already exists. > > > > > > This leads to ctx->ctor having two matching constructor_elts for each > > > > > > index. > > > > > > > > > > > > This patch partially fixes this issue by making the RANGE_EXPR > > > > > > optimization in cxx_eval_vec_init truncate ctx->ctor before adding the > > > > > > single RANGE_EXPR constructor_elt. This isn't a complete fix because > > > > > > the RANGE_EXPR optimization applies only when the constant initializer > > > > > > is relocatable, so whenever it's not relocatable we can still build up > > > > > > an invalid CONSTRUCTOR, e.g. if in the first testcase we add an NSDMI > > > > > > such as 'e *p = this;' to struct e, then the ICE still occurs even > > > > > > with > > > > > > this patch. > > > > > > > > > > A complete but more risky one-line fix would be to always truncate > > > > > ctx->ctor beforehand, not just when the RANGE_EXPR optimization applies. > > > > > If it's true that the initializer of a VEC_INIT_EXPR can't observe the > > > > > previous elements of the target array, then it should be safe to always > > > > > truncate I think? > > > > > > > > What if default-initialization of the array element type doesn't fully > > > > initialize the elements, e.g. if 'e' had another member without a default > > > > initializer? Does truncation first mean we lose the zero-initialization > > > > of > > > > such a member? > > > > > > Hmm, it looks like we would lose the zero-initialization of such a > > > member with or without truncation first (so with any one of the three > > > proposed fixes). I think it's because the evaluation loop in > > > cxx_eval_vec_init disregards each element's prior (zero-initialized) > > > state. > > > > > > > > > > > We could probably still do the truncation, but clear the > > > > CONSTRUCTOR_NO_CLEARING flag on the element initializer. > > > > > > Ah, this seems to work well. Like this? > > > > > > -- >8 -- > > > > > > Subject: [PATCH] c++: cxx_eval_vec_init after zero initialization [PR96282] > > > > > > In the first testcase below, expand_aggr_init_1 sets up t's default > > > constructor such that the ctor first zero-initializes the entire base b, > > > followed by calling b's default constructor, the latter of which just > > > default-initializes the array member b::m via a VEC_INIT_EXPR. > > > > > > So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is > > > nonempty due to the prior zero-initialization, and we proceed in > > > cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor > > > without first checking if a matching constructor_elt already exists. > > > This leads to ctx->ctor having two matching constructor_elts for each > > > index. > > > > > > This patch fixes this issue by truncating a zero-initialized array > > > object in cxx_eval_vec_init_1 before we begin appending default-initialized > > > array elements to it. Since default-initialization may leave parts of > > > the element type unitialized, we also preserve the array's prior > > > zero-initialized state by clearing CONSTRUCTOR_NO_CLEARING on each > > > appended element initializers. > > > > > > gcc/cp/ChangeLog: > > > > > > PR c++/96282 > > > * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and > > > then clear CONSTRUCTOR_NO_CLEARING on each appended element > > > initializer if we're default-initializing a previously > > > zero-initialized array object. > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR c++/96282 > > > * g++.dg/cpp0x/constexpr-array26.C: New test. > > > * g++.dg/cpp0x/constexpr-array27.C: New test. > > > * g++.dg/cpp2a/constexpr-init18.C: New test. > > > --- > > > gcc/cp/constexpr.c | 17 ++++++++++++++++- > > > gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ > > > gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ > > > gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 ++++++++++++++++ > > > 4 files changed, 58 insertions(+), 1 deletion(-) > > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > > > > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > > > index b1c1d249c6e..706bef323b2 100644 > > > --- a/gcc/cp/constexpr.c > > > +++ b/gcc/cp/constexpr.c > > > @@ -4171,6 +4171,17 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree > > > atype, tree init, > > > pre_init = true; > > > } > > > + bool zero_initialized_p = false; > > > + if ((pre_init || value_init || !init) && initializer_zerop (ctx->ctor)) > > > > Does initializer_zerop capture the difference between a default-initialized or > > value-initialized containing object? I would expect it to be true for both. > > Maybe check CONSTRUCTOR_NO_CLEARING (ctx->ctor) instead? > > D'oh, indeed it looks like initializer_zerop is not what we want here. Hmm, might we need to check both? !CONSTRUCTOR_NO_CLEARING tells us about the omitted initializers, but it seems we'd still need to test if the explicit initializers are zero. > > > > > This all seems parallel to the no_zero_init code in cxx_eval_store_expression. > > I am testing the following, which inspects CONSTRUCTOR_NO_CLEARING and > also beefs up the tests to verify that we handle the multidimensional > array case correctly. > > -- >8 -- > > Subject: [PATCH] c++: cxx_eval_vec_init after zero initialization [PR96282] > > In the first testcase below, expand_aggr_init_1 sets up t's default > constructor such that the ctor first zero-initializes the entire base b, > followed by calling b's default constructor, the latter of which just > default-initializes the array member b::m via a VEC_INIT_EXPR. > > So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is > nonempty due to the prior zero-initialization, and we proceed in > cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor > without first checking if a matching constructor_elt already exists. > This leads to ctx->ctor having two matching constructor_elts for each > index. > > This patch fixes this issue by truncating a zeroed out array CONSTRUCTOR > in cxx_eval_vec_init_1 before we begin appending array elements to it. > We preserve its zeroed out state during evaluation by clearing > CONSTRUCTOR_NO_CLEARING on each new appended element. > > gcc/cp/ChangeLog: > > PR c++/96282 > * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and > then clear CONSTRUCTOR_NO_CLEARING on each appended element > initializer if we're initializing a previously zeroed out > array object. > > gcc/testsuite/ChangeLog: > > PR c++/96282 > * g++.dg/cpp0x/constexpr-array26.C: New test. > * g++.dg/cpp0x/constexpr-array27.C: New test. > * g++.dg/cpp2a/constexpr-init18.C: New test. > --- > gcc/cp/constexpr.c | 18 +++++++++++++++++- > gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ > gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ > gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 ++++++++++++++++ > 4 files changed, 59 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index b1c1d249c6e..364630f6333 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -4171,6 +4171,18 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, > pre_init = true; > } > > + bool zeroed_out = false; > + if ((pre_init || value_init || !init) > + && !CONSTRUCTOR_NO_CLEARING (ctx->ctor)) > + { > + /* We're initializing an array object that had been zeroed out > + earlier. Truncate ctx->ctor and preserve its prior state by > + propagating CONSTRUCTOR_NO_CLEARING to each of the element > + initializers we append to it. */ > + zeroed_out = true; > + vec_safe_truncate (*p, 0); > + } > + > tree nelts = get_array_or_vector_nelts (ctx, atype, non_constant_p, > overflow_p); > unsigned HOST_WIDE_INT max = tree_to_uhwi (nelts); > @@ -4182,7 +4194,11 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, > constexpr_ctx new_ctx; > init_subob_ctx (ctx, new_ctx, idx, pre_init ? init : elttype); > if (new_ctx.ctor != ctx->ctor) > - CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); > + { > + if (zeroed_out) > + CONSTRUCTOR_NO_CLEARING (new_ctx.ctor) = false; > + CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); > + } > if (TREE_CODE (elttype) == ARRAY_TYPE) > { > /* A multidimensional array; recurse. */ > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > new file mode 100644 > index 00000000000..274f55a88bf > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > @@ -0,0 +1,13 @@ > +// PR c++/96282 > +// { dg-do compile { target c++11 } } > + > +struct e { bool v = true; }; > + > +template<int N> > +struct b { e m[N]; }; > + > +template<int N> > +struct t : b<N> { constexpr t() : b<N>() {} }; > + > +constexpr t<1> h1; > +constexpr t<42> h2; > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > new file mode 100644 > index 00000000000..1234caef31d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > @@ -0,0 +1,13 @@ > +// PR c++/96282 > +// { dg-do compile { target c++11 } } > + > +struct e { bool v = true; e *p = this; }; > + > +template<int N> > +struct b { e m[N][N]; }; > + > +template<int N> > +struct t : b<N> { constexpr t() : b<N>() {} }; > + > +constexpr t<1> h1; > +constexpr t<42> h2; > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > new file mode 100644 > index 00000000000..47ad11f2290 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > @@ -0,0 +1,16 @@ > +// PR c++/96282 > +// { dg-do compile { target c++20 } } > + > +struct e { bool v = true; bool w; }; > + > +template<int N> > +struct b { e m[N][N]; }; > + > +template<int N> > +struct t : b<N> { constexpr t() : b<N>() {} }; > + > +constexpr t<1> h1; > +static_assert(h1.m[0][0].w == false); > + > +constexpr t<42> h2; > +static_assert(h2.m[17][17].w == false); > -- > 2.28.0.89.g85b4e0a6dc > >
On 8/4/20 9:45 AM, Patrick Palka wrote: > On Mon, 3 Aug 2020, Patrick Palka wrote: > >> On Mon, 3 Aug 2020, Jason Merrill wrote: >> >>> On 8/3/20 2:45 PM, Patrick Palka wrote: >>>> On Mon, 3 Aug 2020, Jason Merrill wrote: >>>> >>>>> On 8/3/20 8:53 AM, Patrick Palka wrote: >>>>>> On Mon, 3 Aug 2020, Patrick Palka wrote: >>>>>> >>>>>>> In the first testcase below, expand_aggr_init_1 sets up t's default >>>>>>> constructor such that the ctor first zero-initializes the entire base >>>>>>> b, >>>>>>> followed by calling b's default constructor, the latter of which just >>>>>>> default-initializes the array member b::m via a VEC_INIT_EXPR. >>>>>>> >>>>>>> So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor >>>>>>> is >>>>>>> nonempty due to the prior zero-initialization, and we proceed in >>>>>>> cxx_eval_vec_init to append new constructor_elts to the end of >>>>>>> ctx->ctor >>>>>>> without first checking if a matching constructor_elt already exists. >>>>>>> This leads to ctx->ctor having two matching constructor_elts for each >>>>>>> index. >>>>>>> >>>>>>> This patch partially fixes this issue by making the RANGE_EXPR >>>>>>> optimization in cxx_eval_vec_init truncate ctx->ctor before adding the >>>>>>> single RANGE_EXPR constructor_elt. This isn't a complete fix because >>>>>>> the RANGE_EXPR optimization applies only when the constant initializer >>>>>>> is relocatable, so whenever it's not relocatable we can still build up >>>>>>> an invalid CONSTRUCTOR, e.g. if in the first testcase we add an NSDMI >>>>>>> such as 'e *p = this;' to struct e, then the ICE still occurs even >>>>>>> with >>>>>>> this patch. >>>>>> >>>>>> A complete but more risky one-line fix would be to always truncate >>>>>> ctx->ctor beforehand, not just when the RANGE_EXPR optimization applies. >>>>>> If it's true that the initializer of a VEC_INIT_EXPR can't observe the >>>>>> previous elements of the target array, then it should be safe to always >>>>>> truncate I think? >>>>> >>>>> What if default-initialization of the array element type doesn't fully >>>>> initialize the elements, e.g. if 'e' had another member without a default >>>>> initializer? Does truncation first mean we lose the zero-initialization >>>>> of >>>>> such a member? >>>> >>>> Hmm, it looks like we would lose the zero-initialization of such a >>>> member with or without truncation first (so with any one of the three >>>> proposed fixes). I think it's because the evaluation loop in >>>> cxx_eval_vec_init disregards each element's prior (zero-initialized) >>>> state. >>>> >>>>> >>>>> We could probably still do the truncation, but clear the >>>>> CONSTRUCTOR_NO_CLEARING flag on the element initializer. >>>> >>>> Ah, this seems to work well. Like this? >>>> >>>> -- >8 -- >>>> >>>> Subject: [PATCH] c++: cxx_eval_vec_init after zero initialization [PR96282] >>>> >>>> In the first testcase below, expand_aggr_init_1 sets up t's default >>>> constructor such that the ctor first zero-initializes the entire base b, >>>> followed by calling b's default constructor, the latter of which just >>>> default-initializes the array member b::m via a VEC_INIT_EXPR. >>>> >>>> So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is >>>> nonempty due to the prior zero-initialization, and we proceed in >>>> cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor >>>> without first checking if a matching constructor_elt already exists. >>>> This leads to ctx->ctor having two matching constructor_elts for each >>>> index. >>>> >>>> This patch fixes this issue by truncating a zero-initialized array >>>> object in cxx_eval_vec_init_1 before we begin appending default-initialized >>>> array elements to it. Since default-initialization may leave parts of >>>> the element type unitialized, we also preserve the array's prior >>>> zero-initialized state by clearing CONSTRUCTOR_NO_CLEARING on each >>>> appended element initializers. >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> PR c++/96282 >>>> * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and >>>> then clear CONSTRUCTOR_NO_CLEARING on each appended element >>>> initializer if we're default-initializing a previously >>>> zero-initialized array object. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR c++/96282 >>>> * g++.dg/cpp0x/constexpr-array26.C: New test. >>>> * g++.dg/cpp0x/constexpr-array27.C: New test. >>>> * g++.dg/cpp2a/constexpr-init18.C: New test. >>>> --- >>>> gcc/cp/constexpr.c | 17 ++++++++++++++++- >>>> gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ >>>> gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ >>>> gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 ++++++++++++++++ >>>> 4 files changed, 58 insertions(+), 1 deletion(-) >>>> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C >>>> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C >>>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C >>>> >>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >>>> index b1c1d249c6e..706bef323b2 100644 >>>> --- a/gcc/cp/constexpr.c >>>> +++ b/gcc/cp/constexpr.c >>>> @@ -4171,6 +4171,17 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree >>>> atype, tree init, >>>> pre_init = true; >>>> } >>>> + bool zero_initialized_p = false; >>>> + if ((pre_init || value_init || !init) && initializer_zerop (ctx->ctor)) >>> >>> Does initializer_zerop capture the difference between a default-initialized or >>> value-initialized containing object? I would expect it to be true for both. >>> Maybe check CONSTRUCTOR_NO_CLEARING (ctx->ctor) instead? >> >> D'oh, indeed it looks like initializer_zerop is not what we want here. > > Hmm, might we need to check both? !CONSTRUCTOR_NO_CLEARING tells us > about the omitted initializers, but it seems we'd still need to test if > the explicit initializers are zero. We might check initializer_zerop as an assert; at this point the only previous initialization should be zero-initialization. What are you going for with (pre_init || value_init || !init)? All cases other than array copy? We shouldn't ever see zero-initialization before copy, so this test seems unnecessary. >>> >>> This all seems parallel to the no_zero_init code in cxx_eval_store_expression. >> >> I am testing the following, which inspects CONSTRUCTOR_NO_CLEARING and >> also beefs up the tests to verify that we handle the multidimensional >> array case correctly. >> >> -- >8 -- >> >> Subject: [PATCH] c++: cxx_eval_vec_init after zero initialization [PR96282] >> >> In the first testcase below, expand_aggr_init_1 sets up t's default >> constructor such that the ctor first zero-initializes the entire base b, >> followed by calling b's default constructor, the latter of which just >> default-initializes the array member b::m via a VEC_INIT_EXPR. >> >> So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is >> nonempty due to the prior zero-initialization, and we proceed in >> cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor >> without first checking if a matching constructor_elt already exists. >> This leads to ctx->ctor having two matching constructor_elts for each >> index. >> >> This patch fixes this issue by truncating a zeroed out array CONSTRUCTOR >> in cxx_eval_vec_init_1 before we begin appending array elements to it. >> We preserve its zeroed out state during evaluation by clearing >> CONSTRUCTOR_NO_CLEARING on each new appended element. >> >> gcc/cp/ChangeLog: >> >> PR c++/96282 >> * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and >> then clear CONSTRUCTOR_NO_CLEARING on each appended element >> initializer if we're initializing a previously zeroed out >> array object. >> >> gcc/testsuite/ChangeLog: >> >> PR c++/96282 >> * g++.dg/cpp0x/constexpr-array26.C: New test. >> * g++.dg/cpp0x/constexpr-array27.C: New test. >> * g++.dg/cpp2a/constexpr-init18.C: New test. >> --- >> gcc/cp/constexpr.c | 18 +++++++++++++++++- >> gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ >> gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ >> gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 ++++++++++++++++ >> 4 files changed, 59 insertions(+), 1 deletion(-) >> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C >> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C >> create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C >> >> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >> index b1c1d249c6e..364630f6333 100644 >> --- a/gcc/cp/constexpr.c >> +++ b/gcc/cp/constexpr.c >> @@ -4171,6 +4171,18 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, >> pre_init = true; >> } >> >> + bool zeroed_out = false; >> + if ((pre_init || value_init || !init) >> + && !CONSTRUCTOR_NO_CLEARING (ctx->ctor)) >> + { >> + /* We're initializing an array object that had been zeroed out >> + earlier. Truncate ctx->ctor and preserve its prior state by >> + propagating CONSTRUCTOR_NO_CLEARING to each of the element >> + initializers we append to it. */ >> + zeroed_out = true; >> + vec_safe_truncate (*p, 0); >> + } >> + >> tree nelts = get_array_or_vector_nelts (ctx, atype, non_constant_p, >> overflow_p); >> unsigned HOST_WIDE_INT max = tree_to_uhwi (nelts); >> @@ -4182,7 +4194,11 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, >> constexpr_ctx new_ctx; >> init_subob_ctx (ctx, new_ctx, idx, pre_init ? init : elttype); >> if (new_ctx.ctor != ctx->ctor) >> - CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); >> + { >> + if (zeroed_out) >> + CONSTRUCTOR_NO_CLEARING (new_ctx.ctor) = false; >> + CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); >> + } >> if (TREE_CODE (elttype) == ARRAY_TYPE) >> { >> /* A multidimensional array; recurse. */ >> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C >> new file mode 100644 >> index 00000000000..274f55a88bf >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C >> @@ -0,0 +1,13 @@ >> +// PR c++/96282 >> +// { dg-do compile { target c++11 } } >> + >> +struct e { bool v = true; }; >> + >> +template<int N> >> +struct b { e m[N]; }; >> + >> +template<int N> >> +struct t : b<N> { constexpr t() : b<N>() {} }; >> + >> +constexpr t<1> h1; >> +constexpr t<42> h2; >> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C >> new file mode 100644 >> index 00000000000..1234caef31d >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C >> @@ -0,0 +1,13 @@ >> +// PR c++/96282 >> +// { dg-do compile { target c++11 } } >> + >> +struct e { bool v = true; e *p = this; }; >> + >> +template<int N> >> +struct b { e m[N][N]; }; >> + >> +template<int N> >> +struct t : b<N> { constexpr t() : b<N>() {} }; >> + >> +constexpr t<1> h1; >> +constexpr t<42> h2; >> diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C >> new file mode 100644 >> index 00000000000..47ad11f2290 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C >> @@ -0,0 +1,16 @@ >> +// PR c++/96282 >> +// { dg-do compile { target c++20 } } >> + >> +struct e { bool v = true; bool w; }; >> + >> +template<int N> >> +struct b { e m[N][N]; }; >> + >> +template<int N> >> +struct t : b<N> { constexpr t() : b<N>() {} }; >> + >> +constexpr t<1> h1; >> +static_assert(h1.m[0][0].w == false); >> + >> +constexpr t<42> h2; >> +static_assert(h2.m[17][17].w == false); >> -- >> 2.28.0.89.g85b4e0a6dc >> >> >
On Tue, 4 Aug 2020, Jason Merrill wrote: > On 8/4/20 9:45 AM, Patrick Palka wrote: > > On Mon, 3 Aug 2020, Patrick Palka wrote: > > > > > On Mon, 3 Aug 2020, Jason Merrill wrote: > > > > > > > On 8/3/20 2:45 PM, Patrick Palka wrote: > > > > > On Mon, 3 Aug 2020, Jason Merrill wrote: > > > > > > > > > > > On 8/3/20 8:53 AM, Patrick Palka wrote: > > > > > > > On Mon, 3 Aug 2020, Patrick Palka wrote: > > > > > > > > > > > > > > > In the first testcase below, expand_aggr_init_1 sets up t's > > > > > > > > default > > > > > > > > constructor such that the ctor first zero-initializes the entire > > > > > > > > base > > > > > > > > b, > > > > > > > > followed by calling b's default constructor, the latter of which > > > > > > > > just > > > > > > > > default-initializes the array member b::m via a VEC_INIT_EXPR. > > > > > > > > > > > > > > > > So upon constexpr evaluation of this latter VEC_INIT_EXPR, > > > > > > > > ctx->ctor > > > > > > > > is > > > > > > > > nonempty due to the prior zero-initialization, and we proceed in > > > > > > > > cxx_eval_vec_init to append new constructor_elts to the end of > > > > > > > > ctx->ctor > > > > > > > > without first checking if a matching constructor_elt already > > > > > > > > exists. > > > > > > > > This leads to ctx->ctor having two matching constructor_elts for > > > > > > > > each > > > > > > > > index. > > > > > > > > > > > > > > > > This patch partially fixes this issue by making the RANGE_EXPR > > > > > > > > optimization in cxx_eval_vec_init truncate ctx->ctor before > > > > > > > > adding the > > > > > > > > single RANGE_EXPR constructor_elt. This isn't a complete fix > > > > > > > > because > > > > > > > > the RANGE_EXPR optimization applies only when the constant > > > > > > > > initializer > > > > > > > > is relocatable, so whenever it's not relocatable we can still > > > > > > > > build up > > > > > > > > an invalid CONSTRUCTOR, e.g. if in the first testcase we add an > > > > > > > > NSDMI > > > > > > > > such as 'e *p = this;' to struct e, then the ICE still occurs > > > > > > > > even > > > > > > > > with > > > > > > > > this patch. > > > > > > > > > > > > > > A complete but more risky one-line fix would be to always truncate > > > > > > > ctx->ctor beforehand, not just when the RANGE_EXPR optimization > > > > > > > applies. > > > > > > > If it's true that the initializer of a VEC_INIT_EXPR can't observe > > > > > > > the > > > > > > > previous elements of the target array, then it should be safe to > > > > > > > always > > > > > > > truncate I think? > > > > > > > > > > > > What if default-initialization of the array element type doesn't > > > > > > fully > > > > > > initialize the elements, e.g. if 'e' had another member without a > > > > > > default > > > > > > initializer? Does truncation first mean we lose the > > > > > > zero-initialization > > > > > > of > > > > > > such a member? > > > > > > > > > > Hmm, it looks like we would lose the zero-initialization of such a > > > > > member with or without truncation first (so with any one of the three > > > > > proposed fixes). I think it's because the evaluation loop in > > > > > cxx_eval_vec_init disregards each element's prior (zero-initialized) > > > > > state. > > > > > > > > > > > > > > > > > We could probably still do the truncation, but clear the > > > > > > CONSTRUCTOR_NO_CLEARING flag on the element initializer. > > > > > > > > > > Ah, this seems to work well. Like this? > > > > > > > > > > -- >8 -- > > > > > > > > > > Subject: [PATCH] c++: cxx_eval_vec_init after zero initialization > > > > > [PR96282] > > > > > > > > > > In the first testcase below, expand_aggr_init_1 sets up t's default > > > > > constructor such that the ctor first zero-initializes the entire base > > > > > b, > > > > > followed by calling b's default constructor, the latter of which just > > > > > default-initializes the array member b::m via a VEC_INIT_EXPR. > > > > > > > > > > So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor > > > > > is > > > > > nonempty due to the prior zero-initialization, and we proceed in > > > > > cxx_eval_vec_init to append new constructor_elts to the end of > > > > > ctx->ctor > > > > > without first checking if a matching constructor_elt already exists. > > > > > This leads to ctx->ctor having two matching constructor_elts for each > > > > > index. > > > > > > > > > > This patch fixes this issue by truncating a zero-initialized array > > > > > object in cxx_eval_vec_init_1 before we begin appending > > > > > default-initialized > > > > > array elements to it. Since default-initialization may leave parts of > > > > > the element type unitialized, we also preserve the array's prior > > > > > zero-initialized state by clearing CONSTRUCTOR_NO_CLEARING on each > > > > > appended element initializers. > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > PR c++/96282 > > > > > * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and > > > > > then clear CONSTRUCTOR_NO_CLEARING on each appended element > > > > > initializer if we're default-initializing a previously > > > > > zero-initialized array object. > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > PR c++/96282 > > > > > * g++.dg/cpp0x/constexpr-array26.C: New test. > > > > > * g++.dg/cpp0x/constexpr-array27.C: New test. > > > > > * g++.dg/cpp2a/constexpr-init18.C: New test. > > > > > --- > > > > > gcc/cp/constexpr.c | 17 > > > > > ++++++++++++++++- > > > > > gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ > > > > > gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ > > > > > gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 > > > > > ++++++++++++++++ > > > > > 4 files changed, 58 insertions(+), 1 deletion(-) > > > > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > > > > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > > > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > > > > > > > > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > > > > > index b1c1d249c6e..706bef323b2 100644 > > > > > --- a/gcc/cp/constexpr.c > > > > > +++ b/gcc/cp/constexpr.c > > > > > @@ -4171,6 +4171,17 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, > > > > > tree > > > > > atype, tree init, > > > > > pre_init = true; > > > > > } > > > > > + bool zero_initialized_p = false; > > > > > + if ((pre_init || value_init || !init) && initializer_zerop > > > > > (ctx->ctor)) > > > > > > > > Does initializer_zerop capture the difference between a > > > > default-initialized or > > > > value-initialized containing object? I would expect it to be true for > > > > both. > > > > Maybe check CONSTRUCTOR_NO_CLEARING (ctx->ctor) instead? > > > > > > D'oh, indeed it looks like initializer_zerop is not what we want here. > > > > Hmm, might we need to check both? !CONSTRUCTOR_NO_CLEARING tells us > > about the omitted initializers, but it seems we'd still need to test if > > the explicit initializers are zero. > > We might check initializer_zerop as an assert; at this point the only previous > initialization should be zero-initialization. Sounds good. I wasn't quite sure what we may or may not assume about the previous initializer. > > What are you going for with (pre_init || value_init || !init)? All cases > other than array copy? We shouldn't ever see zero-initialization before copy, > so this test seems unnecessary. Yeah, the intent was to make sure to include the recursive initialization case while excluding the array copy case. I suppose we can add this test as an assert too, but it doesn't seem that worthwhile. In the below I removed the pre_init || value_init || !init test and added the initializer_zerop test as an assert. Does it look OK to commit after bootstrap/regtest succeeds? -- >8 -- Subject: [PATCH] c++: cxx_eval_vec_init after zero-initialization [PR96282] In the first testcase below, expand_aggr_init_1 sets up t's default constructor such that the ctor first zero-initializes the entire base b, followed by calling b's default constructor, the latter of which just default-initializes the array member b::m via a VEC_INIT_EXPR. So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is nonempty due to the prior zero-initialization, and we proceed in cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor without first checking if a matching constructor_elt already exists. This leads to ctx->ctor having two matching constructor_elts for each index. This patch fixes this issue by truncating a zero-initialized array CONSTRUCTOR in cxx_eval_vec_init_1 before we begin appending array elements to it. We preserve its zeroed out state during evaluation by clearing CONSTRUCTOR_NO_CLEARING on each new appended element. gcc/cp/ChangeLog: PR c++/96282 * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and then clear CONSTRUCTOR_NO_CLEARING on each appended element initializer if we're initializing a previously zero-initialized array object. gcc/testsuite/ChangeLog: PR c++/96282 * g++.dg/cpp0x/constexpr-array26.C: New test. * g++.dg/cpp0x/constexpr-array27.C: New test. * g++.dg/cpp2a/constexpr-init18.C: New test. --- gcc/cp/constexpr.c | 18 +++++++++++++++++- gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 ++++++++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index b1c1d249c6e..bec888646ea 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -4171,6 +4171,18 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, pre_init = true; } + bool zeroed_out = false; + if (!CONSTRUCTOR_NO_CLEARING (ctx->ctor)) + { + /* We're initializing an array object that had been zero-initialized + earlier. Truncate ctx->ctor and preserve its zeroed state by + clearing CONSTRUCTOR_NO_CLEARING on each of the element + initializers we append to it. */ + gcc_assert (initializer_zerop (ctx->ctor)); + zeroed_out = true; + vec_safe_truncate (*p, 0); + } + tree nelts = get_array_or_vector_nelts (ctx, atype, non_constant_p, overflow_p); unsigned HOST_WIDE_INT max = tree_to_uhwi (nelts); @@ -4182,7 +4194,11 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, constexpr_ctx new_ctx; init_subob_ctx (ctx, new_ctx, idx, pre_init ? init : elttype); if (new_ctx.ctor != ctx->ctor) - CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); + { + if (zeroed_out) + CONSTRUCTOR_NO_CLEARING (new_ctx.ctor) = false; + CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); + } if (TREE_CODE (elttype) == ARRAY_TYPE) { /* A multidimensional array; recurse. */ diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C new file mode 100644 index 00000000000..274f55a88bf --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C @@ -0,0 +1,13 @@ +// PR c++/96282 +// { dg-do compile { target c++11 } } + +struct e { bool v = true; }; + +template<int N> +struct b { e m[N]; }; + +template<int N> +struct t : b<N> { constexpr t() : b<N>() {} }; + +constexpr t<1> h1; +constexpr t<42> h2; diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C new file mode 100644 index 00000000000..1234caef31d --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C @@ -0,0 +1,13 @@ +// PR c++/96282 +// { dg-do compile { target c++11 } } + +struct e { bool v = true; e *p = this; }; + +template<int N> +struct b { e m[N][N]; }; + +template<int N> +struct t : b<N> { constexpr t() : b<N>() {} }; + +constexpr t<1> h1; +constexpr t<42> h2; diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C new file mode 100644 index 00000000000..47ad11f2290 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C @@ -0,0 +1,16 @@ +// PR c++/96282 +// { dg-do compile { target c++20 } } + +struct e { bool v = true; bool w; }; + +template<int N> +struct b { e m[N][N]; }; + +template<int N> +struct t : b<N> { constexpr t() : b<N>() {} }; + +constexpr t<1> h1; +static_assert(h1.m[0][0].w == false); + +constexpr t<42> h2; +static_assert(h2.m[17][17].w == false);
On 8/4/20 2:31 PM, Patrick Palka wrote: > On Tue, 4 Aug 2020, Jason Merrill wrote: > >> On 8/4/20 9:45 AM, Patrick Palka wrote: >>> On Mon, 3 Aug 2020, Patrick Palka wrote: >>> >>>> On Mon, 3 Aug 2020, Jason Merrill wrote: >>>> >>>>> On 8/3/20 2:45 PM, Patrick Palka wrote: >>>>>> On Mon, 3 Aug 2020, Jason Merrill wrote: >>>>>> >>>>>>> On 8/3/20 8:53 AM, Patrick Palka wrote: >>>>>>>> On Mon, 3 Aug 2020, Patrick Palka wrote: >>>>>>>> >>>>>>>>> In the first testcase below, expand_aggr_init_1 sets up t's >>>>>>>>> default >>>>>>>>> constructor such that the ctor first zero-initializes the entire >>>>>>>>> base >>>>>>>>> b, >>>>>>>>> followed by calling b's default constructor, the latter of which >>>>>>>>> just >>>>>>>>> default-initializes the array member b::m via a VEC_INIT_EXPR. >>>>>>>>> >>>>>>>>> So upon constexpr evaluation of this latter VEC_INIT_EXPR, >>>>>>>>> ctx->ctor >>>>>>>>> is >>>>>>>>> nonempty due to the prior zero-initialization, and we proceed in >>>>>>>>> cxx_eval_vec_init to append new constructor_elts to the end of >>>>>>>>> ctx->ctor >>>>>>>>> without first checking if a matching constructor_elt already >>>>>>>>> exists. >>>>>>>>> This leads to ctx->ctor having two matching constructor_elts for >>>>>>>>> each >>>>>>>>> index. >>>>>>>>> >>>>>>>>> This patch partially fixes this issue by making the RANGE_EXPR >>>>>>>>> optimization in cxx_eval_vec_init truncate ctx->ctor before >>>>>>>>> adding the >>>>>>>>> single RANGE_EXPR constructor_elt. This isn't a complete fix >>>>>>>>> because >>>>>>>>> the RANGE_EXPR optimization applies only when the constant >>>>>>>>> initializer >>>>>>>>> is relocatable, so whenever it's not relocatable we can still >>>>>>>>> build up >>>>>>>>> an invalid CONSTRUCTOR, e.g. if in the first testcase we add an >>>>>>>>> NSDMI >>>>>>>>> such as 'e *p = this;' to struct e, then the ICE still occurs >>>>>>>>> even >>>>>>>>> with >>>>>>>>> this patch. >>>>>>>> >>>>>>>> A complete but more risky one-line fix would be to always truncate >>>>>>>> ctx->ctor beforehand, not just when the RANGE_EXPR optimization >>>>>>>> applies. >>>>>>>> If it's true that the initializer of a VEC_INIT_EXPR can't observe >>>>>>>> the >>>>>>>> previous elements of the target array, then it should be safe to >>>>>>>> always >>>>>>>> truncate I think? >>>>>>> >>>>>>> What if default-initialization of the array element type doesn't >>>>>>> fully >>>>>>> initialize the elements, e.g. if 'e' had another member without a >>>>>>> default >>>>>>> initializer? Does truncation first mean we lose the >>>>>>> zero-initialization >>>>>>> of >>>>>>> such a member? >>>>>> >>>>>> Hmm, it looks like we would lose the zero-initialization of such a >>>>>> member with or without truncation first (so with any one of the three >>>>>> proposed fixes). I think it's because the evaluation loop in >>>>>> cxx_eval_vec_init disregards each element's prior (zero-initialized) >>>>>> state. >>>>>> >>>>>>> >>>>>>> We could probably still do the truncation, but clear the >>>>>>> CONSTRUCTOR_NO_CLEARING flag on the element initializer. >>>>>> >>>>>> Ah, this seems to work well. Like this? >>>>>> >>>>>> -- >8 -- >>>>>> >>>>>> Subject: [PATCH] c++: cxx_eval_vec_init after zero initialization >>>>>> [PR96282] >>>>>> >>>>>> In the first testcase below, expand_aggr_init_1 sets up t's default >>>>>> constructor such that the ctor first zero-initializes the entire base >>>>>> b, >>>>>> followed by calling b's default constructor, the latter of which just >>>>>> default-initializes the array member b::m via a VEC_INIT_EXPR. >>>>>> >>>>>> So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor >>>>>> is >>>>>> nonempty due to the prior zero-initialization, and we proceed in >>>>>> cxx_eval_vec_init to append new constructor_elts to the end of >>>>>> ctx->ctor >>>>>> without first checking if a matching constructor_elt already exists. >>>>>> This leads to ctx->ctor having two matching constructor_elts for each >>>>>> index. >>>>>> >>>>>> This patch fixes this issue by truncating a zero-initialized array >>>>>> object in cxx_eval_vec_init_1 before we begin appending >>>>>> default-initialized >>>>>> array elements to it. Since default-initialization may leave parts of >>>>>> the element type unitialized, we also preserve the array's prior >>>>>> zero-initialized state by clearing CONSTRUCTOR_NO_CLEARING on each >>>>>> appended element initializers. >>>>>> >>>>>> gcc/cp/ChangeLog: >>>>>> >>>>>> PR c++/96282 >>>>>> * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and >>>>>> then clear CONSTRUCTOR_NO_CLEARING on each appended element >>>>>> initializer if we're default-initializing a previously >>>>>> zero-initialized array object. >>>>>> >>>>>> gcc/testsuite/ChangeLog: >>>>>> >>>>>> PR c++/96282 >>>>>> * g++.dg/cpp0x/constexpr-array26.C: New test. >>>>>> * g++.dg/cpp0x/constexpr-array27.C: New test. >>>>>> * g++.dg/cpp2a/constexpr-init18.C: New test. >>>>>> --- >>>>>> gcc/cp/constexpr.c | 17 >>>>>> ++++++++++++++++- >>>>>> gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ >>>>>> gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ >>>>>> gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 >>>>>> ++++++++++++++++ >>>>>> 4 files changed, 58 insertions(+), 1 deletion(-) >>>>>> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C >>>>>> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C >>>>>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C >>>>>> >>>>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >>>>>> index b1c1d249c6e..706bef323b2 100644 >>>>>> --- a/gcc/cp/constexpr.c >>>>>> +++ b/gcc/cp/constexpr.c >>>>>> @@ -4171,6 +4171,17 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, >>>>>> tree >>>>>> atype, tree init, >>>>>> pre_init = true; >>>>>> } >>>>>> + bool zero_initialized_p = false; >>>>>> + if ((pre_init || value_init || !init) && initializer_zerop >>>>>> (ctx->ctor)) >>>>> >>>>> Does initializer_zerop capture the difference between a >>>>> default-initialized or >>>>> value-initialized containing object? I would expect it to be true for >>>>> both. >>>>> Maybe check CONSTRUCTOR_NO_CLEARING (ctx->ctor) instead? >>>> >>>> D'oh, indeed it looks like initializer_zerop is not what we want here. >>> >>> Hmm, might we need to check both? !CONSTRUCTOR_NO_CLEARING tells us >>> about the omitted initializers, but it seems we'd still need to test if >>> the explicit initializers are zero. >> >> We might check initializer_zerop as an assert; at this point the only previous >> initialization should be zero-initialization. > > Sounds good. I wasn't quite sure what we may or may not assume about > the previous initializer. > >> >> What are you going for with (pre_init || value_init || !init)? All cases >> other than array copy? We shouldn't ever see zero-initialization before copy, >> so this test seems unnecessary. > > Yeah, the intent was to make sure to include the recursive > initialization case while excluding the array copy case. I suppose we > can add this test as an assert too, but it doesn't seem that worthwhile. > > In the below I removed the pre_init || value_init || !init test and > added the initializer_zerop test as an assert. Does it look OK to > commit after bootstrap/regtest succeeds? > > -- >8 -- > > Subject: [PATCH] c++: cxx_eval_vec_init after zero-initialization [PR96282] > > In the first testcase below, expand_aggr_init_1 sets up t's default > constructor such that the ctor first zero-initializes the entire base b, > followed by calling b's default constructor, the latter of which just > default-initializes the array member b::m via a VEC_INIT_EXPR. > > So upon constexpr evaluation of this latter VEC_INIT_EXPR, ctx->ctor is > nonempty due to the prior zero-initialization, and we proceed in > cxx_eval_vec_init to append new constructor_elts to the end of ctx->ctor > without first checking if a matching constructor_elt already exists. > This leads to ctx->ctor having two matching constructor_elts for each > index. > > This patch fixes this issue by truncating a zero-initialized array > CONSTRUCTOR in cxx_eval_vec_init_1 before we begin appending array > elements to it. We preserve its zeroed out state during evaluation by > clearing CONSTRUCTOR_NO_CLEARING on each new appended element. > > gcc/cp/ChangeLog: > > PR c++/96282 > * constexpr.c (cxx_eval_vec_init_1): Truncate ctx->ctor and > then clear CONSTRUCTOR_NO_CLEARING on each appended element > initializer if we're initializing a previously zero-initialized > array object. > > gcc/testsuite/ChangeLog: > > PR c++/96282 > * g++.dg/cpp0x/constexpr-array26.C: New test. > * g++.dg/cpp0x/constexpr-array27.C: New test. > * g++.dg/cpp2a/constexpr-init18.C: New test. > --- > gcc/cp/constexpr.c | 18 +++++++++++++++++- > gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C | 13 +++++++++++++ > gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C | 13 +++++++++++++ > gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C | 16 ++++++++++++++++ > 4 files changed, 59 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index b1c1d249c6e..bec888646ea 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -4171,6 +4171,18 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, > pre_init = true; > } > > + bool zeroed_out = false; > + if (!CONSTRUCTOR_NO_CLEARING (ctx->ctor)) > + { > + /* We're initializing an array object that had been zero-initialized > + earlier. Truncate ctx->ctor and preserve its zeroed state by > + clearing CONSTRUCTOR_NO_CLEARING on each of the element > + initializers we append to it. */ > + gcc_assert (initializer_zerop (ctx->ctor)); Let's make this gcc_checking_assert. OK with that change. > + zeroed_out = true; > + vec_safe_truncate (*p, 0); > + } > + > tree nelts = get_array_or_vector_nelts (ctx, atype, non_constant_p, > overflow_p); > unsigned HOST_WIDE_INT max = tree_to_uhwi (nelts); > @@ -4182,7 +4194,11 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, > constexpr_ctx new_ctx; > init_subob_ctx (ctx, new_ctx, idx, pre_init ? init : elttype); > if (new_ctx.ctor != ctx->ctor) > - CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); > + { > + if (zeroed_out) > + CONSTRUCTOR_NO_CLEARING (new_ctx.ctor) = false; > + CONSTRUCTOR_APPEND_ELT (*p, idx, new_ctx.ctor); > + } > if (TREE_CODE (elttype) == ARRAY_TYPE) > { > /* A multidimensional array; recurse. */ > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > new file mode 100644 > index 00000000000..274f55a88bf > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C > @@ -0,0 +1,13 @@ > +// PR c++/96282 > +// { dg-do compile { target c++11 } } > + > +struct e { bool v = true; }; > + > +template<int N> > +struct b { e m[N]; }; > + > +template<int N> > +struct t : b<N> { constexpr t() : b<N>() {} }; > + > +constexpr t<1> h1; > +constexpr t<42> h2; > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > new file mode 100644 > index 00000000000..1234caef31d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C > @@ -0,0 +1,13 @@ > +// PR c++/96282 > +// { dg-do compile { target c++11 } } > + > +struct e { bool v = true; e *p = this; }; > + > +template<int N> > +struct b { e m[N][N]; }; > + > +template<int N> > +struct t : b<N> { constexpr t() : b<N>() {} }; > + > +constexpr t<1> h1; > +constexpr t<42> h2; > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > new file mode 100644 > index 00000000000..47ad11f2290 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-init18.C > @@ -0,0 +1,16 @@ > +// PR c++/96282 > +// { dg-do compile { target c++20 } } > + > +struct e { bool v = true; bool w; }; > + > +template<int N> > +struct b { e m[N][N]; }; > + > +template<int N> > +struct t : b<N> { constexpr t() : b<N>() {} }; > + > +constexpr t<1> h1; > +static_assert(h1.m[0][0].w == false); > + > +constexpr t<42> h2; > +static_assert(h2.m[17][17].w == false); >
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index b1c1d249c6e..3808a0713ba 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -4189,7 +4189,7 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, if (value_init || init == NULL_TREE) { eltinit = NULL_TREE; - reuse = i == 0; + reuse = true; } else eltinit = cp_build_array_ref (input_location, init, idx, complain); @@ -4206,7 +4206,7 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, return ctx->ctor; eltinit = cxx_eval_constant_expression (&new_ctx, init, lval, non_constant_p, overflow_p); - reuse = i == 0; + reuse = true; } else { @@ -4222,33 +4222,37 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, } if (*non_constant_p && !ctx->quiet) break; - if (new_ctx.ctor != ctx->ctor) - { - /* We appended this element above; update the value. */ - gcc_assert ((*p)->last().index == idx); - (*p)->last().value = eltinit; - } - 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 + if (i == 0 + && reuse && (eltinit == NULL_TREE || (initializer_constant_valid_p (eltinit, TREE_TYPE (eltinit)) == null_pointer_node))) { if (new_ctx.ctor != ctx->ctor) eltinit = new_ctx.ctor; - tree range = build2 (RANGE_EXPR, size_type_node, - build_int_cst (size_type_node, 1), - build_int_cst (size_type_node, max - 1)); - CONSTRUCTOR_APPEND_ELT (*p, range, unshare_constructor (eltinit)); + vec_safe_truncate (*p, 0); + if (max > 1) + idx = build2 (RANGE_EXPR, size_type_node, + build_int_cst (size_type_node, 0), + build_int_cst (size_type_node, max - 1)); + CONSTRUCTOR_APPEND_ELT (*p, idx, eltinit); break; } else if (i == 0) vec_safe_reserve (*p, max); + + if (new_ctx.ctor != ctx->ctor) + { + /* We appended this element above; update the value. */ + gcc_assert ((*p)->last().index == idx); + (*p)->last().value = eltinit; + } + else + CONSTRUCTOR_APPEND_ELT (*p, idx, eltinit); } if (!*non_constant_p) diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C new file mode 100644 index 00000000000..274f55a88bf --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array26.C @@ -0,0 +1,13 @@ +// PR c++/96282 +// { dg-do compile { target c++11 } } + +struct e { bool v = true; }; + +template<int N> +struct b { e m[N]; }; + +template<int N> +struct t : b<N> { constexpr t() : b<N>() {} }; + +constexpr t<1> h1; +constexpr t<42> h2; diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C new file mode 100644 index 00000000000..a5ce3f7be08 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array27.C @@ -0,0 +1,18 @@ +// Verify that default initialization an array of aggregates within an aggregate +// does not scale exponentially with the number of dimensions. + +// { dg-do compile { target c++11 } } +// Pass -fsyntax-only to stress only the performance of the frontend. +// { dg-additional-options "-fsyntax-only" } + +struct A +{ + int a = 42; +}; + +struct B +{ + A b[2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2][2]; +}; + +constexpr B c;