[PATCH] c++: Don't defer local statics initialized with constant expressions [PR108702]

[PATCH] c++: Don't defer local statics initialized with constant expressions [PR108702]

[Jakub Jelinek]

Hi!

The stmtexpr19.C testcase used to be rejected as it has a static
variable in statement expression in constexpr context, but as that
static variable is initialized by constant expression, when P2647R1
was implemented we agreed to make it valid.

Now, as reported, the testcase compiles fine, but doesn't actually link
because the static variable isn't defined anywhere, and with -flto ICEs
because of this problem.  This is because we never
varpool_node::finalize_decl those vars, the constant expression in which
the DECL_EXPR is present for the static VAR_DECL is folded (constant
evaluated) into just the address of the VAR_DECL.
Now, similar testcase included below (do we want to include it in the
testsuite too?) works fine, because in
cp_finish_decl -> make_rtl_for_nonlocal_decl
we have since PR70353 fix:
  /* We defer emission of local statics until the corresponding
     DECL_EXPR is expanded.  But with constexpr its function might never
     be expanded, so go ahead and tell cgraph about the variable now.  */
  defer_p = ((DECL_FUNCTION_SCOPE_P (decl)
              && !var_in_maybe_constexpr_fn (decl))
             || DECL_VIRTUAL_P (decl));
and so don't defer them in constexpr/consteval functions.  The following
patch extends that and doesn't defer vars initialized by constant
expressions either, because otherwise there is nothing to finalize those.
It is true that e.g. with -O0
int foo (int x) {
  if (x) { static int y = 1; ++y; }
  if (0) { static int z = 1; ++z; }
  return sizeof (({ static int w = 1; w; }));
}
we used to emit just y and z and with the patch emit also w, but with
optimizations that is optimized away properly.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

The testcase I was talking above that works because of the
&& !var_in_maybe_constexpr_fn (decl) case is:

extern "C" void abort ();

constexpr const int *
foo ()
{
  static constexpr int a = 1;
  return &a;
}

consteval const int *
bar ()
{
  static constexpr int a = 1;
  return &a;
}

[[gnu::noipa]] void
baz (const int *x)
{
  if (*x != 1)
    abort ();
}

int
main ()
{
  constexpr const int *p = foo ();
  constexpr const int *q = bar ();
  baz (p);
  baz (q);
  if (p == q)
    abort ();
}

2023-02-09  Jakub Jelinek  <jakub@redhat.com>

	PR c++/108702
	* decl.cc (make_rtl_for_nonlocal_decl): Don't defer local statics
	initialized by constant expressions.

	* g++.dg/ext/stmtexpr19.C: Use dg-do link rather than dg-do compile.

--- gcc/cp/decl.cc.jj	2023-01-24 11:10:13.151076134 +0100
+++ gcc/cp/decl.cc	2023-02-09 13:29:50.527083618 +0100
@@ -7731,9 +7731,12 @@ make_rtl_for_nonlocal_decl (tree decl, t
 
   /* We defer emission of local statics until the corresponding
      DECL_EXPR is expanded.  But with constexpr its function might never
-     be expanded, so go ahead and tell cgraph about the variable now.  */
+     be expanded, so go ahead and tell cgraph about the variable now.
+     Also don't defer local statics initialized by constant expressions,
+     see PR108702.  */
   defer_p = ((DECL_FUNCTION_SCOPE_P (decl)
-	      && !var_in_maybe_constexpr_fn (decl))
+	      && !var_in_maybe_constexpr_fn (decl)
+	      && !DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl))
 	     || DECL_VIRTUAL_P (decl));
 
   /* Defer template instantiations.  */
--- gcc/testsuite/g++.dg/ext/stmtexpr19.C.jj	2022-11-19 09:26:30.168061316 +0100
+++ gcc/testsuite/g++.dg/ext/stmtexpr19.C	2023-02-09 13:32:48.887453520 +0100
@@ -1,6 +1,6 @@
 // PR c++/81073
 // { dg-options "" }
-// { dg-do compile { target c++11 } }
+// { dg-do link { target c++11 } }
 
 struct test { const int *addr; };
 

	Jakub

Re: [PATCH] c++: Don't defer local statics initialized with constant expressions [PR108702]

[Jason Merrill]

On 2/9/23 11:14, Jakub Jelinek wrote:
> Hi!
> 
> The stmtexpr19.C testcase used to be rejected as it has a static
> variable in statement expression in constexpr context, but as that
> static variable is initialized by constant expression, when P2647R1
> was implemented we agreed to make it valid.
> 
> Now, as reported, the testcase compiles fine, but doesn't actually link
> because the static variable isn't defined anywhere, and with -flto ICEs
> because of this problem.  This is because we never
> varpool_node::finalize_decl those vars, the constant expression in which
> the DECL_EXPR is present for the static VAR_DECL is folded (constant
> evaluated) into just the address of the VAR_DECL.

Would it make sense to define it when we see the DECL_EXPR in constant 
evaluation?

> Now, similar testcase included below (do we want to include it in the
> testsuite too?) works fine, because in
> cp_finish_decl -> make_rtl_for_nonlocal_decl
> we have since PR70353 fix:
>    /* We defer emission of local statics until the corresponding
>       DECL_EXPR is expanded.  But with constexpr its function might never
>       be expanded, so go ahead and tell cgraph about the variable now.  */
>    defer_p = ((DECL_FUNCTION_SCOPE_P (decl)
>                && !var_in_maybe_constexpr_fn (decl))
>               || DECL_VIRTUAL_P (decl));
> and so don't defer them in constexpr/consteval functions.  The following
> patch extends that and doesn't defer vars initialized by constant
> expressions either, because otherwise there is nothing to finalize those.
> It is true that e.g. with -O0
> int foo (int x) {
>    if (x) { static int y = 1; ++y; }
>    if (0) { static int z = 1; ++z; }
>    return sizeof (({ static int w = 1; w; }));
> }
> we used to emit just y and z and with the patch emit also w, but with
> optimizations that is optimized away properly.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> The testcase I was talking above that works because of the
> && !var_in_maybe_constexpr_fn (decl) case is:
> 
> extern "C" void abort ();
> 
> constexpr const int *
> foo ()
> {
>    static constexpr int a = 1;
>    return &a;
> }
> 
> consteval const int *
> bar ()
> {
>    static constexpr int a = 1;
>    return &a;
> }
> 
> [[gnu::noipa]] void
> baz (const int *x)
> {
>    if (*x != 1)
>      abort ();
> }
> 
> int
> main ()
> {
>    constexpr const int *p = foo ();
>    constexpr const int *q = bar ();
>    baz (p);
>    baz (q);
>    if (p == q)
>      abort ();
> }
> 
> 2023-02-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/108702
> 	* decl.cc (make_rtl_for_nonlocal_decl): Don't defer local statics
> 	initialized by constant expressions.
> 
> 	* g++.dg/ext/stmtexpr19.C: Use dg-do link rather than dg-do compile.
> 
> --- gcc/cp/decl.cc.jj	2023-01-24 11:10:13.151076134 +0100
> +++ gcc/cp/decl.cc	2023-02-09 13:29:50.527083618 +0100
> @@ -7731,9 +7731,12 @@ make_rtl_for_nonlocal_decl (tree decl, t
>   
>     /* We defer emission of local statics until the corresponding
>        DECL_EXPR is expanded.  But with constexpr its function might never
> -     be expanded, so go ahead and tell cgraph about the variable now.  */
> +     be expanded, so go ahead and tell cgraph about the variable now.
> +     Also don't defer local statics initialized by constant expressions,
> +     see PR108702.  */
>     defer_p = ((DECL_FUNCTION_SCOPE_P (decl)
> -	      && !var_in_maybe_constexpr_fn (decl))
> +	      && !var_in_maybe_constexpr_fn (decl)
> +	      && !DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl))
>   	     || DECL_VIRTUAL_P (decl));
>   
>     /* Defer template instantiations.  */
> --- gcc/testsuite/g++.dg/ext/stmtexpr19.C.jj	2022-11-19 09:26:30.168061316 +0100
> +++ gcc/testsuite/g++.dg/ext/stmtexpr19.C	2023-02-09 13:32:48.887453520 +0100
> @@ -1,6 +1,6 @@
>   // PR c++/81073
>   // { dg-options "" }
> -// { dg-do compile { target c++11 } }
> +// { dg-do link { target c++11 } }
>   
>   struct test { const int *addr; };
>   
> 
> 	Jakub
> 

[PATCH] c++, v2: Don't defer local statics initialized with constant expressions [PR108702]

[Jakub Jelinek]

On Thu, Mar 02, 2023 at 11:48:04AM -0500, Jason Merrill wrote:
> > The stmtexpr19.C testcase used to be rejected as it has a static
> > variable in statement expression in constexpr context, but as that
> > static variable is initialized by constant expression, when P2647R1
> > was implemented we agreed to make it valid.
> > 
> > Now, as reported, the testcase compiles fine, but doesn't actually link
> > because the static variable isn't defined anywhere, and with -flto ICEs
> > because of this problem.  This is because we never
> > varpool_node::finalize_decl those vars, the constant expression in which
> > the DECL_EXPR is present for the static VAR_DECL is folded (constant
> > evaluated) into just the address of the VAR_DECL.
> 
> Would it make sense to define it when we see the DECL_EXPR in constant
> evaluation?

So like this?
Passes GXX_TESTSUITE_STDS=98,11,14,17,20,2b make check-g++ so far.

2023-03-03  Jakub Jelinek  <jakub@redhat.com>

	PR c++/108702
	* constexpr.cc: Include toplev.h.
	(cxx_eval_constant_expression) <case DECL_EXPR>: When seeing a local
	static initialized by constant expression outside of a constexpr
	function which has been deferred by make_rtl_for_nonlocal_decl,
	call rest_of_decl_compilation on it.

	* g++.dg/ext/stmtexpr19.C: Use dg-do link rather than dg-do compile.

--- gcc/cp/constexpr.cc.jj	2023-03-03 00:34:44.113679918 +0100
+++ gcc/cp/constexpr.cc	2023-03-03 13:26:57.602871900 +0100
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3.
 #include "attribs.h"
 #include "fold-const.h"
 #include "intl.h"
+#include "toplev.h"
 
 static bool verify_constant (tree, bool, bool *, bool *);
 #define VERIFY_CONSTANT(X)						\
@@ -7127,6 +7128,24 @@ cxx_eval_constant_expression (const cons
 	    break;
 	  }
 
+	/* make_rtl_for_nonlocal_decl could have deferred emission of
+	   a local static var, but if it appears in a statement expression
+	   which is constant expression evaluated to e.g. just the address
+	   of the variable, its DECL_EXPR will never be seen during
+	   gimple lowering's record_vars_into as the statement expression
+	   will not be in the IL at all.  */
+	if (VAR_P (r)
+	    && TREE_STATIC (r)
+	    && !DECL_REALLY_EXTERN (r)
+	    && DECL_FUNCTION_SCOPE_P (r)
+	    && !var_in_maybe_constexpr_fn (r)
+	    && decl_constant_var_p (r))
+	  {
+	    varpool_node *node = varpool_node::get (r);
+	    if (node == NULL || !node->definition)
+	      rest_of_decl_compilation (r, 0, at_eof);
+	  }
+
 	if (AGGREGATE_TYPE_P (TREE_TYPE (r))
 	    || VECTOR_TYPE_P (TREE_TYPE (r)))
 	  {
--- gcc/testsuite/g++.dg/ext/stmtexpr19.C.jj	2023-02-09 15:52:29.623359240 +0100
+++ gcc/testsuite/g++.dg/ext/stmtexpr19.C	2023-03-03 12:24:20.217186735 +0100
@@ -1,6 +1,6 @@
 // PR c++/81073
 // { dg-options "" }
-// { dg-do compile { target c++11 } }
+// { dg-do link { target c++11 } }
 
 struct test { const int *addr; };
 


	Jakub

Re: [PATCH] c++, v2: Don't defer local statics initialized with constant expressions [PR108702]

[Jason Merrill]

On 3/3/23 10:18, Jakub Jelinek wrote:
> On Thu, Mar 02, 2023 at 11:48:04AM -0500, Jason Merrill wrote:
>>> The stmtexpr19.C testcase used to be rejected as it has a static
>>> variable in statement expression in constexpr context, but as that
>>> static variable is initialized by constant expression, when P2647R1
>>> was implemented we agreed to make it valid.
>>>
>>> Now, as reported, the testcase compiles fine, but doesn't actually link
>>> because the static variable isn't defined anywhere, and with -flto ICEs
>>> because of this problem.  This is because we never
>>> varpool_node::finalize_decl those vars, the constant expression in which
>>> the DECL_EXPR is present for the static VAR_DECL is folded (constant
>>> evaluated) into just the address of the VAR_DECL.
>>
>> Would it make sense to define it when we see the DECL_EXPR in constant
>> evaluation?
> 
> So like this?

OK, thanks.

> Passes GXX_TESTSUITE_STDS=98,11,14,17,20,2b make check-g++ so far.
> 
> 2023-03-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/108702
> 	* constexpr.cc: Include toplev.h.
> 	(cxx_eval_constant_expression) <case DECL_EXPR>: When seeing a local
> 	static initialized by constant expression outside of a constexpr
> 	function which has been deferred by make_rtl_for_nonlocal_decl,
> 	call rest_of_decl_compilation on it.
> 
> 	* g++.dg/ext/stmtexpr19.C: Use dg-do link rather than dg-do compile.
> 
> --- gcc/cp/constexpr.cc.jj	2023-03-03 00:34:44.113679918 +0100
> +++ gcc/cp/constexpr.cc	2023-03-03 13:26:57.602871900 +0100
> @@ -40,6 +40,7 @@ along with GCC; see the file COPYING3.
>   #include "attribs.h"
>   #include "fold-const.h"
>   #include "intl.h"
> +#include "toplev.h"
>   
>   static bool verify_constant (tree, bool, bool *, bool *);
>   #define VERIFY_CONSTANT(X)						\
> @@ -7127,6 +7128,24 @@ cxx_eval_constant_expression (const cons
>   	    break;
>   	  }
>   
> +	/* make_rtl_for_nonlocal_decl could have deferred emission of
> +	   a local static var, but if it appears in a statement expression
> +	   which is constant expression evaluated to e.g. just the address
> +	   of the variable, its DECL_EXPR will never be seen during
> +	   gimple lowering's record_vars_into as the statement expression
> +	   will not be in the IL at all.  */
> +	if (VAR_P (r)
> +	    && TREE_STATIC (r)
> +	    && !DECL_REALLY_EXTERN (r)
> +	    && DECL_FUNCTION_SCOPE_P (r)
> +	    && !var_in_maybe_constexpr_fn (r)
> +	    && decl_constant_var_p (r))
> +	  {
> +	    varpool_node *node = varpool_node::get (r);
> +	    if (node == NULL || !node->definition)
> +	      rest_of_decl_compilation (r, 0, at_eof);
> +	  }
> +
>   	if (AGGREGATE_TYPE_P (TREE_TYPE (r))
>   	    || VECTOR_TYPE_P (TREE_TYPE (r)))
>   	  {
> --- gcc/testsuite/g++.dg/ext/stmtexpr19.C.jj	2023-02-09 15:52:29.623359240 +0100
> +++ gcc/testsuite/g++.dg/ext/stmtexpr19.C	2023-03-03 12:24:20.217186735 +0100
> @@ -1,6 +1,6 @@
>   // PR c++/81073
>   // { dg-options "" }
> -// { dg-do compile { target c++11 } }
> +// { dg-do link { target c++11 } }
>   
>   struct test { const int *addr; };
>   
> 
> 
> 	Jakub
>