Re: [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)

[Jakub Jelinek]

On Wed, Dec 07, 2016 at 02:42:12PM -0700, Jeff Law wrote:
> >2016-11-16  Jakub Jelinek  
> >
> > 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

Re: [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)

[Jeff Law]

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 >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 (_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  

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

Re: [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)

[Richard Biener]

On Wed, 16 Nov 2016, Jason Merrill wrote:

> On Wed, Nov 16, 2016 at 4:00 PM, Jakub Jelinek  wrote:
> > Jason's recent patch to turn reference vars initialized with invariant
> > addresses broke the first testcase below, because >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 (_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.

Re: [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)

[Jakub Jelinek]

On Wed, Nov 16, 2016 at 04:26:36PM -0500, Jason Merrill wrote:
> On Wed, Nov 16, 2016 at 4:00 PM, Jakub Jelinek  wrote:
> > Jason's recent patch to turn reference vars initialized with invariant
> > addresses broke the first testcase below, because >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 (_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 
> 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 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

Re: [PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)

[Jason Merrill]

On Wed, Nov 16, 2016 at 4:00 PM, Jakub Jelinek  wrote:
> Jason's recent patch to turn reference vars initialized with invariant
> addresses broke the first testcase below, because >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 (_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?
commit 6cdd28bb152fcb07a7eb6c9f053cd435cf719a20
Author: Jason Merrill 
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** 
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);
 

[PATCH] Fix up ICEs with TREE_CONSTANT references (PR c++/78373)

[Jakub Jelinek]

Hi!

Jason's recent patch to turn reference vars initialized with invariant
addresses broke the first testcase below, because >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 (_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  

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.0 +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.jj2016-11-16 09:23:46.0 +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)
+  &&