* [PATCH] Fix gimplification of const var initialization from COND_EXPR (PR c++/80129)
@ 2017-03-21 20:26 Jakub Jelinek
2017-03-22 8:20 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2017-03-21 20:26 UTC (permalink / raw)
To: Richard Biener, Jason Merrill; +Cc: gcc-patches
Hi!
For non-gimple reg types var = x ? y : z; is gimplified as
x ? (var = y) : (var = z);. The problem is that if var is TREE_READONLY,
we can gimplify one of those assignments by promoting them to TREE_STATIC
(if not addressable) and making the initializer into their DECL_INITIAL.
That doesn't work well, because then we conditionally store into that and
during optimizations, as it is a TREE_READONLY var with DECL_INITIAL, just
use that initializer for optimizations.
The following patch fixes that by clearing TREE_READONLY flag if we are
going to store it more than once.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-03-21 Jakub Jelinek <jakub@redhat.com>
PR c++/80129
* gimplify.c (gimplify_modify_expr_rhs) <case COND_EXPR>: Clear
TREE_READONLY on result if writing it more than once.
* g++.dg/torture/pr80129.C: New test.
--- gcc/gimplify.c.jj 2017-03-21 07:56:55.000000000 +0100
+++ gcc/gimplify.c 2017-03-21 13:37:45.555612652 +0100
@@ -5098,6 +5098,13 @@ gimplify_modify_expr_rhs (tree *expr_p,
if (ret != GS_ERROR)
ret = GS_OK;
+ /* If we are going to write RESULT more than once, clear
+ TREE_READONLY flag, otherwise we might gimplify it
+ incorrectly. */
+ if (DECL_P (result)
+ && TREE_TYPE (TREE_OPERAND (cond, 1)) != void_type_node
+ && TREE_TYPE (TREE_OPERAND (cond, 2)) != void_type_node)
+ TREE_READONLY (result) = 0;
if (TREE_TYPE (TREE_OPERAND (cond, 1)) != void_type_node)
TREE_OPERAND (cond, 1)
= build2 (code, void_type_node, result,
--- gcc/testsuite/g++.dg/torture/pr80129.C.jj 2017-03-21 13:40:04.179852313 +0100
+++ gcc/testsuite/g++.dg/torture/pr80129.C 2017-03-21 13:41:32.121735570 +0100
@@ -0,0 +1,14 @@
+// PR c++/80129
+// { dg-do run }
+// { dg-options "-std=c++11" }
+
+struct A { bool a; int b; };
+
+int
+main ()
+{
+ bool c = false;
+ const A x = c ? A {true, 1} : A {false, 0};
+ if (x.a)
+ __builtin_abort ();
+}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix gimplification of const var initialization from COND_EXPR (PR c++/80129)
2017-03-21 20:26 [PATCH] Fix gimplification of const var initialization from COND_EXPR (PR c++/80129) Jakub Jelinek
@ 2017-03-22 8:20 ` Richard Biener
2017-03-22 8:36 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2017-03-22 8:20 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches
On Tue, 21 Mar 2017, Jakub Jelinek wrote:
> Hi!
>
> For non-gimple reg types var = x ? y : z; is gimplified as
> x ? (var = y) : (var = z);. The problem is that if var is TREE_READONLY,
> we can gimplify one of those assignments by promoting them to TREE_STATIC
> (if not addressable) and making the initializer into their DECL_INITIAL.
> That doesn't work well, because then we conditionally store into that and
> during optimizations, as it is a TREE_READONLY var with DECL_INITIAL, just
> use that initializer for optimizations.
>
> The following patch fixes that by clearing TREE_READONLY flag if we are
> going to store it more than once.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2017-03-21 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/80129
> * gimplify.c (gimplify_modify_expr_rhs) <case COND_EXPR>: Clear
> TREE_READONLY on result if writing it more than once.
>
> * g++.dg/torture/pr80129.C: New test.
>
> --- gcc/gimplify.c.jj 2017-03-21 07:56:55.000000000 +0100
> +++ gcc/gimplify.c 2017-03-21 13:37:45.555612652 +0100
> @@ -5098,6 +5098,13 @@ gimplify_modify_expr_rhs (tree *expr_p,
> if (ret != GS_ERROR)
> ret = GS_OK;
>
> + /* If we are going to write RESULT more than once, clear
> + TREE_READONLY flag, otherwise we might gimplify it
> + incorrectly. */
> + if (DECL_P (result)
Can it ever be that result is sth like decl.component? At least I
don't see GENERIC restricted in a way that it cannot. (and generally
assigning to sth TREE_READONLY is fishy at best...)
I see how this fixes the problem at hand (and in a safe way), so the
patch is probably ok, just looking how to avoid this situation in
a more general (and better) way.
> + && TREE_TYPE (TREE_OPERAND (cond, 1)) != void_type_node
> + && TREE_TYPE (TREE_OPERAND (cond, 2)) != void_type_node)
> + TREE_READONLY (result) = 0;
> if (TREE_TYPE (TREE_OPERAND (cond, 1)) != void_type_node)
> TREE_OPERAND (cond, 1)
> = build2 (code, void_type_node, result,
> --- gcc/testsuite/g++.dg/torture/pr80129.C.jj 2017-03-21 13:40:04.179852313 +0100
> +++ gcc/testsuite/g++.dg/torture/pr80129.C 2017-03-21 13:41:32.121735570 +0100
> @@ -0,0 +1,14 @@
> +// PR c++/80129
> +// { dg-do run }
> +// { dg-options "-std=c++11" }
> +
> +struct A { bool a; int b; };
> +
> +int
> +main ()
> +{
> + bool c = false;
> + const A x = c ? A {true, 1} : A {false, 0};
> + if (x.a)
> + __builtin_abort ();
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix gimplification of const var initialization from COND_EXPR (PR c++/80129)
2017-03-22 8:20 ` Richard Biener
@ 2017-03-22 8:36 ` Jakub Jelinek
2017-03-22 8:59 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2017-03-22 8:36 UTC (permalink / raw)
To: Richard Biener; +Cc: Jason Merrill, gcc-patches
On Wed, Mar 22, 2017 at 09:20:32AM +0100, Richard Biener wrote:
> > --- gcc/gimplify.c.jj 2017-03-21 07:56:55.000000000 +0100
> > +++ gcc/gimplify.c 2017-03-21 13:37:45.555612652 +0100
> > @@ -5098,6 +5098,13 @@ gimplify_modify_expr_rhs (tree *expr_p,
> > if (ret != GS_ERROR)
> > ret = GS_OK;
> >
> > + /* If we are going to write RESULT more than once, clear
> > + TREE_READONLY flag, otherwise we might gimplify it
> > + incorrectly. */
> > + if (DECL_P (result)
>
> Can it ever be that result is sth like decl.component? At least I
> don't see GENERIC restricted in a way that it cannot. (and generally
> assigning to sth TREE_READONLY is fishy at best...)
I guess it can, but it can't trigger
if (valid_const_initializer
&& num_nonzero_elements > 1
&& TREE_READONLY (object)
&& VAR_P (object)
&& (flag_merge_constants >= 2 || !TREE_ADDRESSABLE (object)))
which is the problem here. The problem isn't multiple assignments to
TREE_READONLY object if it isn't promoted to static.
So, if you want, I can replace the DECL_P (result) check with VAR_P
(result), and perhaps change the comment somehow.
> I see how this fixes the problem at hand (and in a safe way), so the
> patch is probably ok, just looking how to avoid this situation in
> a more general (and better) way.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix gimplification of const var initialization from COND_EXPR (PR c++/80129)
2017-03-22 8:36 ` Jakub Jelinek
@ 2017-03-22 8:59 ` Richard Biener
2017-03-22 13:13 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2017-03-22 8:59 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches
On Wed, 22 Mar 2017, Jakub Jelinek wrote:
> On Wed, Mar 22, 2017 at 09:20:32AM +0100, Richard Biener wrote:
> > > --- gcc/gimplify.c.jj 2017-03-21 07:56:55.000000000 +0100
> > > +++ gcc/gimplify.c 2017-03-21 13:37:45.555612652 +0100
> > > @@ -5098,6 +5098,13 @@ gimplify_modify_expr_rhs (tree *expr_p,
> > > if (ret != GS_ERROR)
> > > ret = GS_OK;
> > >
> > > + /* If we are going to write RESULT more than once, clear
> > > + TREE_READONLY flag, otherwise we might gimplify it
> > > + incorrectly. */
> > > + if (DECL_P (result)
> >
> > Can it ever be that result is sth like decl.component? At least I
> > don't see GENERIC restricted in a way that it cannot. (and generally
> > assigning to sth TREE_READONLY is fishy at best...)
>
> I guess it can, but it can't trigger
> if (valid_const_initializer
> && num_nonzero_elements > 1
> && TREE_READONLY (object)
> && VAR_P (object)
> && (flag_merge_constants >= 2 || !TREE_ADDRESSABLE (object)))
> which is the problem here. The problem isn't multiple assignments to
> TREE_READONLY object if it isn't promoted to static.
Ok.
> So, if you want, I can replace the DECL_P (result) check with VAR_P
> (result), and perhaps change the comment somehow.
Yeah, changing it to VAR_P and expanding the comment so it says
it avoids incorrect promotion to readonly-static. (so it _is_
fishy that we have multiple assignments to TREE_READONLY objects
given that code looks at a single assignment only)
> > I see how this fixes the problem at hand (and in a safe way), so the
> > patch is probably ok, just looking how to avoid this situation in
> > a more general (and better) way.
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix gimplification of const var initialization from COND_EXPR (PR c++/80129)
2017-03-22 8:59 ` Richard Biener
@ 2017-03-22 13:13 ` Jakub Jelinek
2017-03-22 13:26 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2017-03-22 13:13 UTC (permalink / raw)
To: Richard Biener; +Cc: Jason Merrill, gcc-patches
On Wed, Mar 22, 2017 at 09:59:09AM +0100, Richard Biener wrote:
> Yeah, changing it to VAR_P and expanding the comment so it says
> it avoids incorrect promotion to readonly-static. (so it _is_
> fishy that we have multiple assignments to TREE_READONLY objects
> given that code looks at a single assignment only)
So like this?
2017-03-22 Jakub Jelinek <jakub@redhat.com>
PR c++/80129
* gimplify.c (gimplify_modify_expr_rhs) <case COND_EXPR>: Clear
TREE_READONLY on result if writing it more than once.
* g++.dg/torture/pr80129.C: New test.
--- gcc/gimplify.c.jj 2017-03-21 07:56:55.000000000 +0100
+++ gcc/gimplify.c 2017-03-21 13:37:45.555612652 +0100
@@ -5098,6 +5098,14 @@ gimplify_modify_expr_rhs (tree *expr_p,
if (ret != GS_ERROR)
ret = GS_OK;
+ /* If we are going to write RESULT more than once, clear
+ TREE_READONLY flag, otherwise we might incorrectly promote
+ the variable to static const and initialize it at compile
+ time in one of the branches. */
+ if (VAR_P (result)
+ && TREE_TYPE (TREE_OPERAND (cond, 1)) != void_type_node
+ && TREE_TYPE (TREE_OPERAND (cond, 2)) != void_type_node)
+ TREE_READONLY (result) = 0;
if (TREE_TYPE (TREE_OPERAND (cond, 1)) != void_type_node)
TREE_OPERAND (cond, 1)
= build2 (code, void_type_node, result,
--- gcc/testsuite/g++.dg/torture/pr80129.C.jj 2017-03-21 13:40:04.179852313 +0100
+++ gcc/testsuite/g++.dg/torture/pr80129.C 2017-03-21 13:41:32.121735570 +0100
@@ -0,0 +1,14 @@
+// PR c++/80129
+// { dg-do run }
+// { dg-options "-std=c++11" }
+
+struct A { bool a; int b; };
+
+int
+main ()
+{
+ bool c = false;
+ const A x = c ? A {true, 1} : A {false, 0};
+ if (x.a)
+ __builtin_abort ();
+}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix gimplification of const var initialization from COND_EXPR (PR c++/80129)
2017-03-22 13:13 ` Jakub Jelinek
@ 2017-03-22 13:26 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2017-03-22 13:26 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches
On Wed, 22 Mar 2017, Jakub Jelinek wrote:
> On Wed, Mar 22, 2017 at 09:59:09AM +0100, Richard Biener wrote:
> > Yeah, changing it to VAR_P and expanding the comment so it says
> > it avoids incorrect promotion to readonly-static. (so it _is_
> > fishy that we have multiple assignments to TREE_READONLY objects
> > given that code looks at a single assignment only)
>
> So like this?
Yes.
Thanks,
Richard.
> 2017-03-22 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/80129
> * gimplify.c (gimplify_modify_expr_rhs) <case COND_EXPR>: Clear
> TREE_READONLY on result if writing it more than once.
>
> * g++.dg/torture/pr80129.C: New test.
>
> --- gcc/gimplify.c.jj 2017-03-21 07:56:55.000000000 +0100
> +++ gcc/gimplify.c 2017-03-21 13:37:45.555612652 +0100
> @@ -5098,6 +5098,14 @@ gimplify_modify_expr_rhs (tree *expr_p,
> if (ret != GS_ERROR)
> ret = GS_OK;
>
> + /* If we are going to write RESULT more than once, clear
> + TREE_READONLY flag, otherwise we might incorrectly promote
> + the variable to static const and initialize it at compile
> + time in one of the branches. */
> + if (VAR_P (result)
> + && TREE_TYPE (TREE_OPERAND (cond, 1)) != void_type_node
> + && TREE_TYPE (TREE_OPERAND (cond, 2)) != void_type_node)
> + TREE_READONLY (result) = 0;
> if (TREE_TYPE (TREE_OPERAND (cond, 1)) != void_type_node)
> TREE_OPERAND (cond, 1)
> = build2 (code, void_type_node, result,
> --- gcc/testsuite/g++.dg/torture/pr80129.C.jj 2017-03-21 13:40:04.179852313 +0100
> +++ gcc/testsuite/g++.dg/torture/pr80129.C 2017-03-21 13:41:32.121735570 +0100
> @@ -0,0 +1,14 @@
> +// PR c++/80129
> +// { dg-do run }
> +// { dg-options "-std=c++11" }
> +
> +struct A { bool a; int b; };
> +
> +int
> +main ()
> +{
> + bool c = false;
> + const A x = c ? A {true, 1} : A {false, 0};
> + if (x.a)
> + __builtin_abort ();
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-22 13:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-21 20:26 [PATCH] Fix gimplification of const var initialization from COND_EXPR (PR c++/80129) Jakub Jelinek
2017-03-22 8:20 ` Richard Biener
2017-03-22 8:36 ` Jakub Jelinek
2017-03-22 8:59 ` Richard Biener
2017-03-22 13:13 ` Jakub Jelinek
2017-03-22 13:26 ` Richard Biener
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).