* [PATCH] c++: Handle structured bindings like anon unions in initializers [PR108474]
@ 2023-01-21 9:59 Jakub Jelinek
2023-01-22 20:40 ` Jason Merrill
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2023-01-21 9:59 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
As reported by Andrew Pinski, structured bindings (with the exception
of the ones using std::tuple_{size,element} and get which are really
standalone variables in addition to the binding one) also use
DECL_VALUE_EXPR and needs the same treatment in static initializers.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok
for trunk?
2023-01-21 Jakub Jelinek <jakub@redhat.com>
PR c++/108474
* cp-gimplify.cc (cp_fold_r): Handle structured bindings
vars like anon union artificial vars.
* g++.dg/cpp1z/decomp57.C: New test.
* g++.dg/cpp1z/decomp58.C: New test.
--- gcc/cp/cp-gimplify.cc.jj 2023-01-19 23:27:27.998400866 +0100
+++ gcc/cp/cp-gimplify.cc 2023-01-20 11:00:06.093446737 +0100
@@ -1012,8 +1012,12 @@ cp_fold_r (tree *stmt_p, int *walk_subtr
case VAR_DECL:
/* In initializers replace anon union artificial VAR_DECLs
- with their DECL_VALUE_EXPRs, as nothing will do it later. */
- if (DECL_ANON_UNION_VAR_P (stmt) && !data->genericize)
+ with their DECL_VALUE_EXPRs, as nothing will do it later.
+ Ditto for structured bindings. */
+ if (!data->genericize
+ && DECL_HAS_VALUE_EXPR_P (stmt)
+ && (DECL_ANON_UNION_VAR_P (stmt)
+ || (DECL_DECOMPOSITION_P (stmt) && DECL_DECOMP_BASE (stmt))))
{
*stmt_p = stmt = unshare_expr (DECL_VALUE_EXPR (stmt));
break;
--- gcc/testsuite/g++.dg/cpp1z/decomp57.C.jj 2023-01-20 11:02:08.547656638 +0100
+++ gcc/testsuite/g++.dg/cpp1z/decomp57.C 2023-01-20 10:53:01.781649565 +0100
@@ -0,0 +1,27 @@
+// PR c++/108474
+// { dg-do link { target c++17 } }
+
+struct T { int i, j; };
+T h;
+auto [i, j] = h;
+int &r = i;
+int s = i;
+int *t = &i;
+
+void
+foo (int **p, int *q)
+{
+ static int &u = i;
+ static int v = i;
+ static int *w = &i;
+ int &x = i;
+ int y = i;
+ int *z = &i;
+ *p = &i;
+ *q = i;
+}
+
+int
+main ()
+{
+}
--- gcc/testsuite/g++.dg/cpp1z/decomp58.C.jj 2023-01-20 11:02:12.314601575 +0100
+++ gcc/testsuite/g++.dg/cpp1z/decomp58.C 2023-01-20 10:54:02.117767542 +0100
@@ -0,0 +1,39 @@
+// PR c++/108474
+// { dg-do link { target c++17 } }
+
+namespace std {
+ template <typename T> struct tuple_size;
+ template <int, typename> struct tuple_element;
+}
+
+struct A {
+ int i;
+ template <int I> int& get() { return i; }
+};
+
+template <> struct std::tuple_size <A> { static const int value = 2; };
+template <int I> struct std::tuple_element <I, A> { using type = int; };
+
+struct A a;
+auto [i, j] = a;
+int &r = i;
+int s = i;
+int *t = &i;
+
+void
+foo (int **p, int *q)
+{
+ static int &u = i;
+ static int v = i;
+ static int *w = &i;
+ int &x = i;
+ int y = i;
+ int *z = &i;
+ *p = &i;
+ *q = i;
+}
+
+int
+main ()
+{
+}
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c++: Handle structured bindings like anon unions in initializers [PR108474]
2023-01-21 9:59 [PATCH] c++: Handle structured bindings like anon unions in initializers [PR108474] Jakub Jelinek
@ 2023-01-22 20:40 ` Jason Merrill
2023-01-22 20:50 ` Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Jason Merrill @ 2023-01-22 20:40 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 1/21/23 04:59, Jakub Jelinek wrote:
> Hi!
>
> As reported by Andrew Pinski, structured bindings (with the exception
> of the ones using std::tuple_{size,element} and get which are really
> standalone variables in addition to the binding one) also use
> DECL_VALUE_EXPR and needs the same treatment in static initializers.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok
> for trunk?
>
> 2023-01-21 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/108474
> * cp-gimplify.cc (cp_fold_r): Handle structured bindings
> vars like anon union artificial vars.
>
> * g++.dg/cpp1z/decomp57.C: New test.
> * g++.dg/cpp1z/decomp58.C: New test.
>
> --- gcc/cp/cp-gimplify.cc.jj 2023-01-19 23:27:27.998400866 +0100
> +++ gcc/cp/cp-gimplify.cc 2023-01-20 11:00:06.093446737 +0100
> @@ -1012,8 +1012,12 @@ cp_fold_r (tree *stmt_p, int *walk_subtr
>
> case VAR_DECL:
> /* In initializers replace anon union artificial VAR_DECLs
> - with their DECL_VALUE_EXPRs, as nothing will do it later. */
> - if (DECL_ANON_UNION_VAR_P (stmt) && !data->genericize)
> + with their DECL_VALUE_EXPRs, as nothing will do it later.
> + Ditto for structured bindings. */
> + if (!data->genericize
> + && DECL_HAS_VALUE_EXPR_P (stmt)
> + && (DECL_ANON_UNION_VAR_P (stmt)
> + || (DECL_DECOMPOSITION_P (stmt) && DECL_DECOMP_BASE (stmt))))
Is there a reason not to do this for all cases of DECL_HAS_VALUE_EXPR_P,
without the extra checks?
> {
> *stmt_p = stmt = unshare_expr (DECL_VALUE_EXPR (stmt));
> break;
> --- gcc/testsuite/g++.dg/cpp1z/decomp57.C.jj 2023-01-20 11:02:08.547656638 +0100
> +++ gcc/testsuite/g++.dg/cpp1z/decomp57.C 2023-01-20 10:53:01.781649565 +0100
> @@ -0,0 +1,27 @@
> +// PR c++/108474
> +// { dg-do link { target c++17 } }
> +
> +struct T { int i, j; };
> +T h;
> +auto [i, j] = h;
> +int &r = i;
> +int s = i;
> +int *t = &i;
> +
> +void
> +foo (int **p, int *q)
> +{
> + static int &u = i;
> + static int v = i;
> + static int *w = &i;
> + int &x = i;
> + int y = i;
> + int *z = &i;
> + *p = &i;
> + *q = i;
> +}
> +
> +int
> +main ()
> +{
> +}
> --- gcc/testsuite/g++.dg/cpp1z/decomp58.C.jj 2023-01-20 11:02:12.314601575 +0100
> +++ gcc/testsuite/g++.dg/cpp1z/decomp58.C 2023-01-20 10:54:02.117767542 +0100
> @@ -0,0 +1,39 @@
> +// PR c++/108474
> +// { dg-do link { target c++17 } }
> +
> +namespace std {
> + template <typename T> struct tuple_size;
> + template <int, typename> struct tuple_element;
> +}
> +
> +struct A {
> + int i;
> + template <int I> int& get() { return i; }
> +};
> +
> +template <> struct std::tuple_size <A> { static const int value = 2; };
> +template <int I> struct std::tuple_element <I, A> { using type = int; };
> +
> +struct A a;
> +auto [i, j] = a;
> +int &r = i;
> +int s = i;
> +int *t = &i;
> +
> +void
> +foo (int **p, int *q)
> +{
> + static int &u = i;
> + static int v = i;
> + static int *w = &i;
> + int &x = i;
> + int y = i;
> + int *z = &i;
> + *p = &i;
> + *q = i;
> +}
> +
> +int
> +main ()
> +{
> +}
>
> Jakub
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c++: Handle structured bindings like anon unions in initializers [PR108474]
2023-01-22 20:40 ` Jason Merrill
@ 2023-01-22 20:50 ` Jakub Jelinek
2023-01-23 0:19 ` Jason Merrill
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2023-01-22 20:50 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Sun, Jan 22, 2023 at 03:40:26PM -0500, Jason Merrill wrote:
> > 2023-01-21 Jakub Jelinek <jakub@redhat.com>
> >
> > PR c++/108474
> > * cp-gimplify.cc (cp_fold_r): Handle structured bindings
> > vars like anon union artificial vars.
> >
> > * g++.dg/cpp1z/decomp57.C: New test.
> > * g++.dg/cpp1z/decomp58.C: New test.
> >
> > --- gcc/cp/cp-gimplify.cc.jj 2023-01-19 23:27:27.998400866 +0100
> > +++ gcc/cp/cp-gimplify.cc 2023-01-20 11:00:06.093446737 +0100
> > @@ -1012,8 +1012,12 @@ cp_fold_r (tree *stmt_p, int *walk_subtr
> > case VAR_DECL:
> > /* In initializers replace anon union artificial VAR_DECLs
> > - with their DECL_VALUE_EXPRs, as nothing will do it later. */
> > - if (DECL_ANON_UNION_VAR_P (stmt) && !data->genericize)
> > + with their DECL_VALUE_EXPRs, as nothing will do it later.
> > + Ditto for structured bindings. */
> > + if (!data->genericize
> > + && DECL_HAS_VALUE_EXPR_P (stmt)
> > + && (DECL_ANON_UNION_VAR_P (stmt)
> > + || (DECL_DECOMPOSITION_P (stmt) && DECL_DECOMP_BASE (stmt))))
>
> Is there a reason not to do this for all cases of DECL_HAS_VALUE_EXPR_P,
> without the extra checks?
I was just trying to be careful, because unfortunately this spot
doesn't mean it really is only expanded in static var DECL_INITIAL,
it can make it into dynamic initializers, and most of DECL_VALUE_EXPRs
appear only in runtime code, otherwise we'd have much more of these issues.
But if you think it is ok, I'll test tonight a version just with
if (!data->genericize && DECL_HAS_VALUE_EXPR_P (stmt)
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c++: Handle structured bindings like anon unions in initializers [PR108474]
2023-01-22 20:50 ` Jakub Jelinek
@ 2023-01-23 0:19 ` Jason Merrill
2023-01-23 8:25 ` Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Jason Merrill @ 2023-01-23 0:19 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 1/22/23 15:50, Jakub Jelinek wrote:
> On Sun, Jan 22, 2023 at 03:40:26PM -0500, Jason Merrill wrote:
>>> 2023-01-21 Jakub Jelinek <jakub@redhat.com>
>>>
>>> PR c++/108474
>>> * cp-gimplify.cc (cp_fold_r): Handle structured bindings
>>> vars like anon union artificial vars.
>>>
>>> * g++.dg/cpp1z/decomp57.C: New test.
>>> * g++.dg/cpp1z/decomp58.C: New test.
>>>
>>> --- gcc/cp/cp-gimplify.cc.jj 2023-01-19 23:27:27.998400866 +0100
>>> +++ gcc/cp/cp-gimplify.cc 2023-01-20 11:00:06.093446737 +0100
>>> @@ -1012,8 +1012,12 @@ cp_fold_r (tree *stmt_p, int *walk_subtr
>>> case VAR_DECL:
>>> /* In initializers replace anon union artificial VAR_DECLs
>>> - with their DECL_VALUE_EXPRs, as nothing will do it later. */
>>> - if (DECL_ANON_UNION_VAR_P (stmt) && !data->genericize)
>>> + with their DECL_VALUE_EXPRs, as nothing will do it later.
>>> + Ditto for structured bindings. */
>>> + if (!data->genericize
>>> + && DECL_HAS_VALUE_EXPR_P (stmt)
>>> + && (DECL_ANON_UNION_VAR_P (stmt)
>>> + || (DECL_DECOMPOSITION_P (stmt) && DECL_DECOMP_BASE (stmt))))
>>
>> Is there a reason not to do this for all cases of DECL_HAS_VALUE_EXPR_P,
>> without the extra checks?
>
> I was just trying to be careful, because unfortunately this spot
> doesn't mean it really is only expanded in static var DECL_INITIAL,
> it can make it into dynamic initializers, and most of DECL_VALUE_EXPRs
> appear only in runtime code, otherwise we'd have much more of these issues.
But it shouldn't be harmful anywhere, right?
> But if you think it is ok, I'll test tonight a version just with
> if (!data->genericize && DECL_HAS_VALUE_EXPR_P (stmt)
OK with that change.
Though, actually, why not instead fix expand_expr_real_1 (and staticp)
to look through DECL_VALUE_EXPR?
Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c++: Handle structured bindings like anon unions in initializers [PR108474]
2023-01-23 0:19 ` Jason Merrill
@ 2023-01-23 8:25 ` Jakub Jelinek
2023-01-23 11:16 ` [PATCH] c++, cgraphbuild: Handle DECL_VALUE_EXPRs in record_reference [PR108474] Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2023-01-23 8:25 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Sun, Jan 22, 2023 at 07:19:07PM -0500, Jason Merrill wrote:
> > I was just trying to be careful, because unfortunately this spot
> > doesn't mean it really is only expanded in static var DECL_INITIAL,
> > it can make it into dynamic initializers, and most of DECL_VALUE_EXPRs
> > appear only in runtime code, otherwise we'd have much more of these issues.
>
> But it shouldn't be harmful anywhere, right?
>
> > But if you think it is ok, I'll test tonight a version just with
> > if (!data->genericize && DECL_HAS_VALUE_EXPR_P (stmt)
>
> OK with that change.
>
> Though, actually, why not instead fix expand_expr_real_1 (and staticp) to
> look through DECL_VALUE_EXPR?
I'm afraid that is way too late. GIMPLE optimizations don't try to
regimplify expressions they take from DECL_INITIAL of static vars, they
just unshare them and use them. So, if this is to be done in the generic
code, it would need to be done by cgraph around the time when it gimplifies
function if there is any such point for variables.
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] c++, cgraphbuild: Handle DECL_VALUE_EXPRs in record_reference [PR108474]
2023-01-23 8:25 ` Jakub Jelinek
@ 2023-01-23 11:16 ` Jakub Jelinek
2023-01-23 12:37 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2023-01-23 11:16 UTC (permalink / raw)
To: Jason Merrill, Jan Hubicka, Richard Biener; +Cc: gcc-patches
On Mon, Jan 23, 2023 at 09:25:50AM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Sun, Jan 22, 2023 at 07:19:07PM -0500, Jason Merrill wrote:
> > > I was just trying to be careful, because unfortunately this spot
> > > doesn't mean it really is only expanded in static var DECL_INITIAL,
> > > it can make it into dynamic initializers, and most of DECL_VALUE_EXPRs
> > > appear only in runtime code, otherwise we'd have much more of these issues.
> >
> > But it shouldn't be harmful anywhere, right?
> >
> > > But if you think it is ok, I'll test tonight a version just with
> > > if (!data->genericize && DECL_HAS_VALUE_EXPR_P (stmt)
> >
> > OK with that change.
> >
> > Though, actually, why not instead fix expand_expr_real_1 (and staticp) to
> > look through DECL_VALUE_EXPR?
>
> I'm afraid that is way too late. GIMPLE optimizations don't try to
> regimplify expressions they take from DECL_INITIAL of static vars, they
> just unshare them and use them. So, if this is to be done in the generic
> code, it would need to be done by cgraph around the time when it gimplifies
> function if there is any such point for variables.
I believe the right spot is record_reference, which is called through
walk_tree from record_references_in_initializer which is called from
varpool_node::analyze.
The new tests as well as g++.dg/init/pr53932.C pass with it, will do full
bootstrap/regtest soon.
What do you think about this?
2023-01-23 Jakub Jelinek <jakub@redhat.com>
PR c++/108474
* cgraphbuild.cc: Include gimplify.h.
(record_reference): Replace VAR_DECLs with DECL_HAS_VALUE_EXPR_P with
their corresponding DECL_VALUE_EXPR expressions after unsharing.
* cp-gimplify.cc (cp_fold_r): Revert 2023-01-19 changes.
* g++.dg/cpp1z/decomp57.C: New test.
* g++.dg/cpp1z/decomp58.C: New test.
--- gcc/cgraphbuild.cc.jj 2023-01-02 09:32:34.889104620 +0100
+++ gcc/cgraphbuild.cc 2023-01-23 12:10:35.042067571 +0100
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.
#include "gimple-walk.h"
#include "ipa-utils.h"
#include "except.h"
+#include "gimplify.h"
/* Context of record_reference. */
struct record_reference_ctx
@@ -79,6 +80,17 @@ record_reference (tree *tp, int *walk_su
if (VAR_P (decl))
{
+ /* Replace vars with their DECL_VALUE_EXPR if any.
+ This is normally done during gimplification, but
+ static var initializers are never gimplified. */
+ if (DECL_HAS_VALUE_EXPR_P (decl))
+ {
+ tree *p;
+ for (p = tp; *p != decl; p = &TREE_OPERAND (*p, 0))
+ ;
+ *p = unshare_expr (DECL_VALUE_EXPR (decl));
+ return record_reference (tp, walk_subtrees, data);
+ }
varpool_node *vnode = varpool_node::get_create (decl);
ctx->varpool_node->create_reference (vnode, IPA_REF_ADDR);
}
--- gcc/cp/cp-gimplify.cc.jj 2023-01-23 11:47:49.889875804 +0100
+++ gcc/cp/cp-gimplify.cc 2023-01-23 11:49:01.227841759 +0100
@@ -1010,16 +1010,6 @@ cp_fold_r (tree *stmt_p, int *walk_subtr
}
break;
- case VAR_DECL:
- /* In initializers replace anon union artificial VAR_DECLs
- with their DECL_VALUE_EXPRs, as nothing will do it later. */
- if (DECL_ANON_UNION_VAR_P (stmt) && !data->genericize)
- {
- *stmt_p = stmt = unshare_expr (DECL_VALUE_EXPR (stmt));
- break;
- }
- break;
-
default:
break;
}
--- gcc/testsuite/g++.dg/cpp1z/decomp57.C.jj 2023-01-23 11:48:36.367202107 +0100
+++ gcc/testsuite/g++.dg/cpp1z/decomp57.C 2023-01-23 11:48:36.367202107 +0100
@@ -0,0 +1,27 @@
+// PR c++/108474
+// { dg-do link { target c++17 } }
+
+struct T { int i, j; };
+T h;
+auto [i, j] = h;
+int &r = i;
+int s = i;
+int *t = &i;
+
+void
+foo (int **p, int *q)
+{
+ static int &u = i;
+ static int v = i;
+ static int *w = &i;
+ int &x = i;
+ int y = i;
+ int *z = &i;
+ *p = &i;
+ *q = i;
+}
+
+int
+main ()
+{
+}
--- gcc/testsuite/g++.dg/cpp1z/decomp58.C.jj 2023-01-23 11:48:36.367202107 +0100
+++ gcc/testsuite/g++.dg/cpp1z/decomp58.C 2023-01-23 11:48:36.367202107 +0100
@@ -0,0 +1,39 @@
+// PR c++/108474
+// { dg-do link { target c++17 } }
+
+namespace std {
+ template <typename T> struct tuple_size;
+ template <int, typename> struct tuple_element;
+}
+
+struct A {
+ int i;
+ template <int I> int& get() { return i; }
+};
+
+template <> struct std::tuple_size <A> { static const int value = 2; };
+template <int I> struct std::tuple_element <I, A> { using type = int; };
+
+struct A a;
+auto [i, j] = a;
+int &r = i;
+int s = i;
+int *t = &i;
+
+void
+foo (int **p, int *q)
+{
+ static int &u = i;
+ static int v = i;
+ static int *w = &i;
+ int &x = i;
+ int y = i;
+ int *z = &i;
+ *p = &i;
+ *q = i;
+}
+
+int
+main ()
+{
+}
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c++, cgraphbuild: Handle DECL_VALUE_EXPRs in record_reference [PR108474]
2023-01-23 11:16 ` [PATCH] c++, cgraphbuild: Handle DECL_VALUE_EXPRs in record_reference [PR108474] Jakub Jelinek
@ 2023-01-23 12:37 ` Richard Biener
2023-01-23 15:17 ` Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2023-01-23 12:37 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, Jan Hubicka, gcc-patches
On Mon, 23 Jan 2023, Jakub Jelinek wrote:
> On Mon, Jan 23, 2023 at 09:25:50AM +0100, Jakub Jelinek via Gcc-patches wrote:
> > On Sun, Jan 22, 2023 at 07:19:07PM -0500, Jason Merrill wrote:
> > > > I was just trying to be careful, because unfortunately this spot
> > > > doesn't mean it really is only expanded in static var DECL_INITIAL,
> > > > it can make it into dynamic initializers, and most of DECL_VALUE_EXPRs
> > > > appear only in runtime code, otherwise we'd have much more of these issues.
> > >
> > > But it shouldn't be harmful anywhere, right?
> > >
> > > > But if you think it is ok, I'll test tonight a version just with
> > > > if (!data->genericize && DECL_HAS_VALUE_EXPR_P (stmt)
> > >
> > > OK with that change.
> > >
> > > Though, actually, why not instead fix expand_expr_real_1 (and staticp) to
> > > look through DECL_VALUE_EXPR?
> >
> > I'm afraid that is way too late. GIMPLE optimizations don't try to
> > regimplify expressions they take from DECL_INITIAL of static vars, they
> > just unshare them and use them. So, if this is to be done in the generic
> > code, it would need to be done by cgraph around the time when it gimplifies
> > function if there is any such point for variables.
>
> I believe the right spot is record_reference, which is called through
> walk_tree from record_references_in_initializer which is called from
> varpool_node::analyze.
>
> The new tests as well as g++.dg/init/pr53932.C pass with it, will do full
> bootstrap/regtest soon.
>
> What do you think about this?
Guess it should work. Do we (accidentially?) do anything special
to static var initializers in nested functions? Can you think of
any other C/C++ construct that would have DECL_VALUE_EXPR in them?
Richard.
> 2023-01-23 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/108474
> * cgraphbuild.cc: Include gimplify.h.
> (record_reference): Replace VAR_DECLs with DECL_HAS_VALUE_EXPR_P with
> their corresponding DECL_VALUE_EXPR expressions after unsharing.
>
> * cp-gimplify.cc (cp_fold_r): Revert 2023-01-19 changes.
>
> * g++.dg/cpp1z/decomp57.C: New test.
> * g++.dg/cpp1z/decomp58.C: New test.
>
> --- gcc/cgraphbuild.cc.jj 2023-01-02 09:32:34.889104620 +0100
> +++ gcc/cgraphbuild.cc 2023-01-23 12:10:35.042067571 +0100
> @@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.
> #include "gimple-walk.h"
> #include "ipa-utils.h"
> #include "except.h"
> +#include "gimplify.h"
>
> /* Context of record_reference. */
> struct record_reference_ctx
> @@ -79,6 +80,17 @@ record_reference (tree *tp, int *walk_su
>
> if (VAR_P (decl))
> {
> + /* Replace vars with their DECL_VALUE_EXPR if any.
> + This is normally done during gimplification, but
> + static var initializers are never gimplified. */
> + if (DECL_HAS_VALUE_EXPR_P (decl))
> + {
> + tree *p;
> + for (p = tp; *p != decl; p = &TREE_OPERAND (*p, 0))
> + ;
> + *p = unshare_expr (DECL_VALUE_EXPR (decl));
> + return record_reference (tp, walk_subtrees, data);
> + }
> varpool_node *vnode = varpool_node::get_create (decl);
> ctx->varpool_node->create_reference (vnode, IPA_REF_ADDR);
> }
> --- gcc/cp/cp-gimplify.cc.jj 2023-01-23 11:47:49.889875804 +0100
> +++ gcc/cp/cp-gimplify.cc 2023-01-23 11:49:01.227841759 +0100
> @@ -1010,16 +1010,6 @@ cp_fold_r (tree *stmt_p, int *walk_subtr
> }
> break;
>
> - case VAR_DECL:
> - /* In initializers replace anon union artificial VAR_DECLs
> - with their DECL_VALUE_EXPRs, as nothing will do it later. */
> - if (DECL_ANON_UNION_VAR_P (stmt) && !data->genericize)
> - {
> - *stmt_p = stmt = unshare_expr (DECL_VALUE_EXPR (stmt));
> - break;
> - }
> - break;
> -
> default:
> break;
> }
> --- gcc/testsuite/g++.dg/cpp1z/decomp57.C.jj 2023-01-23 11:48:36.367202107 +0100
> +++ gcc/testsuite/g++.dg/cpp1z/decomp57.C 2023-01-23 11:48:36.367202107 +0100
> @@ -0,0 +1,27 @@
> +// PR c++/108474
> +// { dg-do link { target c++17 } }
> +
> +struct T { int i, j; };
> +T h;
> +auto [i, j] = h;
> +int &r = i;
> +int s = i;
> +int *t = &i;
> +
> +void
> +foo (int **p, int *q)
> +{
> + static int &u = i;
> + static int v = i;
> + static int *w = &i;
> + int &x = i;
> + int y = i;
> + int *z = &i;
> + *p = &i;
> + *q = i;
> +}
> +
> +int
> +main ()
> +{
> +}
> --- gcc/testsuite/g++.dg/cpp1z/decomp58.C.jj 2023-01-23 11:48:36.367202107 +0100
> +++ gcc/testsuite/g++.dg/cpp1z/decomp58.C 2023-01-23 11:48:36.367202107 +0100
> @@ -0,0 +1,39 @@
> +// PR c++/108474
> +// { dg-do link { target c++17 } }
> +
> +namespace std {
> + template <typename T> struct tuple_size;
> + template <int, typename> struct tuple_element;
> +}
> +
> +struct A {
> + int i;
> + template <int I> int& get() { return i; }
> +};
> +
> +template <> struct std::tuple_size <A> { static const int value = 2; };
> +template <int I> struct std::tuple_element <I, A> { using type = int; };
> +
> +struct A a;
> +auto [i, j] = a;
> +int &r = i;
> +int s = i;
> +int *t = &i;
> +
> +void
> +foo (int **p, int *q)
> +{
> + static int &u = i;
> + static int v = i;
> + static int *w = &i;
> + int &x = i;
> + int y = i;
> + int *z = &i;
> + *p = &i;
> + *q = i;
> +}
> +
> +int
> +main ()
> +{
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c++, cgraphbuild: Handle DECL_VALUE_EXPRs in record_reference [PR108474]
2023-01-23 12:37 ` Richard Biener
@ 2023-01-23 15:17 ` Jakub Jelinek
2023-01-23 15:22 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2023-01-23 15:17 UTC (permalink / raw)
To: Richard Biener; +Cc: Jason Merrill, Jan Hubicka, gcc-patches
On Mon, Jan 23, 2023 at 12:37:59PM +0000, Richard Biener wrote:
> Guess it should work. Do we (accidentially?) do anything special
> to static var initializers in nested functions?
I don't think we touch those, we walk the bodies of functions, I don't
think we traverse static var DECL_INITIALs.
> Can you think of
> any other C/C++ construct that would have DECL_VALUE_EXPR in them?
A lot of them, coroutines, FE NRV, anon union vars, lambdas,
in modules, OpenMP privatized members, structured bindings,
__FUNCTION__ etc., later VLAs, tree-nested, OpenMP lowering.
But anon union vars and structured bindings are the only ones
known for which this is actually needed in static initializers.
I'm afraid no idea about modules case, for OpenMP one needs executable
code, tried the __FUNCTION__ case and while there is DECL_VALUE_EXPR
used, the initializer is already replaced by STRING_CST in the FE somewhere,
VLAs aren't allowed.
BTW, the patch successfully passed bootstrap/regtested on x86_64-linux and
i686-linux.
> > 2023-01-23 Jakub Jelinek <jakub@redhat.com>
> >
> > PR c++/108474
> > * cgraphbuild.cc: Include gimplify.h.
> > (record_reference): Replace VAR_DECLs with DECL_HAS_VALUE_EXPR_P with
> > their corresponding DECL_VALUE_EXPR expressions after unsharing.
> >
> > * cp-gimplify.cc (cp_fold_r): Revert 2023-01-19 changes.
> >
> > * g++.dg/cpp1z/decomp57.C: New test.
> > * g++.dg/cpp1z/decomp58.C: New test.
> >
> > --- gcc/cgraphbuild.cc.jj 2023-01-02 09:32:34.889104620 +0100
> > +++ gcc/cgraphbuild.cc 2023-01-23 12:10:35.042067571 +0100
> > @@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.
> > #include "gimple-walk.h"
> > #include "ipa-utils.h"
> > #include "except.h"
> > +#include "gimplify.h"
> >
> > /* Context of record_reference. */
> > struct record_reference_ctx
> > @@ -79,6 +80,17 @@ record_reference (tree *tp, int *walk_su
> >
> > if (VAR_P (decl))
> > {
> > + /* Replace vars with their DECL_VALUE_EXPR if any.
> > + This is normally done during gimplification, but
> > + static var initializers are never gimplified. */
> > + if (DECL_HAS_VALUE_EXPR_P (decl))
> > + {
> > + tree *p;
> > + for (p = tp; *p != decl; p = &TREE_OPERAND (*p, 0))
> > + ;
> > + *p = unshare_expr (DECL_VALUE_EXPR (decl));
> > + return record_reference (tp, walk_subtrees, data);
> > + }
> > varpool_node *vnode = varpool_node::get_create (decl);
> > ctx->varpool_node->create_reference (vnode, IPA_REF_ADDR);
> > }
> > --- gcc/cp/cp-gimplify.cc.jj 2023-01-23 11:47:49.889875804 +0100
> > +++ gcc/cp/cp-gimplify.cc 2023-01-23 11:49:01.227841759 +0100
> > @@ -1010,16 +1010,6 @@ cp_fold_r (tree *stmt_p, int *walk_subtr
> > }
> > break;
> >
> > - case VAR_DECL:
> > - /* In initializers replace anon union artificial VAR_DECLs
> > - with their DECL_VALUE_EXPRs, as nothing will do it later. */
> > - if (DECL_ANON_UNION_VAR_P (stmt) && !data->genericize)
> > - {
> > - *stmt_p = stmt = unshare_expr (DECL_VALUE_EXPR (stmt));
> > - break;
> > - }
> > - break;
> > -
> > default:
> > break;
> > }
> > --- gcc/testsuite/g++.dg/cpp1z/decomp57.C.jj 2023-01-23 11:48:36.367202107 +0100
> > +++ gcc/testsuite/g++.dg/cpp1z/decomp57.C 2023-01-23 11:48:36.367202107 +0100
> > @@ -0,0 +1,27 @@
> > +// PR c++/108474
> > +// { dg-do link { target c++17 } }
> > +
> > +struct T { int i, j; };
> > +T h;
> > +auto [i, j] = h;
> > +int &r = i;
> > +int s = i;
> > +int *t = &i;
> > +
> > +void
> > +foo (int **p, int *q)
> > +{
> > + static int &u = i;
> > + static int v = i;
> > + static int *w = &i;
> > + int &x = i;
> > + int y = i;
> > + int *z = &i;
> > + *p = &i;
> > + *q = i;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > +}
> > --- gcc/testsuite/g++.dg/cpp1z/decomp58.C.jj 2023-01-23 11:48:36.367202107 +0100
> > +++ gcc/testsuite/g++.dg/cpp1z/decomp58.C 2023-01-23 11:48:36.367202107 +0100
> > @@ -0,0 +1,39 @@
> > +// PR c++/108474
> > +// { dg-do link { target c++17 } }
> > +
> > +namespace std {
> > + template <typename T> struct tuple_size;
> > + template <int, typename> struct tuple_element;
> > +}
> > +
> > +struct A {
> > + int i;
> > + template <int I> int& get() { return i; }
> > +};
> > +
> > +template <> struct std::tuple_size <A> { static const int value = 2; };
> > +template <int I> struct std::tuple_element <I, A> { using type = int; };
> > +
> > +struct A a;
> > +auto [i, j] = a;
> > +int &r = i;
> > +int s = i;
> > +int *t = &i;
> > +
> > +void
> > +foo (int **p, int *q)
> > +{
> > + static int &u = i;
> > + static int v = i;
> > + static int *w = &i;
> > + int &x = i;
> > + int y = i;
> > + int *z = &i;
> > + *p = &i;
> > + *q = i;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > +}
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c++, cgraphbuild: Handle DECL_VALUE_EXPRs in record_reference [PR108474]
2023-01-23 15:17 ` Jakub Jelinek
@ 2023-01-23 15:22 ` Richard Biener
0 siblings, 0 replies; 9+ messages in thread
From: Richard Biener @ 2023-01-23 15:22 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, Jan Hubicka, gcc-patches
On Mon, 23 Jan 2023, Jakub Jelinek wrote:
> On Mon, Jan 23, 2023 at 12:37:59PM +0000, Richard Biener wrote:
> > Guess it should work. Do we (accidentially?) do anything special
> > to static var initializers in nested functions?
>
> I don't think we touch those, we walk the bodies of functions, I don't
> think we traverse static var DECL_INITIALs.
>
> > Can you think of
> > any other C/C++ construct that would have DECL_VALUE_EXPR in them?
>
> A lot of them, coroutines, FE NRV, anon union vars, lambdas,
> in modules, OpenMP privatized members, structured bindings,
> __FUNCTION__ etc., later VLAs, tree-nested, OpenMP lowering.
>
> But anon union vars and structured bindings are the only ones
> known for which this is actually needed in static initializers.
> I'm afraid no idea about modules case, for OpenMP one needs executable
> code, tried the __FUNCTION__ case and while there is DECL_VALUE_EXPR
> used, the initializer is already replaced by STRING_CST in the FE somewhere,
> VLAs aren't allowed.
>
> BTW, the patch successfully passed bootstrap/regtested on x86_64-linux and
> i686-linux.
So let's try and revert quickly if any odd fallout appears?
Richard.
>
> > > 2023-01-23 Jakub Jelinek <jakub@redhat.com>
> > >
> > > PR c++/108474
> > > * cgraphbuild.cc: Include gimplify.h.
> > > (record_reference): Replace VAR_DECLs with DECL_HAS_VALUE_EXPR_P with
> > > their corresponding DECL_VALUE_EXPR expressions after unsharing.
> > >
> > > * cp-gimplify.cc (cp_fold_r): Revert 2023-01-19 changes.
> > >
> > > * g++.dg/cpp1z/decomp57.C: New test.
> > > * g++.dg/cpp1z/decomp58.C: New test.
> > >
> > > --- gcc/cgraphbuild.cc.jj 2023-01-02 09:32:34.889104620 +0100
> > > +++ gcc/cgraphbuild.cc 2023-01-23 12:10:35.042067571 +0100
> > > @@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.
> > > #include "gimple-walk.h"
> > > #include "ipa-utils.h"
> > > #include "except.h"
> > > +#include "gimplify.h"
> > >
> > > /* Context of record_reference. */
> > > struct record_reference_ctx
> > > @@ -79,6 +80,17 @@ record_reference (tree *tp, int *walk_su
> > >
> > > if (VAR_P (decl))
> > > {
> > > + /* Replace vars with their DECL_VALUE_EXPR if any.
> > > + This is normally done during gimplification, but
> > > + static var initializers are never gimplified. */
> > > + if (DECL_HAS_VALUE_EXPR_P (decl))
> > > + {
> > > + tree *p;
> > > + for (p = tp; *p != decl; p = &TREE_OPERAND (*p, 0))
> > > + ;
> > > + *p = unshare_expr (DECL_VALUE_EXPR (decl));
> > > + return record_reference (tp, walk_subtrees, data);
> > > + }
> > > varpool_node *vnode = varpool_node::get_create (decl);
> > > ctx->varpool_node->create_reference (vnode, IPA_REF_ADDR);
> > > }
> > > --- gcc/cp/cp-gimplify.cc.jj 2023-01-23 11:47:49.889875804 +0100
> > > +++ gcc/cp/cp-gimplify.cc 2023-01-23 11:49:01.227841759 +0100
> > > @@ -1010,16 +1010,6 @@ cp_fold_r (tree *stmt_p, int *walk_subtr
> > > }
> > > break;
> > >
> > > - case VAR_DECL:
> > > - /* In initializers replace anon union artificial VAR_DECLs
> > > - with their DECL_VALUE_EXPRs, as nothing will do it later. */
> > > - if (DECL_ANON_UNION_VAR_P (stmt) && !data->genericize)
> > > - {
> > > - *stmt_p = stmt = unshare_expr (DECL_VALUE_EXPR (stmt));
> > > - break;
> > > - }
> > > - break;
> > > -
> > > default:
> > > break;
> > > }
> > > --- gcc/testsuite/g++.dg/cpp1z/decomp57.C.jj 2023-01-23 11:48:36.367202107 +0100
> > > +++ gcc/testsuite/g++.dg/cpp1z/decomp57.C 2023-01-23 11:48:36.367202107 +0100
> > > @@ -0,0 +1,27 @@
> > > +// PR c++/108474
> > > +// { dg-do link { target c++17 } }
> > > +
> > > +struct T { int i, j; };
> > > +T h;
> > > +auto [i, j] = h;
> > > +int &r = i;
> > > +int s = i;
> > > +int *t = &i;
> > > +
> > > +void
> > > +foo (int **p, int *q)
> > > +{
> > > + static int &u = i;
> > > + static int v = i;
> > > + static int *w = &i;
> > > + int &x = i;
> > > + int y = i;
> > > + int *z = &i;
> > > + *p = &i;
> > > + *q = i;
> > > +}
> > > +
> > > +int
> > > +main ()
> > > +{
> > > +}
> > > --- gcc/testsuite/g++.dg/cpp1z/decomp58.C.jj 2023-01-23 11:48:36.367202107 +0100
> > > +++ gcc/testsuite/g++.dg/cpp1z/decomp58.C 2023-01-23 11:48:36.367202107 +0100
> > > @@ -0,0 +1,39 @@
> > > +// PR c++/108474
> > > +// { dg-do link { target c++17 } }
> > > +
> > > +namespace std {
> > > + template <typename T> struct tuple_size;
> > > + template <int, typename> struct tuple_element;
> > > +}
> > > +
> > > +struct A {
> > > + int i;
> > > + template <int I> int& get() { return i; }
> > > +};
> > > +
> > > +template <> struct std::tuple_size <A> { static const int value = 2; };
> > > +template <int I> struct std::tuple_element <I, A> { using type = int; };
> > > +
> > > +struct A a;
> > > +auto [i, j] = a;
> > > +int &r = i;
> > > +int s = i;
> > > +int *t = &i;
> > > +
> > > +void
> > > +foo (int **p, int *q)
> > > +{
> > > + static int &u = i;
> > > + static int v = i;
> > > + static int *w = &i;
> > > + int &x = i;
> > > + int y = i;
> > > + int *z = &i;
> > > + *p = &i;
> > > + *q = i;
> > > +}
> > > +
> > > +int
> > > +main ()
> > > +{
> > > +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-01-23 15:22 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-21 9:59 [PATCH] c++: Handle structured bindings like anon unions in initializers [PR108474] Jakub Jelinek
2023-01-22 20:40 ` Jason Merrill
2023-01-22 20:50 ` Jakub Jelinek
2023-01-23 0:19 ` Jason Merrill
2023-01-23 8:25 ` Jakub Jelinek
2023-01-23 11:16 ` [PATCH] c++, cgraphbuild: Handle DECL_VALUE_EXPRs in record_reference [PR108474] Jakub Jelinek
2023-01-23 12:37 ` Richard Biener
2023-01-23 15:17 ` Jakub Jelinek
2023-01-23 15:22 ` 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).