* [PATCH] Do not discard the constructors of empty structs [PR c++/64527]
@ 2015-04-16 1:00 Patrick Palka
2015-04-16 14:32 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Patrick Palka @ 2015-04-16 1:00 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, Patrick Palka
gimplify_init_constructor() only attempts to gimplify a CONSTRUCTOR's
elts under two conditions: if the object has not been zero-initialized,
or if categorize_ctor_elements() detected more than one "nonzero" elt in
the CONSTRUCTOR. With the PR's testcase, both of these conditions are
false: the gimplifier decides to zero-initialize the object (apparently
because one of the object's field was not initialized), and
categorize_ctor_elements() rightly does not count an empty struct field
as a non-zero component of the CONSTRUCTOR (so num_nonzero_elements
remains 0). So we end up not emitting the call to field "A a"'s
constructor.
In order to fix this issue, I think we should unconditionally call
gimplify_init_ctor_eval even if we have cleared the object and even if
the object has no nonzero elements. Because even so, one of the
constructor's elts may be side-effecting and in such a case we would
still want to emit it. The function already has logic to skip over
redundant zero-initializations so we should be safe in that regard.
Bootstrap and regtesting (including fortran + ada) on x86_64-unknown-linux-gnu
in progress. OK for trunk if it succeeds?
gcc/
PR c++/64527
* gcc/gimplify.c (gimplify_init_constructor): Unconditionally
call gimplify_init_ctor_eval.
gcc/testsuite/
* g++.dg/init/pr64527.C: New test.
---
gcc/gimplify.c | 6 +-----
gcc/testsuite/g++.dg/init/pr64527.C | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/init/pr64527.C
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index ff0a225..5158243 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -4005,11 +4005,7 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
gimplify_stmt (expr_p, pre_p);
}
- /* If we have not block cleared the object, or if there are nonzero
- elements in the constructor, add assignments to the individual
- scalar fields of the object. */
- if (!cleared || num_nonzero_elements > 0)
- gimplify_init_ctor_eval (object, elts, pre_p, cleared);
+ gimplify_init_ctor_eval (object, elts, pre_p, cleared);
*expr_p = NULL_TREE;
}
diff --git a/gcc/testsuite/g++.dg/init/pr64527.C b/gcc/testsuite/g++.dg/init/pr64527.C
new file mode 100644
index 0000000..36b8214
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/pr64527.C
@@ -0,0 +1,21 @@
+// { dg-do run { target c++11 } }
+
+static int g;
+
+struct A {
+ A() { g = 1; }
+};
+
+struct accessor {
+ A a;
+ int x;
+};
+
+int
+main (void)
+{
+ (void) accessor{};
+
+ if (g != 1)
+ __builtin_abort ();
+}
--
2.4.0.rc2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Do not discard the constructors of empty structs [PR c++/64527]
2015-04-16 1:00 [PATCH] Do not discard the constructors of empty structs [PR c++/64527] Patrick Palka
@ 2015-04-16 14:32 ` Jason Merrill
2015-04-16 15:05 ` Patrick Palka
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2015-04-16 14:32 UTC (permalink / raw)
To: Patrick Palka, gcc-patches
On 04/15/2015 09:00 PM, Patrick Palka wrote:
> - if (!cleared || num_nonzero_elements > 0)
How about adding || TREE_SIDE_EFFECTS (TREE_OPERAND (*expr_p), 1) to
this test rather than removing it entirely?
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Do not discard the constructors of empty structs [PR c++/64527]
2015-04-16 14:32 ` Jason Merrill
@ 2015-04-16 15:05 ` Patrick Palka
2015-04-16 19:55 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Patrick Palka @ 2015-04-16 15:05 UTC (permalink / raw)
To: Jason Merrill; +Cc: Patrick Palka, gcc-patches
On Thu, 16 Apr 2015, Jason Merrill wrote:
> On 04/15/2015 09:00 PM, Patrick Palka wrote:
>> - if (!cleared || num_nonzero_elements > 0)
>
> How about adding || TREE_SIDE_EFFECTS (TREE_OPERAND (*expr_p), 1) to this
> test rather than removing it entirely?
That works too. Does the following patch look OK if testing succeeds?
gcc/
PR c++/64527
* gcc/gimplify.c (gimplify_init_constructor): Always emit a
side-effecting constructor.
gcc/testsuite/
* g++.dg/init/pr64527.C: New test.
---
gcc/gimplify.c | 11 ++++++++---
gcc/testsuite/g++.dg/init/pr64527.C | 21 +++++++++++++++++++++
2 files changed, 29 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/init/pr64527.C
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index ff0a225..29d3f4d 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3994,6 +3994,9 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
pre_p, post_p, &preeval_data);
}
+ bool ctor_has_side_effects_p
+ = TREE_SIDE_EFFECTS (TREE_OPERAND (*expr_p, 1));
+
if (cleared)
{
/* Zap the CONSTRUCTOR element list, which simplifies this case.
@@ -4006,9 +4009,11 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
}
/* If we have not block cleared the object, or if there are nonzero
- elements in the constructor, add assignments to the individual
- scalar fields of the object. */
- if (!cleared || num_nonzero_elements > 0)
+ elements in the constructor, or if the constructor has side effects,
+ add assignments to the individual scalar fields of the object. */
+ if (!cleared
+ || num_nonzero_elements > 0
+ || ctor_has_side_effects_p)
gimplify_init_ctor_eval (object, elts, pre_p, cleared);
*expr_p = NULL_TREE;
diff --git a/gcc/testsuite/g++.dg/init/pr64527.C b/gcc/testsuite/g++.dg/init/pr64527.C
new file mode 100644
index 0000000..36b8214
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/pr64527.C
@@ -0,0 +1,21 @@
+// { dg-do run { target c++11 } }
+
+static int g;
+
+struct A {
+ A() { g = 1; }
+};
+
+struct accessor {
+ A a;
+ int x;
+};
+
+int
+main (void)
+{
+ (void) accessor{};
+
+ if (g != 1)
+ __builtin_abort ();
+}
--
2.4.0.rc2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Do not discard the constructors of empty structs [PR c++/64527]
2015-04-16 15:05 ` Patrick Palka
@ 2015-04-16 19:55 ` Jason Merrill
2015-04-17 12:18 ` Patrick Palka
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2015-04-16 19:55 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches
OK.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Do not discard the constructors of empty structs [PR c++/64527]
2015-04-16 19:55 ` Jason Merrill
@ 2015-04-17 12:18 ` Patrick Palka
0 siblings, 0 replies; 5+ messages in thread
From: Patrick Palka @ 2015-04-17 12:18 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Thu, Apr 16, 2015 at 3:54 PM, Jason Merrill <jason@redhat.com> wrote:
> OK.
Thanks, committed as revision 222176.
>
> Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-04-17 12:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-16 1:00 [PATCH] Do not discard the constructors of empty structs [PR c++/64527] Patrick Palka
2015-04-16 14:32 ` Jason Merrill
2015-04-16 15:05 ` Patrick Palka
2015-04-16 19:55 ` Jason Merrill
2015-04-17 12:18 ` Patrick Palka
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).