* [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)
@ 2016-11-16 21:00 Jakub Jelinek
2016-11-16 21:27 ` Jason Merrill
2016-12-07 21:42 ` Jeff Law
0 siblings, 2 replies; 6+ messages in thread
From: Jakub Jelinek @ 2016-11-16 21:00 UTC (permalink / raw)
To: Richard Biener, Jason Merrill; +Cc: gcc-patches
Hi!
Jason's recent patch to turn reference vars initialized with invariant
addresses broke the first testcase below, because &self->singleton
is considered TREE_CONSTANT (because self is TREE_CONSTANT VAR_DECL and
singleton field has constant offset), but after going into SSA form
it is not supposed to be TREE_CONSTANT anymore (&self_2->singleton),
because SSA_NAMEs don't have TREE_CONSTANT set on them.
The following patch fixes it by gimplifying such vars into their
DECL_INITIAL unless in OpenMP regions, where such folding is deferred
until omplower pass finishes.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-11-16 Jakub Jelinek <jakub@redhat.com>
PR c++/78373
* gimplify.c (gimplify_decl_expr): For TREE_CONSTANT reference
vars with is_gimple_min_invariant initializer after stripping
useless conversions keep non-NULL DECL_INITIAL.
(gimplify_var_or_parm_decl): Add fallback argument. Gimplify
TREE_CONSTANT reference vars with is_gimple_min_invariant initializer
outside of OpenMP contexts to the initializer if fb_rvalue is
allowed.
(gimplify_compound_lval, gimplify_expr): Pass through fallback
argument to gimplify_var_or_parm_decl.
* omp-low.c (lower_omp_regimplify_p): Return non-zero for
TREE_CONSTANT reference vars with is_gimple_min_invariant
initializer.
* g++.dg/opt/pr78373.C: New test.
* g++.dg/gomp/pr78373.C: New test.
--- gcc/gimplify.c.jj 2016-11-10 12:34:12.000000000 +0100
+++ gcc/gimplify.c 2016-11-16 16:15:13.496510476 +0100
@@ -1645,10 +1645,23 @@ gimplify_decl_expr (tree *stmt_p, gimple
{
if (!TREE_STATIC (decl))
{
+ tree new_init = NULL_TREE;
+ if (TREE_CONSTANT (decl)
+ && TREE_CODE (TREE_TYPE (decl)) == REFERENCE_TYPE)
+ {
+ new_init = init;
+ STRIP_USELESS_TYPE_CONVERSION (new_init);
+ if (is_gimple_min_invariant (new_init))
+ new_init = unshare_expr (new_init);
+ else
+ new_init = NULL_TREE;
+ }
DECL_INITIAL (decl) = NULL_TREE;
init = build2 (INIT_EXPR, void_type_node, decl, init);
gimplify_and_add (init, seq_p);
ggc_free (init);
+ if (new_init && DECL_INITIAL (decl) == NULL_TREE)
+ DECL_INITIAL (decl) = new_init;
}
else
/* We must still examine initializers for static variables
@@ -2546,7 +2559,7 @@ static tree nonlocal_vla_vars;
DECL_VALUE_EXPR, and it's worth re-examining things. */
static enum gimplify_status
-gimplify_var_or_parm_decl (tree *expr_p)
+gimplify_var_or_parm_decl (tree *expr_p, fallback_t fallback)
{
tree decl = *expr_p;
@@ -2607,6 +2620,29 @@ gimplify_var_or_parm_decl (tree *expr_p)
return GS_OK;
}
+ /* Gimplify TREE_CONSTANT C++ reference vars as their DECL_INITIAL. */
+ if (VAR_P (decl)
+ && TREE_CONSTANT (decl)
+ && TREE_CODE (TREE_TYPE (decl)) == REFERENCE_TYPE
+ && DECL_INITIAL (decl)
+ && (fallback & fb_rvalue) != 0
+ && is_gimple_min_invariant (DECL_INITIAL (decl)))
+ {
+ if (gimplify_omp_ctxp)
+ {
+ /* Don't do this in OpenMP regions, see e.g. libgomp.c++/target-14.C
+ testcase. We'll do that in the omplower pass later instead. */
+ if ((gimplify_omp_ctxp->region_type != ORT_TARGET
+ || gimplify_omp_ctxp->outer_context != NULL
+ || !lookup_attribute ("omp declare target",
+ DECL_ATTRIBUTES (cfun->decl)))
+ && gimplify_omp_ctxp->region_type != ORT_NONE)
+ return GS_ALL_DONE;
+ }
+ *expr_p = unshare_expr (DECL_INITIAL (decl));
+ return GS_OK;
+ }
+
return GS_ALL_DONE;
}
@@ -2712,7 +2748,7 @@ gimplify_compound_lval (tree *expr_p, gi
/* Expand DECL_VALUE_EXPR now. In some cases that may expose
additional COMPONENT_REFs. */
else if ((VAR_P (*p) || TREE_CODE (*p) == PARM_DECL)
- && gimplify_var_or_parm_decl (p) == GS_OK)
+ && gimplify_var_or_parm_decl (p, fallback) == GS_OK)
goto restart;
else
break;
@@ -11624,7 +11660,7 @@ gimplify_expr (tree *expr_p, gimple_seq
case VAR_DECL:
case PARM_DECL:
- ret = gimplify_var_or_parm_decl (expr_p);
+ ret = gimplify_var_or_parm_decl (expr_p, fallback);
break;
case RESULT_DECL:
--- gcc/omp-low.c.jj 2016-11-16 09:23:46.000000000 +0100
+++ gcc/omp-low.c 2016-11-16 15:44:35.071617469 +0100
@@ -16963,6 +16963,14 @@ lower_omp_regimplify_p (tree *tp, int *w
&& bitmap_bit_p (task_shared_vars, DECL_UID (t)))
return t;
+ /* And C++ references with TREE_CONSTANT set too. */
+ if (VAR_P (t)
+ && TREE_CONSTANT (t)
+ && TREE_CODE (TREE_TYPE (t)) == REFERENCE_TYPE
+ && DECL_INITIAL (t)
+ && is_gimple_min_invariant (DECL_INITIAL (t)))
+ return t;
+
/* If a global variable has been privatized, TREE_CONSTANT on
ADDR_EXPR might be wrong. */
if (data == NULL && TREE_CODE (t) == ADDR_EXPR)
--- gcc/testsuite/g++.dg/opt/pr78373.C.jj 2016-11-16 16:26:14.706204090 +0100
+++ gcc/testsuite/g++.dg/opt/pr78373.C 2016-11-16 16:25:46.000000000 +0100
@@ -0,0 +1,22 @@
+// PR c++/78373
+// { dg-do compile { target c++11 } }
+
+struct A {
+ static A singleton;
+};
+struct B {
+ void m_fn2();
+ virtual int m_fn1();
+};
+struct D : B {
+ static int m_fn3(int, int, int, A) {
+ D &self = singleton;
+ self.m_fn2();
+ }
+ static D singleton;
+};
+template <typename, typename> struct C { bool m_fn4() const; };
+template <typename Base, typename Traits> bool C<Base, Traits>::m_fn4() const {
+ Traits::m_fn3(0, 0, 0, Base::singleton);
+}
+template struct C<A, D>;
--- gcc/testsuite/g++.dg/gomp/pr78373.C.jj 2016-11-16 16:26:39.971886570 +0100
+++ gcc/testsuite/g++.dg/gomp/pr78373.C 2016-11-16 16:27:16.506427431 +0100
@@ -0,0 +1,53 @@
+// PR c++/78373
+// { dg-do compile }
+
+void fn ();
+
+const int a = 5;
+int c = 6;
+
+void
+foo (void)
+{
+ const int &b = a;
+ int &d = c;
+ fn ();
+ d += b;
+ fn ();
+ d += b;
+}
+
+void
+bar (void)
+{
+ #pragma omp parallel
+ {
+ const int &b = a;
+ int &d = c;
+ fn ();
+ d += b;
+ fn ();
+ d += b;
+ }
+}
+
+void
+baz (void)
+{
+ const int &b = a;
+ int &d = c;
+ #pragma omp parallel firstprivate (b, d)
+ {
+ fn ();
+ d += b;
+ fn ();
+ d += b;
+ }
+ #pragma omp parallel shared (b, d)
+ {
+ fn ();
+ d += b;
+ fn ();
+ d += b;
+ }
+}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)
2016-11-16 21:00 [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373) Jakub Jelinek
@ 2016-11-16 21:27 ` Jason Merrill
2016-11-16 21:55 ` Jakub Jelinek
2016-11-17 9:00 ` Richard Biener
2016-12-07 21:42 ` Jeff Law
1 sibling, 2 replies; 6+ messages in thread
From: Jason Merrill @ 2016-11-16 21:27 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 916 bytes --]
On Wed, Nov 16, 2016 at 4:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Jason's recent patch to turn reference vars initialized with invariant
> addresses broke the first testcase below, because &self->singleton
> is considered TREE_CONSTANT (because self is TREE_CONSTANT VAR_DECL and
> singleton field has constant offset), but after going into SSA form
> it is not supposed to be TREE_CONSTANT anymore (&self_2->singleton),
> because SSA_NAMEs don't have TREE_CONSTANT set on them.
>
> The following patch fixes it by gimplifying such vars into their
> DECL_INITIAL unless in OpenMP regions, where such folding is deferred
> until omplower pass finishes.
Hmm, this seems like a workaround; why don't we see the same problem
with constant pointer variables?
A simpler workaround would be to not set TREE_CONSTANT on references
in the first place, since the constexpr code doesn't need it. What do
you think?
[-- Attachment #2: const-ref.diff --]
[-- Type: text/plain, Size: 1314 bytes --]
commit 6cdd28bb152fcb07a7eb6c9f053cd435cf719a20
Author: Jason Merrill <jason@redhat.com>
Date: Wed Nov 16 16:13:25 2016 -0500
ref
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index c54a2de..87db589 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6839,7 +6839,8 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
/* Set these flags now for templates. We'll update the flags in
store_init_value for instantiations. */
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1;
- if (decl_maybe_constant_var_p (decl))
+ if (decl_maybe_constant_var_p (decl)
+ && TREE_CODE (type) != REFERENCE_TYPE)
TREE_CONSTANT (decl) = 1;
}
}
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 022a478..dcdb710 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -824,7 +824,8 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags)
const_init = (reduced_constant_expression_p (value)
|| error_operand_p (value));
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = const_init;
- TREE_CONSTANT (decl) = const_init && decl_maybe_constant_var_p (decl);
+ if (TREE_CODE (type) != REFERENCE_TYPE)
+ TREE_CONSTANT (decl) = const_init && decl_maybe_constant_var_p (decl);
}
value = cp_fully_fold (value);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)
2016-11-16 21:27 ` Jason Merrill
@ 2016-11-16 21:55 ` Jakub Jelinek
2016-11-17 9:00 ` Richard Biener
1 sibling, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2016-11-16 21:55 UTC (permalink / raw)
To: Jason Merrill; +Cc: Richard Biener, gcc-patches List
On Wed, Nov 16, 2016 at 04:26:36PM -0500, Jason Merrill wrote:
> On Wed, Nov 16, 2016 at 4:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > Jason's recent patch to turn reference vars initialized with invariant
> > addresses broke the first testcase below, because &self->singleton
> > is considered TREE_CONSTANT (because self is TREE_CONSTANT VAR_DECL and
> > singleton field has constant offset), but after going into SSA form
> > it is not supposed to be TREE_CONSTANT anymore (&self_2->singleton),
> > because SSA_NAMEs don't have TREE_CONSTANT set on them.
> >
> > The following patch fixes it by gimplifying such vars into their
> > DECL_INITIAL unless in OpenMP regions, where such folding is deferred
> > until omplower pass finishes.
>
> Hmm, this seems like a workaround; why don't we see the same problem
> with constant pointer variables?
Dunno, tried to construct a testcase, but it doesn't fail. If it is e.g.
TREE_CONSTANT because of being constexpr, then the FE already replaces it
by its initializer.
> A simpler workaround would be to not set TREE_CONSTANT on references
> in the first place, since the constexpr code doesn't need it. What do
> you think?
If the FE doesn't need it, indeed it would be simpler this way.
> commit 6cdd28bb152fcb07a7eb6c9f053cd435cf719a20
> Author: Jason Merrill <jason@redhat.com>
> Date: Wed Nov 16 16:13:25 2016 -0500
>
> ref
>
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index c54a2de..87db589 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -6839,7 +6839,8 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
> /* Set these flags now for templates. We'll update the flags in
> store_init_value for instantiations. */
> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1;
> - if (decl_maybe_constant_var_p (decl))
> + if (decl_maybe_constant_var_p (decl)
> + && TREE_CODE (type) != REFERENCE_TYPE)
> TREE_CONSTANT (decl) = 1;
> }
> }
> diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
> index 022a478..dcdb710 100644
> --- a/gcc/cp/typeck2.c
> +++ b/gcc/cp/typeck2.c
> @@ -824,7 +824,8 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags)
> const_init = (reduced_constant_expression_p (value)
> || error_operand_p (value));
> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = const_init;
> - TREE_CONSTANT (decl) = const_init && decl_maybe_constant_var_p (decl);
> + if (TREE_CODE (type) != REFERENCE_TYPE)
> + TREE_CONSTANT (decl) = const_init && decl_maybe_constant_var_p (decl);
> }
> value = cp_fully_fold (value);
>
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)
2016-11-16 21:27 ` Jason Merrill
2016-11-16 21:55 ` Jakub Jelinek
@ 2016-11-17 9:00 ` Richard Biener
1 sibling, 0 replies; 6+ messages in thread
From: Richard Biener @ 2016-11-17 9:00 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jakub Jelinek, gcc-patches List
On Wed, 16 Nov 2016, Jason Merrill wrote:
> On Wed, Nov 16, 2016 at 4:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > Jason's recent patch to turn reference vars initialized with invariant
> > addresses broke the first testcase below, because &self->singleton
> > is considered TREE_CONSTANT (because self is TREE_CONSTANT VAR_DECL and
> > singleton field has constant offset), but after going into SSA form
> > it is not supposed to be TREE_CONSTANT anymore (&self_2->singleton),
> > because SSA_NAMEs don't have TREE_CONSTANT set on them.
> >
> > The following patch fixes it by gimplifying such vars into their
> > DECL_INITIAL unless in OpenMP regions, where such folding is deferred
> > until omplower pass finishes.
>
> Hmm, this seems like a workaround; why don't we see the same problem
> with constant pointer variables?
>
> A simpler workaround would be to not set TREE_CONSTANT on references
> in the first place, since the constexpr code doesn't need it. What do
> you think?
If that works then it's certainly my preference at this point.
Richard.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)
2016-11-16 21:00 [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373) Jakub Jelinek
2016-11-16 21:27 ` Jason Merrill
@ 2016-12-07 21:42 ` Jeff Law
2016-12-07 21:44 ` Jakub Jelinek
1 sibling, 1 reply; 6+ messages in thread
From: Jeff Law @ 2016-12-07 21:42 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener, Jason Merrill; +Cc: gcc-patches
On 11/16/2016 02:00 PM, Jakub Jelinek wrote:
> Hi!
>
> Jason's recent patch to turn reference vars initialized with invariant
> addresses broke the first testcase below, because &self->singleton
> is considered TREE_CONSTANT (because self is TREE_CONSTANT VAR_DECL and
> singleton field has constant offset), but after going into SSA form
> it is not supposed to be TREE_CONSTANT anymore (&self_2->singleton),
> because SSA_NAMEs don't have TREE_CONSTANT set on them.
>
> The following patch fixes it by gimplifying such vars into their
> DECL_INITIAL unless in OpenMP regions, where such folding is deferred
> until omplower pass finishes.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2016-11-16 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/78373
> * gimplify.c (gimplify_decl_expr): For TREE_CONSTANT reference
> vars with is_gimple_min_invariant initializer after stripping
> useless conversions keep non-NULL DECL_INITIAL.
> (gimplify_var_or_parm_decl): Add fallback argument. Gimplify
> TREE_CONSTANT reference vars with is_gimple_min_invariant initializer
> outside of OpenMP contexts to the initializer if fb_rvalue is
> allowed.
> (gimplify_compound_lval, gimplify_expr): Pass through fallback
> argument to gimplify_var_or_parm_decl.
> * omp-low.c (lower_omp_regimplify_p): Return non-zero for
> TREE_CONSTANT reference vars with is_gimple_min_invariant
> initializer.
>
> * g++.dg/opt/pr78373.C: New test.
> * g++.dg/gomp/pr78373.C: New test.
Is this still relevant?
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)
2016-12-07 21:42 ` Jeff Law
@ 2016-12-07 21:44 ` Jakub Jelinek
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2016-12-07 21:44 UTC (permalink / raw)
To: Jeff Law; +Cc: Richard Biener, Jason Merrill, gcc-patches
On Wed, Dec 07, 2016 at 02:42:12PM -0700, Jeff Law wrote:
> >2016-11-16 Jakub Jelinek <jakub@redhat.com>
> >
> > PR c++/78373
> > * gimplify.c (gimplify_decl_expr): For TREE_CONSTANT reference
> > vars with is_gimple_min_invariant initializer after stripping
> > useless conversions keep non-NULL DECL_INITIAL.
> > (gimplify_var_or_parm_decl): Add fallback argument. Gimplify
> > TREE_CONSTANT reference vars with is_gimple_min_invariant initializer
> > outside of OpenMP contexts to the initializer if fb_rvalue is
> > allowed.
> > (gimplify_compound_lval, gimplify_expr): Pass through fallback
> > argument to gimplify_var_or_parm_decl.
> > * omp-low.c (lower_omp_regimplify_p): Return non-zero for
> > TREE_CONSTANT reference vars with is_gimple_min_invariant
> > initializer.
> >
> > * g++.dg/opt/pr78373.C: New test.
> > * g++.dg/gomp/pr78373.C: New test.
> Is this still relevant?
No, Jason has committed a simpler change, see the PR.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-07 21:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-16 21:00 [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373) Jakub Jelinek
2016-11-16 21:27 ` Jason Merrill
2016-11-16 21:55 ` Jakub Jelinek
2016-11-17 9:00 ` Richard Biener
2016-12-07 21:42 ` Jeff Law
2016-12-07 21:44 ` Jakub Jelinek
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).