public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Add support for __real__/__imag__ modifications in constant expressions [PR88174]
@ 2022-06-09  8:37 Jakub Jelinek
  2022-06-10 17:27 ` Jason Merrill
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2022-06-09  8:37 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi!

We claim we support P0415R1 (constexpr complex), but e.g.
#include <complex>

constexpr bool
foo ()
{
  std::complex<double> a (1.0, 2.0);
  a += 3.0;
  a.real (6.0);
  return a.real () == 6.0 && a.imag () == 2.0;
}

static_assert (foo ());

fails with
test.C:12:20: error: non-constant condition for static assertion
   12 | static_assert (foo ());
      |                ~~~~^~
test.C:12:20:   in ‘constexpr’ expansion of ‘foo()’
test.C:8:10:   in ‘constexpr’ expansion of ‘a.std::complex<double>::real(6.0e+0)’
test.C:12:20: error: modification of ‘__real__ a.std::complex<double>::_M_value’ is not a constant expression

The problem is we don't handle REALPART_EXPR and IMAGPART_EXPR
in cxx_eval_store_expression.
The following patch attempts to support it (with a requirement
that those are the outermost expressions, ARRAY_REF/COMPONENT_REF
etc. are just not possible on the result of these, BIT_FIELD_REF
would be theoretically possible if trying to extract some bits
from one part of a complex int, but I don't see how it could appear
in the FE trees.

For these references, the code handles value being COMPLEX_CST,
COMPLEX_EXPR or CONSTRUCTOR_NO_CLEARING empty CONSTRUCTOR (what we use
to represent uninitialized values for C++20 and later) and the
code starts by rewriting it to COMPLEX_EXPR, so that we can freely
adjust the individual parts and later on possibly optimize it back
to COMPLEX_CST if both halves are constant.

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

2022-06-09  Jakub Jelinek  <jakub@redhat.com>

	PR c++/88174
	* constexpr.cc (cxx_eval_store_expression): Handle REALPART_EXPR
	and IMAGPART_EXPR.

	* g++.dg/cpp1y/constexpr-complex1.C: New test.

--- gcc/cp/constexpr.cc.jj	2022-06-08 08:21:02.973448193 +0200
+++ gcc/cp/constexpr.cc	2022-06-08 17:13:04.986040449 +0200
@@ -5707,6 +5707,20 @@ cxx_eval_store_expression (const constex
 	  }
 	  break;
 
+	case REALPART_EXPR:
+	  gcc_assert (probe == target);
+	  vec_safe_push (refs, integer_zero_node);
+	  vec_safe_push (refs, TREE_TYPE (probe));
+	  probe = TREE_OPERAND (probe, 0);
+	  break;
+
+	case IMAGPART_EXPR:
+	  gcc_assert (probe == target);
+	  vec_safe_push (refs, integer_one_node);
+	  vec_safe_push (refs, TREE_TYPE (probe));
+	  probe = TREE_OPERAND (probe, 0);
+	  break;
+
 	default:
 	  if (evaluated)
 	    object = probe;
@@ -5749,6 +5763,8 @@ cxx_eval_store_expression (const constex
   auto_vec<int> index_pos_hints;
   bool activated_union_member_p = false;
   bool empty_base = false;
+  int complex_part = -1;
+  tree *complex_expr = NULL;
   while (!refs->is_empty ())
     {
       if (*valp == NULL_TREE)
@@ -5785,14 +5801,36 @@ cxx_eval_store_expression (const constex
 	  *valp = ary_ctor;
 	}
 
-      /* If the value of object is already zero-initialized, any new ctors for
-	 subobjects will also be zero-initialized.  */
-      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
-
       enum tree_code code = TREE_CODE (type);
       tree reftype = refs->pop();
       tree index = refs->pop();
 
+      if (code == COMPLEX_TYPE)
+	{
+	  if (TREE_CODE (*valp) == COMPLEX_CST)
+	    *valp = build2 (COMPLEX_EXPR, type, TREE_REALPART (*valp),
+			    TREE_IMAGPART (*valp));
+	  else if (TREE_CODE (*valp) == CONSTRUCTOR
+		   && CONSTRUCTOR_NELTS (*valp) == 0
+		   && CONSTRUCTOR_NO_CLEARING (*valp))
+	    {
+	      tree r = build_constructor (reftype, NULL);
+	      CONSTRUCTOR_NO_CLEARING (r) = 1;
+	      *valp = build2 (COMPLEX_EXPR, type, r, r);
+	    }
+	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
+	  complex_expr = valp;
+	  valp = &TREE_OPERAND (*valp, index != integer_zero_node);
+	  gcc_checking_assert (refs->is_empty ());
+	  type = reftype;
+	  complex_part = index != integer_zero_node;
+	  break;
+	}
+
+      /* If the value of object is already zero-initialized, any new ctors for
+	 subobjects will also be zero-initialized.  */
+      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
+
       if (code == RECORD_TYPE && is_empty_field (index))
 	/* Don't build a sub-CONSTRUCTOR for an empty base or field, as they
 	   have no data and might have an offset lower than previously declared
@@ -5946,6 +5984,24 @@ cxx_eval_store_expression (const constex
 	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
 	  valp = &cep->value;
 	}
+      if (complex_part != -1)
+	{
+	  if (TREE_CODE (*valp) == COMPLEX_CST)
+	    *valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp),
+			    TREE_REALPART (*valp),
+			    TREE_IMAGPART (*valp));
+	  else if (TREE_CODE (*valp) == CONSTRUCTOR
+		   && CONSTRUCTOR_NELTS (*valp) == 0
+		   && CONSTRUCTOR_NO_CLEARING (*valp))
+	    {
+	      tree r = build_constructor (TREE_TYPE (TREE_TYPE (*valp)), NULL);
+	      CONSTRUCTOR_NO_CLEARING (r) = 1;
+	      *valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp), r, r);
+	    }
+	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
+	  complex_expr = valp;
+	  valp = &TREE_OPERAND (*valp, complex_part);
+	}
     }
 
   if (*non_constant_p)
@@ -6016,6 +6072,22 @@ cxx_eval_store_expression (const constex
 	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
 	  CONSTRUCTOR_NO_CLEARING (elt) = false;
       }
+  if (complex_expr)
+    {
+      if (tree c = const_binop (COMPLEX_EXPR, TREE_TYPE (*complex_expr),
+				TREE_OPERAND (*complex_expr, 0),
+				TREE_OPERAND (*complex_expr, 1)))
+	*complex_expr = c;
+      else
+	{
+	  TREE_CONSTANT (*complex_expr)
+	    = (TREE_CONSTANT (TREE_OPERAND (*complex_expr, 0))
+	       & TREE_CONSTANT (TREE_OPERAND (*complex_expr, 1)));
+	  TREE_SIDE_EFFECTS (*complex_expr)
+	    = (TREE_SIDE_EFFECTS (TREE_OPERAND (*complex_expr, 0))
+	       | TREE_SIDE_EFFECTS (TREE_OPERAND (*complex_expr, 1)));
+	}
+    }
 
   if (lval)
     return target;
--- gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C.jj	2022-06-08 17:32:39.190148964 +0200
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C	2022-06-08 17:29:04.413321741 +0200
@@ -0,0 +1,24 @@
+// PR c++/88174
+// { dg-do compile { target c++14 } }
+
+constexpr bool
+foo (double x, double y, double z, double w)
+{
+  __complex__ double a = 0;
+  __real__ a = x;
+  __imag__ a = y;
+#if __cpp_constexpr >= 201907L
+  __complex__ double b;
+  __real__ b = z;
+#else
+  __complex__ double b = z;
+#endif
+  __imag__ b = w;
+  a += b;
+  a -= b;
+  a *= b;
+  a /= b;
+  return __real__ a == x && __imag__ a == y;
+}
+
+static_assert (foo (1.0, 2.0, 3.0, 4.0), "");

	Jakub


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] c++: Add support for __real__/__imag__ modifications in constant expressions [PR88174]
  2022-06-09  8:37 [PATCH] c++: Add support for __real__/__imag__ modifications in constant expressions [PR88174] Jakub Jelinek
@ 2022-06-10 17:27 ` Jason Merrill
  2022-06-10 19:57   ` Jakub Jelinek
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2022-06-10 17:27 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 6/9/22 04:37, Jakub Jelinek wrote:
> Hi!
> 
> We claim we support P0415R1 (constexpr complex), but e.g.
> #include <complex>
> 
> constexpr bool
> foo ()
> {
>    std::complex<double> a (1.0, 2.0);
>    a += 3.0;
>    a.real (6.0);
>    return a.real () == 6.0 && a.imag () == 2.0;
> }
> 
> static_assert (foo ());
> 
> fails with
> test.C:12:20: error: non-constant condition for static assertion
>     12 | static_assert (foo ());
>        |                ~~~~^~
> test.C:12:20:   in ‘constexpr’ expansion of ‘foo()’
> test.C:8:10:   in ‘constexpr’ expansion of ‘a.std::complex<double>::real(6.0e+0)’
> test.C:12:20: error: modification of ‘__real__ a.std::complex<double>::_M_value’ is not a constant expression
> 
> The problem is we don't handle REALPART_EXPR and IMAGPART_EXPR
> in cxx_eval_store_expression.
> The following patch attempts to support it (with a requirement
> that those are the outermost expressions, ARRAY_REF/COMPONENT_REF
> etc. are just not possible on the result of these, BIT_FIELD_REF
> would be theoretically possible if trying to extract some bits
> from one part of a complex int, but I don't see how it could appear
> in the FE trees.
> 
> For these references, the code handles value being COMPLEX_CST,
> COMPLEX_EXPR or CONSTRUCTOR_NO_CLEARING empty CONSTRUCTOR (what we use
> to represent uninitialized values for C++20 and later) and the
> code starts by rewriting it to COMPLEX_EXPR, so that we can freely
> adjust the individual parts and later on possibly optimize it back
> to COMPLEX_CST if both halves are constant.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2022-06-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/88174
> 	* constexpr.cc (cxx_eval_store_expression): Handle REALPART_EXPR
> 	and IMAGPART_EXPR.
> 
> 	* g++.dg/cpp1y/constexpr-complex1.C: New test.
> 
> --- gcc/cp/constexpr.cc.jj	2022-06-08 08:21:02.973448193 +0200
> +++ gcc/cp/constexpr.cc	2022-06-08 17:13:04.986040449 +0200
> @@ -5707,6 +5707,20 @@ cxx_eval_store_expression (const constex
>   	  }
>   	  break;
>   
> +	case REALPART_EXPR:
> +	  gcc_assert (probe == target);

Doesn't this assert mean that complex_expr will always be == valp?

> +	  vec_safe_push (refs, integer_zero_node);
> +	  vec_safe_push (refs, TREE_TYPE (probe));
> +	  probe = TREE_OPERAND (probe, 0);
> +	  break;
> +
> +	case IMAGPART_EXPR:
> +	  gcc_assert (probe == target);
> +	  vec_safe_push (refs, integer_one_node);
> +	  vec_safe_push (refs, TREE_TYPE (probe));
> +	  probe = TREE_OPERAND (probe, 0);
> +	  break;
> +
>   	default:
>   	  if (evaluated)
>   	    object = probe;
> @@ -5749,6 +5763,8 @@ cxx_eval_store_expression (const constex
>     auto_vec<int> index_pos_hints;
>     bool activated_union_member_p = false;
>     bool empty_base = false;
> +  int complex_part = -1;
> +  tree *complex_expr = NULL;
>     while (!refs->is_empty ())
>       {
>         if (*valp == NULL_TREE)
> @@ -5785,14 +5801,36 @@ cxx_eval_store_expression (const constex
>   	  *valp = ary_ctor;
>   	}
>   
> -      /* If the value of object is already zero-initialized, any new ctors for
> -	 subobjects will also be zero-initialized.  */
> -      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> -
>         enum tree_code code = TREE_CODE (type);
>         tree reftype = refs->pop();
>         tree index = refs->pop();
>   
> +      if (code == COMPLEX_TYPE)
> +	{
> +	  if (TREE_CODE (*valp) == COMPLEX_CST)
> +	    *valp = build2 (COMPLEX_EXPR, type, TREE_REALPART (*valp),
> +			    TREE_IMAGPART (*valp));
> +	  else if (TREE_CODE (*valp) == CONSTRUCTOR
> +		   && CONSTRUCTOR_NELTS (*valp) == 0
> +		   && CONSTRUCTOR_NO_CLEARING (*valp))
> +	    {
> +	      tree r = build_constructor (reftype, NULL);
> +	      CONSTRUCTOR_NO_CLEARING (r) = 1;
> +	      *valp = build2 (COMPLEX_EXPR, type, r, r);
> +	    }
> +	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> +	  complex_expr = valp;
> +	  valp = &TREE_OPERAND (*valp, index != integer_zero_node);
> +	  gcc_checking_assert (refs->is_empty ());
> +	  type = reftype;
> +	  complex_part = index != integer_zero_node;
> +	  break;
> +	}
> +
> +      /* If the value of object is already zero-initialized, any new ctors for
> +	 subobjects will also be zero-initialized.  */
> +      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> +
>         if (code == RECORD_TYPE && is_empty_field (index))
>   	/* Don't build a sub-CONSTRUCTOR for an empty base or field, as they
>   	   have no data and might have an offset lower than previously declared
> @@ -5946,6 +5984,24 @@ cxx_eval_store_expression (const constex
>   	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
>   	  valp = &cep->value;
>   	}
> +      if (complex_part != -1)
> +	{
> +	  if (TREE_CODE (*valp) == COMPLEX_CST)
> +	    *valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp),
> +			    TREE_REALPART (*valp),
> +			    TREE_IMAGPART (*valp));
> +	  else if (TREE_CODE (*valp) == CONSTRUCTOR
> +		   && CONSTRUCTOR_NELTS (*valp) == 0
> +		   && CONSTRUCTOR_NO_CLEARING (*valp))
> +	    {
> +	      tree r = build_constructor (TREE_TYPE (TREE_TYPE (*valp)), NULL);
> +	      CONSTRUCTOR_NO_CLEARING (r) = 1;
> +	      *valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp), r, r);
> +	    }
> +	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> +	  complex_expr = valp;
> +	  valp = &TREE_OPERAND (*valp, complex_part);

I don't understand this block; shouldn't valp point to the real or imag 
part of the complex number at this point?  How could complex_part be set 
without us handling the complex case in the loop already?

> +	}
>       }
>   
>     if (*non_constant_p)
> @@ -6016,6 +6072,22 @@ cxx_eval_store_expression (const constex
>   	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
>   	  CONSTRUCTOR_NO_CLEARING (elt) = false;
>         }
> +  if (complex_expr)

I might have added the COMPLEX_EXPR to ctors instead of a separate 
variable, but this is fine too.

> +    {
> +      if (tree c = const_binop (COMPLEX_EXPR, TREE_TYPE (*complex_expr),
> +				TREE_OPERAND (*complex_expr, 0),
> +				TREE_OPERAND (*complex_expr, 1)))
> +	*complex_expr = c;
> +      else
> +	{
> +	  TREE_CONSTANT (*complex_expr)
> +	    = (TREE_CONSTANT (TREE_OPERAND (*complex_expr, 0))
> +	       & TREE_CONSTANT (TREE_OPERAND (*complex_expr, 1)));
> +	  TREE_SIDE_EFFECTS (*complex_expr)
> +	    = (TREE_SIDE_EFFECTS (TREE_OPERAND (*complex_expr, 0))
> +	       | TREE_SIDE_EFFECTS (TREE_OPERAND (*complex_expr, 1)));
> +	}
> +    }
>   
>     if (lval)
>       return target;
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C.jj	2022-06-08 17:32:39.190148964 +0200
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C	2022-06-08 17:29:04.413321741 +0200
> @@ -0,0 +1,24 @@
> +// PR c++/88174
> +// { dg-do compile { target c++14 } }
> +
> +constexpr bool
> +foo (double x, double y, double z, double w)
> +{
> +  __complex__ double a = 0;
> +  __real__ a = x;
> +  __imag__ a = y;
> +#if __cpp_constexpr >= 201907L
> +  __complex__ double b;
> +  __real__ b = z;
> +#else
> +  __complex__ double b = z;
> +#endif
> +  __imag__ b = w;
> +  a += b;
> +  a -= b;
> +  a *= b;
> +  a /= b;
> +  return __real__ a == x && __imag__ a == y;
> +}
> +
> +static_assert (foo (1.0, 2.0, 3.0, 4.0), "");
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] c++: Add support for __real__/__imag__ modifications in constant expressions [PR88174]
  2022-06-10 17:27 ` Jason Merrill
@ 2022-06-10 19:57   ` Jakub Jelinek
  2022-06-17 17:06     ` [PATCH] c++, v2: " Jakub Jelinek
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2022-06-10 19:57 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Fri, Jun 10, 2022 at 01:27:28PM -0400, Jason Merrill wrote:
> > --- gcc/cp/constexpr.cc.jj	2022-06-08 08:21:02.973448193 +0200
> > +++ gcc/cp/constexpr.cc	2022-06-08 17:13:04.986040449 +0200
> > @@ -5707,6 +5707,20 @@ cxx_eval_store_expression (const constex
> >   	  }
> >   	  break;
> > +	case REALPART_EXPR:
> > +	  gcc_assert (probe == target);
> 
> Doesn't this assert mean that complex_expr will always be == valp?

No, even when handling the pushed *PART_EXPR, it will set
valp = &TREE_OPERAND (*valp, index != integer_zero_node);
So, valp will be either &TREE_OPERAND (*complex_expr, 0)
or &TREE_OPERAND (*complex_expr, 1).
As *valp = init; is what is usually then stored and we want to store there
the scalar.

> > @@ -5946,6 +5984,24 @@ cxx_eval_store_expression (const constex
> >   	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
> >   	  valp = &cep->value;
> >   	}
> > +      if (complex_part != -1)
> > +	{
> > +	  if (TREE_CODE (*valp) == COMPLEX_CST)
> > +	    *valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp),
> > +			    TREE_REALPART (*valp),
> > +			    TREE_IMAGPART (*valp));
> > +	  else if (TREE_CODE (*valp) == CONSTRUCTOR
> > +		   && CONSTRUCTOR_NELTS (*valp) == 0
> > +		   && CONSTRUCTOR_NO_CLEARING (*valp))
> > +	    {
> > +	      tree r = build_constructor (TREE_TYPE (TREE_TYPE (*valp)), NULL);
> > +	      CONSTRUCTOR_NO_CLEARING (r) = 1;
> > +	      *valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp), r, r);
> > +	    }
> > +	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> > +	  complex_expr = valp;
> > +	  valp = &TREE_OPERAND (*valp, complex_part);
> 
> I don't understand this block; shouldn't valp point to the real or imag part
> of the complex number at this point?  How could complex_part be set without
> us handling the complex case in the loop already?

Because for most references, the code will do:
      vec_safe_push (ctors, *valp);
      vec_safe_push (indexes, index);
I chose not to do this for *PART_EXPR, because the COMPLEX_EXPR isn't a
CONSTRUCTOR and code later on e.g. walks all the ctors and accesses
CONSTRUCTOR_NO_CLEARING on them etc.  As the *PART_EXPR is asserted to
be outermost only, complex_expr is a variant of that ctors push and
complex_part of the indexes.
The reason for the above if is just in case the evaluation of the rhs
of the store would store to the complex and could e.g. make it a COMPLEX_CST
again.

> > +	}
> >       }
> >     if (*non_constant_p)
> > @@ -6016,6 +6072,22 @@ cxx_eval_store_expression (const constex
> >   	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
> >   	  CONSTRUCTOR_NO_CLEARING (elt) = false;
> >         }
> > +  if (complex_expr)
> 
> I might have added the COMPLEX_EXPR to ctors instead of a separate variable,
> but this is fine too.

See above.
The COMPLEX_EXPR needs special handling (conversion into COMPLEX_CST if it
is constant) anyway.

	Jakub


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] c++, v2: Add support for __real__/__imag__ modifications in constant expressions [PR88174]
  2022-06-10 19:57   ` Jakub Jelinek
@ 2022-06-17 17:06     ` Jakub Jelinek
  2022-06-20 20:03       ` Jason Merrill
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2022-06-17 17:06 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Fri, Jun 10, 2022 at 09:57:06PM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Fri, Jun 10, 2022 at 01:27:28PM -0400, Jason Merrill wrote:
> > Doesn't this assert mean that complex_expr will always be == valp?
> 
> No, even when handling the pushed *PART_EXPR, it will set
> valp = &TREE_OPERAND (*valp, index != integer_zero_node);
> So, valp will be either &TREE_OPERAND (*complex_expr, 0)
> or &TREE_OPERAND (*complex_expr, 1).
> As *valp = init; is what is usually then stored and we want to store there
> the scalar.
> 
> > I don't understand this block; shouldn't valp point to the real or imag part
> > of the complex number at this point?  How could complex_part be set without
> > us handling the complex case in the loop already?
> 
> Because for most references, the code will do:
>       vec_safe_push (ctors, *valp);
>       vec_safe_push (indexes, index);
> I chose not to do this for *PART_EXPR, because the COMPLEX_EXPR isn't a
> CONSTRUCTOR and code later on e.g. walks all the ctors and accesses
> CONSTRUCTOR_NO_CLEARING on them etc.  As the *PART_EXPR is asserted to
> be outermost only, complex_expr is a variant of that ctors push and
> complex_part of the indexes.
> The reason for the above if is just in case the evaluation of the rhs
> of the store would store to the complex and could e.g. make it a COMPLEX_CST
> again.
> 
> > I might have added the COMPLEX_EXPR to ctors instead of a separate variable,
> > but this is fine too.
> 
> See above.
> The COMPLEX_EXPR needs special handling (conversion into COMPLEX_CST if it
> is constant) anyway.

Here is a variant patch which pushes even the *PART_EXPR related entries
into ctors and indexes vectors, so it doesn't need to use extra variables
for the complex stuff.

2022-06-17  Jakub Jelinek  <jakub@redhat.com>

	PR c++/88174
	* constexpr.cc (cxx_eval_store_expression): Handle REALPART_EXPR
	and IMAGPART_EXPR.  Change ctors from releasing_vec to
	auto_vec<tree *>, adjust all uses.

	* g++.dg/cpp1y/constexpr-complex1.C: New test.

--- gcc/cp/constexpr.cc.jj	2022-06-09 17:42:23.606243920 +0200
+++ gcc/cp/constexpr.cc	2022-06-17 18:59:54.809208997 +0200
@@ -5714,6 +5714,20 @@ cxx_eval_store_expression (const constex
 	  }
 	  break;
 
+	case REALPART_EXPR:
+	  gcc_assert (probe == target);
+	  vec_safe_push (refs, probe);
+	  vec_safe_push (refs, TREE_TYPE (probe));
+	  probe = TREE_OPERAND (probe, 0);
+	  break;
+
+	case IMAGPART_EXPR:
+	  gcc_assert (probe == target);
+	  vec_safe_push (refs, probe);
+	  vec_safe_push (refs, TREE_TYPE (probe));
+	  probe = TREE_OPERAND (probe, 0);
+	  break;
+
 	default:
 	  if (evaluated)
 	    object = probe;
@@ -5752,7 +5766,8 @@ cxx_eval_store_expression (const constex
   type = TREE_TYPE (object);
   bool no_zero_init = true;
 
-  releasing_vec ctors, indexes;
+  auto_vec<tree *> ctors;
+  releasing_vec indexes;
   auto_vec<int> index_pos_hints;
   bool activated_union_member_p = false;
   bool empty_base = false;
@@ -5792,14 +5807,36 @@ cxx_eval_store_expression (const constex
 	  *valp = ary_ctor;
 	}
 
-      /* If the value of object is already zero-initialized, any new ctors for
-	 subobjects will also be zero-initialized.  */
-      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
-
       enum tree_code code = TREE_CODE (type);
       tree reftype = refs->pop();
       tree index = refs->pop();
 
+      if (code == COMPLEX_TYPE)
+	{
+	  if (TREE_CODE (*valp) == COMPLEX_CST)
+	    *valp = build2 (COMPLEX_EXPR, type, TREE_REALPART (*valp),
+			    TREE_IMAGPART (*valp));
+	  else if (TREE_CODE (*valp) == CONSTRUCTOR
+		   && CONSTRUCTOR_NELTS (*valp) == 0
+		   && CONSTRUCTOR_NO_CLEARING (*valp))
+	    {
+	      tree r = build_constructor (reftype, NULL);
+	      CONSTRUCTOR_NO_CLEARING (r) = 1;
+	      *valp = build2 (COMPLEX_EXPR, type, r, r);
+	    }
+	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
+	  ctors.safe_push (valp);
+	  vec_safe_push (indexes, index);
+	  valp = &TREE_OPERAND (*valp, TREE_CODE (index) == IMAGPART_EXPR);
+	  gcc_checking_assert (refs->is_empty ());
+	  type = reftype;
+	  break;
+	}
+
+      /* If the value of object is already zero-initialized, any new ctors for
+	 subobjects will also be zero-initialized.  */
+      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
+
       if (code == RECORD_TYPE && is_empty_field (index))
 	/* Don't build a sub-CONSTRUCTOR for an empty base or field, as they
 	   have no data and might have an offset lower than previously declared
@@ -5842,7 +5879,7 @@ cxx_eval_store_expression (const constex
 	  no_zero_init = true;
 	}
 
-      vec_safe_push (ctors, *valp);
+      ctors.safe_push (valp);
       vec_safe_push (indexes, index);
 
       constructor_elt *cep
@@ -5904,11 +5941,11 @@ cxx_eval_store_expression (const constex
 	     semantics are not applied on an object under construction.
 	     They come into effect when the constructor for the most
 	     derived object ends."  */
-	  for (tree elt : *ctors)
+	  for (tree *elt : ctors)
 	    if (same_type_ignoring_top_level_qualifiers_p
-		(TREE_TYPE (const_object_being_modified), TREE_TYPE (elt)))
+		(TREE_TYPE (const_object_being_modified), TREE_TYPE (*elt)))
 	      {
-		fail = TREE_READONLY (elt);
+		fail = TREE_READONLY (*elt);
 		break;
 	      }
 	}
@@ -5949,6 +5986,28 @@ cxx_eval_store_expression (const constex
       valp = ctx->global->values.get (object);
       for (unsigned i = 0; i < vec_safe_length (indexes); i++)
 	{
+	  ctors[i] = valp;
+	  if (TREE_CODE (indexes[i]) == REALPART_EXPR
+	      || TREE_CODE (indexes[i]) == IMAGPART_EXPR)
+	    {
+	      if (TREE_CODE (*valp) == COMPLEX_CST)
+		*valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp),
+				TREE_REALPART (*valp),
+				TREE_IMAGPART (*valp));
+	      else if (TREE_CODE (*valp) == CONSTRUCTOR
+		       && CONSTRUCTOR_NELTS (*valp) == 0
+		       && CONSTRUCTOR_NO_CLEARING (*valp))
+		{
+		  tree r = build_constructor (TREE_TYPE (TREE_TYPE (*valp)),
+					      NULL);
+		  CONSTRUCTOR_NO_CLEARING (r) = 1;
+		  *valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp), r, r);
+		}
+	      gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
+	      valp = &TREE_OPERAND (*valp,
+				    TREE_CODE (indexes[i]) == IMAGPART_EXPR);
+	      break;
+	    }
 	  constructor_elt *cep
 	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
 	  valp = &cep->value;
@@ -6012,17 +6071,41 @@ cxx_eval_store_expression (const constex
   bool c = TREE_CONSTANT (init);
   bool s = TREE_SIDE_EFFECTS (init);
   if (!c || s || activated_union_member_p)
-    for (tree elt : *ctors)
+    for (tree *elt : ctors)
       {
+	if (TREE_CODE (*elt) != CONSTRUCTOR)
+	  continue;
 	if (!c)
-	  TREE_CONSTANT (elt) = false;
+	  TREE_CONSTANT (*elt) = false;
 	if (s)
-	  TREE_SIDE_EFFECTS (elt) = true;
+	  TREE_SIDE_EFFECTS (*elt) = true;
 	/* Clear CONSTRUCTOR_NO_CLEARING since we've activated a member of
 	   this union.  */
-	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
-	  CONSTRUCTOR_NO_CLEARING (elt) = false;
+	if (TREE_CODE (TREE_TYPE (*elt)) == UNION_TYPE)
+	  CONSTRUCTOR_NO_CLEARING (*elt) = false;
       }
+  if (!indexes->is_empty ())
+    {
+      tree last = indexes->last ();
+      if (TREE_CODE (last) == REALPART_EXPR
+	  || TREE_CODE (last) == IMAGPART_EXPR)
+	{
+	  tree *cexpr = ctors.last ();
+	  if (tree c = const_binop (COMPLEX_EXPR, TREE_TYPE (*cexpr),
+				    TREE_OPERAND (*cexpr, 0),
+				    TREE_OPERAND (*cexpr, 1)))
+	    *cexpr = c;
+	  else
+	    {
+	      TREE_CONSTANT (*cexpr)
+		= (TREE_CONSTANT (TREE_OPERAND (*cexpr, 0))
+		   & TREE_CONSTANT (TREE_OPERAND (*cexpr, 1)));
+	      TREE_SIDE_EFFECTS (*cexpr)
+		= (TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 0))
+		   | TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 1)));
+	    }
+	}
+    }
 
   if (lval)
     return target;
--- gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C.jj	2022-06-17 17:41:45.885780190 +0200
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C	2022-06-17 17:41:45.885780190 +0200
@@ -0,0 +1,24 @@
+// PR c++/88174
+// { dg-do compile { target c++14 } }
+
+constexpr bool
+foo (double x, double y, double z, double w)
+{
+  __complex__ double a = 0;
+  __real__ a = x;
+  __imag__ a = y;
+#if __cpp_constexpr >= 201907L
+  __complex__ double b;
+  __real__ b = z;
+#else
+  __complex__ double b = z;
+#endif
+  __imag__ b = w;
+  a += b;
+  a -= b;
+  a *= b;
+  a /= b;
+  return __real__ a == x && __imag__ a == y;
+}
+
+static_assert (foo (1.0, 2.0, 3.0, 4.0), "");


	Jakub


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] c++, v2: Add support for __real__/__imag__ modifications in constant expressions [PR88174]
  2022-06-17 17:06     ` [PATCH] c++, v2: " Jakub Jelinek
@ 2022-06-20 20:03       ` Jason Merrill
  2022-06-27 16:31         ` Jakub Jelinek
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2022-06-20 20:03 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 6/17/22 13:06, Jakub Jelinek wrote:
> On Fri, Jun 10, 2022 at 09:57:06PM +0200, Jakub Jelinek via Gcc-patches wrote:
>> On Fri, Jun 10, 2022 at 01:27:28PM -0400, Jason Merrill wrote:
>>> Doesn't this assert mean that complex_expr will always be == valp?
>>
>> No, even when handling the pushed *PART_EXPR, it will set
>> valp = &TREE_OPERAND (*valp, index != integer_zero_node);
>> So, valp will be either &TREE_OPERAND (*complex_expr, 0)
>> or &TREE_OPERAND (*complex_expr, 1).
>> As *valp = init; is what is usually then stored and we want to store there
>> the scalar.
>>
>>> I don't understand this block; shouldn't valp point to the real or imag part
>>> of the complex number at this point?  How could complex_part be set without
>>> us handling the complex case in the loop already?
>>
>> Because for most references, the code will do:
>>        vec_safe_push (ctors, *valp);
>>        vec_safe_push (indexes, index);
>> I chose not to do this for *PART_EXPR, because the COMPLEX_EXPR isn't a
>> CONSTRUCTOR and code later on e.g. walks all the ctors and accesses
>> CONSTRUCTOR_NO_CLEARING on them etc.  As the *PART_EXPR is asserted to
>> be outermost only, complex_expr is a variant of that ctors push and
>> complex_part of the indexes.
>> The reason for the above if is just in case the evaluation of the rhs
>> of the store would store to the complex and could e.g. make it a COMPLEX_CST
>> again.
>>
>>> I might have added the COMPLEX_EXPR to ctors instead of a separate variable,
>>> but this is fine too.
>>
>> See above.
>> The COMPLEX_EXPR needs special handling (conversion into COMPLEX_CST if it
>> is constant) anyway.
> 
> Here is a variant patch which pushes even the *PART_EXPR related entries
> into ctors and indexes vectors, so it doesn't need to use extra variables
> for the complex stuff.

Thanks.

> 2022-06-17  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/88174
> 	* constexpr.cc (cxx_eval_store_expression): Handle REALPART_EXPR
> 	and IMAGPART_EXPR.  Change ctors from releasing_vec to
> 	auto_vec<tree *>, adjust all uses.
> 
> 	* g++.dg/cpp1y/constexpr-complex1.C: New test.
> 
> --- gcc/cp/constexpr.cc.jj	2022-06-09 17:42:23.606243920 +0200
> +++ gcc/cp/constexpr.cc	2022-06-17 18:59:54.809208997 +0200
> @@ -5714,6 +5714,20 @@ cxx_eval_store_expression (const constex
>   	  }
>   	  break;
>   
> +	case REALPART_EXPR:
> +	  gcc_assert (probe == target);
> +	  vec_safe_push (refs, probe);
> +	  vec_safe_push (refs, TREE_TYPE (probe));
> +	  probe = TREE_OPERAND (probe, 0);
> +	  break;
> +
> +	case IMAGPART_EXPR:
> +	  gcc_assert (probe == target);
> +	  vec_safe_push (refs, probe);
> +	  vec_safe_push (refs, TREE_TYPE (probe));
> +	  probe = TREE_OPERAND (probe, 0);
> +	  break;
> +
>   	default:
>   	  if (evaluated)
>   	    object = probe;
> @@ -5752,7 +5766,8 @@ cxx_eval_store_expression (const constex
>     type = TREE_TYPE (object);
>     bool no_zero_init = true;
>   
> -  releasing_vec ctors, indexes;
> +  auto_vec<tree *> ctors;
> +  releasing_vec indexes;
>     auto_vec<int> index_pos_hints;
>     bool activated_union_member_p = false;
>     bool empty_base = false;
> @@ -5792,14 +5807,36 @@ cxx_eval_store_expression (const constex
>   	  *valp = ary_ctor;
>   	}
>   
> -      /* If the value of object is already zero-initialized, any new ctors for
> -	 subobjects will also be zero-initialized.  */
> -      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> -
>         enum tree_code code = TREE_CODE (type);
>         tree reftype = refs->pop();
>         tree index = refs->pop();
>   
> +      if (code == COMPLEX_TYPE)
> +	{
> +	  if (TREE_CODE (*valp) == COMPLEX_CST)
> +	    *valp = build2 (COMPLEX_EXPR, type, TREE_REALPART (*valp),
> +			    TREE_IMAGPART (*valp));
> +	  else if (TREE_CODE (*valp) == CONSTRUCTOR
> +		   && CONSTRUCTOR_NELTS (*valp) == 0
> +		   && CONSTRUCTOR_NO_CLEARING (*valp))
> +	    {
> +	      tree r = build_constructor (reftype, NULL);
> +	      CONSTRUCTOR_NO_CLEARING (r) = 1;
> +	      *valp = build2 (COMPLEX_EXPR, type, r, r);
> +	    }
> +	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> +	  ctors.safe_push (valp);
> +	  vec_safe_push (indexes, index);
> +	  valp = &TREE_OPERAND (*valp, TREE_CODE (index) == IMAGPART_EXPR);
> +	  gcc_checking_assert (refs->is_empty ());
> +	  type = reftype;
> +	  break;
> +	}
> +
> +      /* If the value of object is already zero-initialized, any new ctors for
> +	 subobjects will also be zero-initialized.  */
> +      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> +
>         if (code == RECORD_TYPE && is_empty_field (index))
>   	/* Don't build a sub-CONSTRUCTOR for an empty base or field, as they
>   	   have no data and might have an offset lower than previously declared
> @@ -5842,7 +5879,7 @@ cxx_eval_store_expression (const constex
>   	  no_zero_init = true;
>   	}
>   
> -      vec_safe_push (ctors, *valp);
> +      ctors.safe_push (valp);
>         vec_safe_push (indexes, index);
>   
>         constructor_elt *cep
> @@ -5904,11 +5941,11 @@ cxx_eval_store_expression (const constex
>   	     semantics are not applied on an object under construction.
>   	     They come into effect when the constructor for the most
>   	     derived object ends."  */
> -	  for (tree elt : *ctors)
> +	  for (tree *elt : ctors)
>   	    if (same_type_ignoring_top_level_qualifiers_p
> -		(TREE_TYPE (const_object_being_modified), TREE_TYPE (elt)))
> +		(TREE_TYPE (const_object_being_modified), TREE_TYPE (*elt)))
>   	      {
> -		fail = TREE_READONLY (elt);
> +		fail = TREE_READONLY (*elt);
>   		break;
>   	      }
>   	}
> @@ -5949,6 +5986,28 @@ cxx_eval_store_expression (const constex
>         valp = ctx->global->values.get (object);
>         for (unsigned i = 0; i < vec_safe_length (indexes); i++)
>   	{
> +	  ctors[i] = valp;
> +	  if (TREE_CODE (indexes[i]) == REALPART_EXPR
> +	      || TREE_CODE (indexes[i]) == IMAGPART_EXPR)
> +	    {
> +	      if (TREE_CODE (*valp) == COMPLEX_CST)
> +		*valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp),
> +				TREE_REALPART (*valp),
> +				TREE_IMAGPART (*valp));
> +	      else if (TREE_CODE (*valp) == CONSTRUCTOR
> +		       && CONSTRUCTOR_NELTS (*valp) == 0
> +		       && CONSTRUCTOR_NO_CLEARING (*valp))
> +		{
> +		  tree r = build_constructor (TREE_TYPE (TREE_TYPE (*valp)),
> +					      NULL);
> +		  CONSTRUCTOR_NO_CLEARING (r) = 1;
> +		  *valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp), r, r);
> +		}
> +	      gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> +	      valp = &TREE_OPERAND (*valp,
> +				    TREE_CODE (indexes[i]) == IMAGPART_EXPR);
> +	      break;
> +	    }

Hmm, why do we need to handle complex in the !preeval case?  I'd think 
we want to preevaluate all complex values or components thereof.

>   	  constructor_elt *cep
>   	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
>   	  valp = &cep->value;
> @@ -6012,17 +6071,41 @@ cxx_eval_store_expression (const constex
>     bool c = TREE_CONSTANT (init);
>     bool s = TREE_SIDE_EFFECTS (init);
>     if (!c || s || activated_union_member_p)
> -    for (tree elt : *ctors)
> +    for (tree *elt : ctors)
>         {
> +	if (TREE_CODE (*elt) != CONSTRUCTOR)
> +	  continue;
>   	if (!c)
> -	  TREE_CONSTANT (elt) = false;
> +	  TREE_CONSTANT (*elt) = false;
>   	if (s)
> -	  TREE_SIDE_EFFECTS (elt) = true;
> +	  TREE_SIDE_EFFECTS (*elt) = true;
>   	/* Clear CONSTRUCTOR_NO_CLEARING since we've activated a member of
>   	   this union.  */
> -	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
> -	  CONSTRUCTOR_NO_CLEARING (elt) = false;
> +	if (TREE_CODE (TREE_TYPE (*elt)) == UNION_TYPE)
> +	  CONSTRUCTOR_NO_CLEARING (*elt) = false;
>         }
> +  if (!indexes->is_empty ())
> +    {
> +      tree last = indexes->last ();
> +      if (TREE_CODE (last) == REALPART_EXPR
> +	  || TREE_CODE (last) == IMAGPART_EXPR)
> +	{
> +	  tree *cexpr = ctors.last ();
> +	  if (tree c = const_binop (COMPLEX_EXPR, TREE_TYPE (*cexpr),
> +				    TREE_OPERAND (*cexpr, 0),
> +				    TREE_OPERAND (*cexpr, 1)))
> +	    *cexpr = c;
> +	  else
> +	    {
> +	      TREE_CONSTANT (*cexpr)
> +		= (TREE_CONSTANT (TREE_OPERAND (*cexpr, 0))
> +		   & TREE_CONSTANT (TREE_OPERAND (*cexpr, 1)));
> +	      TREE_SIDE_EFFECTS (*cexpr)
> +		= (TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 0))
> +		   | TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 1)));

This seems like it needs to come before the ctors loop, so that these 
flags can be propagated out to enclosing constructors.

> +	    }
> +	}
> +    }
>   
>     if (lval)
>       return target;
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C.jj	2022-06-17 17:41:45.885780190 +0200
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C	2022-06-17 17:41:45.885780190 +0200
> @@ -0,0 +1,24 @@
> +// PR c++/88174
> +// { dg-do compile { target c++14 } }
> +
> +constexpr bool
> +foo (double x, double y, double z, double w)
> +{
> +  __complex__ double a = 0;
> +  __real__ a = x;
> +  __imag__ a = y;
> +#if __cpp_constexpr >= 201907L
> +  __complex__ double b;
> +  __real__ b = z;
> +#else
> +  __complex__ double b = z;
> +#endif
> +  __imag__ b = w;
> +  a += b;
> +  a -= b;
> +  a *= b;
> +  a /= b;
> +  return __real__ a == x && __imag__ a == y;
> +}
> +
> +static_assert (foo (1.0, 2.0, 3.0, 4.0), "");
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] c++, v2: Add support for __real__/__imag__ modifications in constant expressions [PR88174]
  2022-06-20 20:03       ` Jason Merrill
@ 2022-06-27 16:31         ` Jakub Jelinek
  2022-07-04 15:50           ` [PATCH] c++, v3: " Jakub Jelinek
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2022-06-27 16:31 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Mon, Jun 20, 2022 at 04:03:50PM -0400, Jason Merrill wrote:
> > +      if (code == COMPLEX_TYPE)
> > +	{
> > +	  if (TREE_CODE (*valp) == COMPLEX_CST)
> > +	    *valp = build2 (COMPLEX_EXPR, type, TREE_REALPART (*valp),
> > +			    TREE_IMAGPART (*valp));
> > +	  else if (TREE_CODE (*valp) == CONSTRUCTOR
> > +		   && CONSTRUCTOR_NELTS (*valp) == 0
> > +		   && CONSTRUCTOR_NO_CLEARING (*valp))
> > +	    {
> > +	      tree r = build_constructor (reftype, NULL);
> > +	      CONSTRUCTOR_NO_CLEARING (r) = 1;
> > +	      *valp = build2 (COMPLEX_EXPR, type, r, r);
> > +	    }
> > +	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> > +	  ctors.safe_push (valp);
> > +	  vec_safe_push (indexes, index);
> > +	  valp = &TREE_OPERAND (*valp, TREE_CODE (index) == IMAGPART_EXPR);
> > +	  gcc_checking_assert (refs->is_empty ());
> > +	  type = reftype;
> > +	  break;
> > +	}
...
> > @@ -5949,6 +5986,28 @@ cxx_eval_store_expression (const constex
> >         valp = ctx->global->values.get (object);
> >         for (unsigned i = 0; i < vec_safe_length (indexes); i++)
> >   	{
> > +	  ctors[i] = valp;
> > +	  if (TREE_CODE (indexes[i]) == REALPART_EXPR
> > +	      || TREE_CODE (indexes[i]) == IMAGPART_EXPR)
> > +	    {
> > +	      if (TREE_CODE (*valp) == COMPLEX_CST)
> > +		*valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp),
> > +				TREE_REALPART (*valp),
> > +				TREE_IMAGPART (*valp));
> > +	      else if (TREE_CODE (*valp) == CONSTRUCTOR
> > +		       && CONSTRUCTOR_NELTS (*valp) == 0
> > +		       && CONSTRUCTOR_NO_CLEARING (*valp))
> > +		{
> > +		  tree r = build_constructor (TREE_TYPE (TREE_TYPE (*valp)),
> > +					      NULL);
> > +		  CONSTRUCTOR_NO_CLEARING (r) = 1;
> > +		  *valp = build2 (COMPLEX_EXPR, TREE_TYPE (*valp), r, r);
> > +		}
> > +	      gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> > +	      valp = &TREE_OPERAND (*valp,
> > +				    TREE_CODE (indexes[i]) == IMAGPART_EXPR);
> > +	      break;
> > +	    }
> 
> Hmm, why do we need to handle complex in the !preeval case?  I'd think we
> want to preevaluate all complex values or components thereof.

Because the late evaluation of the initializer could have touched
the destination, so we need to reevaluate it.
Same reason why we call get_or_insert_ctor_field again in the second
loop as we call it in the first loop.
If it would help, I could move that repeated part into:
tree
canonicalize_complex_to_complex_expr (tree t)
{
  if (TREE_CODE (t) == COMPLEX_CST)
   t = build2 (COMPLEX_EXPR, TREE_TYPE (t),
	       TREE_REALPART (t), TREE_IMAGPART (t));
  else if (TREE_CODE (t) == CONSTRUCTOR
	   && CONSTRUCTOR_NELTS (t) == 0
	   && CONSTRUCTOR_NO_CLEARING (t))
    {
      tree r = build_constructor (TREE_TYPE (TREE_TYPE (t)), NULL);
      CONSTRUCTOR_NO_CLEARING (r) = 1;
      t = build2 (COMPLEX_EXPR, TREE_TYPE (t), r, r);
    }
  return t;
}
and use that to shorten the code.

> 
> >   	  constructor_elt *cep
> >   	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
> >   	  valp = &cep->value;
> > @@ -6012,17 +6071,41 @@ cxx_eval_store_expression (const constex
> >     bool c = TREE_CONSTANT (init);
> >     bool s = TREE_SIDE_EFFECTS (init);
> >     if (!c || s || activated_union_member_p)
> > -    for (tree elt : *ctors)
> > +    for (tree *elt : ctors)
> >         {
> > +	if (TREE_CODE (*elt) != CONSTRUCTOR)
> > +	  continue;
> >   	if (!c)
> > -	  TREE_CONSTANT (elt) = false;
> > +	  TREE_CONSTANT (*elt) = false;
> >   	if (s)
> > -	  TREE_SIDE_EFFECTS (elt) = true;
> > +	  TREE_SIDE_EFFECTS (*elt) = true;
> >   	/* Clear CONSTRUCTOR_NO_CLEARING since we've activated a member of
> >   	   this union.  */
> > -	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
> > -	  CONSTRUCTOR_NO_CLEARING (elt) = false;
> > +	if (TREE_CODE (TREE_TYPE (*elt)) == UNION_TYPE)
> > +	  CONSTRUCTOR_NO_CLEARING (*elt) = false;
> >         }
> > +  if (!indexes->is_empty ())
> > +    {
> > +      tree last = indexes->last ();
> > +      if (TREE_CODE (last) == REALPART_EXPR
> > +	  || TREE_CODE (last) == IMAGPART_EXPR)
> > +	{
> > +	  tree *cexpr = ctors.last ();
> > +	  if (tree c = const_binop (COMPLEX_EXPR, TREE_TYPE (*cexpr),
> > +				    TREE_OPERAND (*cexpr, 0),
> > +				    TREE_OPERAND (*cexpr, 1)))
> > +	    *cexpr = c;
> > +	  else
> > +	    {
> > +	      TREE_CONSTANT (*cexpr)
> > +		= (TREE_CONSTANT (TREE_OPERAND (*cexpr, 0))
> > +		   & TREE_CONSTANT (TREE_OPERAND (*cexpr, 1)));
> > +	      TREE_SIDE_EFFECTS (*cexpr)
> > +		= (TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 0))
> > +		   | TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 1)));
> 
> This seems like it needs to come before the ctors loop, so that these flags
> can be propagated out to enclosing constructors.

I could indeed move this in between
  bool c = TREE_CONSTANT (init);
  bool s = TREE_SIDE_EFFECTS (init);
and
  if (!c || s || activated_union_member_p)
and update c and s from *cexpr flags.

	Jakub


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] c++, v3: Add support for __real__/__imag__ modifications in constant expressions [PR88174]
  2022-06-27 16:31         ` Jakub Jelinek
@ 2022-07-04 15:50           ` Jakub Jelinek
  2022-07-05 20:44             ` Jason Merrill
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2022-07-04 15:50 UTC (permalink / raw)
  To: Jason Merrill, gcc-patches

On Mon, Jun 27, 2022 at 06:31:18PM +0200, Jakub Jelinek via Gcc-patches wrote:
> Because the late evaluation of the initializer could have touched
> the destination, so we need to reevaluate it.
> Same reason why we call get_or_insert_ctor_field again in the second
> loop as we call it in the first loop.
> If it would help, I could move that repeated part into:

> > This seems like it needs to come before the ctors loop, so that these flags
> > can be propagated out to enclosing constructors.
> 
> I could indeed move this in between
>   bool c = TREE_CONSTANT (init);
>   bool s = TREE_SIDE_EFFECTS (init);
> and
>   if (!c || s || activated_union_member_p)
> and update c and s from *cexpr flags.

Here is it in patch form, so far lightly tested, ok for trunk if it passes
full bootstrap/regtest?

2022-07-04  Jakub Jelinek  <jakub@redhat.com>

	PR c++/88174
	* constexpr.cc (canonicalize_complex_to_complex_expr): New function.
	(cxx_eval_store_expression): Handle REALPART_EXPR and IMAGPART_EXPR.
	Change ctors from releasing_vec to auto_vec<tree *>, adjust all uses.

	* g++.dg/cpp1y/constexpr-complex1.C: New test.

--- gcc/cp/constexpr.cc.jj	2022-07-04 12:26:18.147053851 +0200
+++ gcc/cp/constexpr.cc	2022-07-04 17:35:53.100556949 +0200
@@ -5640,6 +5640,26 @@ modifying_const_object_p (tree_code code
   return false;
 }
 
+/* Helper of cxx_eval_store_expression, turn a COMPLEX_CST or
+   empty no clearing CONSTRUCTOR into a COMPLEX_EXPR.  */
+
+static tree
+canonicalize_complex_to_complex_expr (tree t)
+{
+  if (TREE_CODE (t) == COMPLEX_CST)
+    t = build2 (COMPLEX_EXPR, TREE_TYPE (t),
+		TREE_REALPART (t), TREE_IMAGPART (t));
+  else if (TREE_CODE (t) == CONSTRUCTOR
+	   && CONSTRUCTOR_NELTS (t) == 0
+	   && CONSTRUCTOR_NO_CLEARING (t))
+    {
+      tree r = build_constructor (TREE_TYPE (TREE_TYPE (t)), NULL);
+      CONSTRUCTOR_NO_CLEARING (r) = 1;
+      t = build2 (COMPLEX_EXPR, TREE_TYPE (t), r, r);
+    }
+  return t;
+}
+
 /* Evaluate an INIT_EXPR or MODIFY_EXPR.  */
 
 static tree
@@ -5726,6 +5746,20 @@ cxx_eval_store_expression (const constex
 	  }
 	  break;
 
+	case REALPART_EXPR:
+	  gcc_assert (probe == target);
+	  vec_safe_push (refs, probe);
+	  vec_safe_push (refs, TREE_TYPE (probe));
+	  probe = TREE_OPERAND (probe, 0);
+	  break;
+
+	case IMAGPART_EXPR:
+	  gcc_assert (probe == target);
+	  vec_safe_push (refs, probe);
+	  vec_safe_push (refs, TREE_TYPE (probe));
+	  probe = TREE_OPERAND (probe, 0);
+	  break;
+
 	default:
 	  if (evaluated)
 	    object = probe;
@@ -5764,7 +5798,8 @@ cxx_eval_store_expression (const constex
   type = TREE_TYPE (object);
   bool no_zero_init = true;
 
-  releasing_vec ctors, indexes;
+  auto_vec<tree *> ctors;
+  releasing_vec indexes;
   auto_vec<int> index_pos_hints;
   bool activated_union_member_p = false;
   bool empty_base = false;
@@ -5804,14 +5839,26 @@ cxx_eval_store_expression (const constex
 	  *valp = ary_ctor;
 	}
 
-      /* If the value of object is already zero-initialized, any new ctors for
-	 subobjects will also be zero-initialized.  */
-      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
-
       enum tree_code code = TREE_CODE (type);
       tree reftype = refs->pop();
       tree index = refs->pop();
 
+      if (code == COMPLEX_TYPE)
+	{
+	  *valp = canonicalize_complex_to_complex_expr (*valp);
+	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
+	  ctors.safe_push (valp);
+	  vec_safe_push (indexes, index);
+	  valp = &TREE_OPERAND (*valp, TREE_CODE (index) == IMAGPART_EXPR);
+	  gcc_checking_assert (refs->is_empty ());
+	  type = reftype;
+	  break;
+	}
+
+      /* If the value of object is already zero-initialized, any new ctors for
+	 subobjects will also be zero-initialized.  */
+      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
+
       if (code == RECORD_TYPE && is_empty_field (index))
 	/* Don't build a sub-CONSTRUCTOR for an empty base or field, as they
 	   have no data and might have an offset lower than previously declared
@@ -5854,7 +5901,7 @@ cxx_eval_store_expression (const constex
 	  no_zero_init = true;
 	}
 
-      vec_safe_push (ctors, *valp);
+      ctors.safe_push (valp);
       vec_safe_push (indexes, index);
 
       constructor_elt *cep
@@ -5916,11 +5963,11 @@ cxx_eval_store_expression (const constex
 	     semantics are not applied on an object under construction.
 	     They come into effect when the constructor for the most
 	     derived object ends."  */
-	  for (tree elt : *ctors)
+	  for (tree *elt : ctors)
 	    if (same_type_ignoring_top_level_qualifiers_p
-		(TREE_TYPE (const_object_being_modified), TREE_TYPE (elt)))
+		(TREE_TYPE (const_object_being_modified), TREE_TYPE (*elt)))
 	      {
-		fail = TREE_READONLY (elt);
+		fail = TREE_READONLY (*elt);
 		break;
 	      }
 	}
@@ -5961,6 +6008,16 @@ cxx_eval_store_expression (const constex
       valp = ctx->global->values.get (object);
       for (unsigned i = 0; i < vec_safe_length (indexes); i++)
 	{
+	  ctors[i] = valp;
+	  if (TREE_CODE (indexes[i]) == REALPART_EXPR
+	      || TREE_CODE (indexes[i]) == IMAGPART_EXPR)
+	    {
+	      *valp = canonicalize_complex_to_complex_expr (*valp);
+	      gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
+	      valp = &TREE_OPERAND (*valp,
+				    TREE_CODE (indexes[i]) == IMAGPART_EXPR);
+	      break;
+	    }
 	  constructor_elt *cep
 	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
 	  valp = &cep->value;
@@ -6023,17 +6080,45 @@ cxx_eval_store_expression (const constex
      CONSTRUCTORs, if any.  */
   bool c = TREE_CONSTANT (init);
   bool s = TREE_SIDE_EFFECTS (init);
+  if (!indexes->is_empty ())
+    {
+      tree last = indexes->last ();
+      if (TREE_CODE (last) == REALPART_EXPR
+	  || TREE_CODE (last) == IMAGPART_EXPR)
+	{
+	  /* And canonicalize COMPLEX_EXPR into COMPLEX_CST if
+	     possible.  */
+	  tree *cexpr = ctors.last ();
+	  if (tree c = const_binop (COMPLEX_EXPR, TREE_TYPE (*cexpr),
+				    TREE_OPERAND (*cexpr, 0),
+				    TREE_OPERAND (*cexpr, 1)))
+	    *cexpr = c;
+	  else
+	    {
+	      TREE_CONSTANT (*cexpr)
+		= (TREE_CONSTANT (TREE_OPERAND (*cexpr, 0))
+		   & TREE_CONSTANT (TREE_OPERAND (*cexpr, 1)));
+	      TREE_SIDE_EFFECTS (*cexpr)
+		= (TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 0))
+		   | TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 1)));
+	    }
+	  c = TREE_CONSTANT (*cexpr);
+	  s = TREE_SIDE_EFFECTS (*cexpr);
+	}
+    }
   if (!c || s || activated_union_member_p)
-    for (tree elt : *ctors)
+    for (tree *elt : ctors)
       {
+	if (TREE_CODE (*elt) != CONSTRUCTOR)
+	  continue;
 	if (!c)
-	  TREE_CONSTANT (elt) = false;
+	  TREE_CONSTANT (*elt) = false;
 	if (s)
-	  TREE_SIDE_EFFECTS (elt) = true;
+	  TREE_SIDE_EFFECTS (*elt) = true;
 	/* Clear CONSTRUCTOR_NO_CLEARING since we've activated a member of
 	   this union.  */
-	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
-	  CONSTRUCTOR_NO_CLEARING (elt) = false;
+	if (TREE_CODE (TREE_TYPE (*elt)) == UNION_TYPE)
+	  CONSTRUCTOR_NO_CLEARING (*elt) = false;
       }
 
   if (lval)
--- gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C.jj	2022-07-04 17:30:47.884580998 +0200
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C	2022-07-04 17:30:47.884580998 +0200
@@ -0,0 +1,24 @@
+// PR c++/88174
+// { dg-do compile { target c++14 } }
+
+constexpr bool
+foo (double x, double y, double z, double w)
+{
+  __complex__ double a = 0;
+  __real__ a = x;
+  __imag__ a = y;
+#if __cpp_constexpr >= 201907L
+  __complex__ double b;
+  __real__ b = z;
+#else
+  __complex__ double b = z;
+#endif
+  __imag__ b = w;
+  a += b;
+  a -= b;
+  a *= b;
+  a /= b;
+  return __real__ a == x && __imag__ a == y;
+}
+
+static_assert (foo (1.0, 2.0, 3.0, 4.0), "");


	Jakub


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] c++, v3: Add support for __real__/__imag__ modifications in constant expressions [PR88174]
  2022-07-04 15:50           ` [PATCH] c++, v3: " Jakub Jelinek
@ 2022-07-05 20:44             ` Jason Merrill
  2022-07-05 20:57               ` Jakub Jelinek
  2022-07-27  9:09               ` [PATCH] c++, v4: " Jakub Jelinek
  0 siblings, 2 replies; 11+ messages in thread
From: Jason Merrill @ 2022-07-05 20:44 UTC (permalink / raw)
  To: Jakub Jelinek, gcc-patches

On 7/4/22 11:50, Jakub Jelinek wrote:
> On Mon, Jun 27, 2022 at 06:31:18PM +0200, Jakub Jelinek via Gcc-patches wrote:

>>> Hmm, why do we need to handle complex in the !preeval case?  I'd think we
>>> want to preevaluate all complex values or components thereof.

>> Because the late evaluation of the initializer could have touched
>> the destination, so we need to reevaluate it.
>> Same reason why we call get_or_insert_ctor_field again in the second
>> loop as we call it in the first loop.

But preeval should always be true, so we'd never reach the new handling 
in the if (!preeval) block.  Certainly the new testcase doesn't exercise 
this code.

>> If it would help, I could move that repeated part into:
> 
>>> This seems like it needs to come before the ctors loop, so that these flags
>>> can be propagated out to enclosing constructors.
>>
>> I could indeed move this in between
>>    bool c = TREE_CONSTANT (init);
>>    bool s = TREE_SIDE_EFFECTS (init);
>> and
>>    if (!c || s || activated_union_member_p)
>> and update c and s from *cexpr flags.
> 
> Here is it in patch form, so far lightly tested, ok for trunk if it passes
> full bootstrap/regtest?
> 
> 2022-07-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/88174
> 	* constexpr.cc (canonicalize_complex_to_complex_expr): New function.
> 	(cxx_eval_store_expression): Handle REALPART_EXPR and IMAGPART_EXPR.
> 	Change ctors from releasing_vec to auto_vec<tree *>, adjust all uses.
> 
> 	* g++.dg/cpp1y/constexpr-complex1.C: New test.
> 
> --- gcc/cp/constexpr.cc.jj	2022-07-04 12:26:18.147053851 +0200
> +++ gcc/cp/constexpr.cc	2022-07-04 17:35:53.100556949 +0200
> @@ -5640,6 +5640,26 @@ modifying_const_object_p (tree_code code
>     return false;
>   }
>   
> +/* Helper of cxx_eval_store_expression, turn a COMPLEX_CST or
> +   empty no clearing CONSTRUCTOR into a COMPLEX_EXPR.  */
> +
> +static tree
> +canonicalize_complex_to_complex_expr (tree t)
> +{
> +  if (TREE_CODE (t) == COMPLEX_CST)
> +    t = build2 (COMPLEX_EXPR, TREE_TYPE (t),
> +		TREE_REALPART (t), TREE_IMAGPART (t));
> +  else if (TREE_CODE (t) == CONSTRUCTOR
> +	   && CONSTRUCTOR_NELTS (t) == 0
> +	   && CONSTRUCTOR_NO_CLEARING (t))
> +    {
> +      tree r = build_constructor (TREE_TYPE (TREE_TYPE (t)), NULL);
> +      CONSTRUCTOR_NO_CLEARING (r) = 1;
> +      t = build2 (COMPLEX_EXPR, TREE_TYPE (t), r, r);
> +    }
> +  return t;
> +}
> +
>   /* Evaluate an INIT_EXPR or MODIFY_EXPR.  */
>   
>   static tree
> @@ -5726,6 +5746,20 @@ cxx_eval_store_expression (const constex
>   	  }
>   	  break;
>   
> +	case REALPART_EXPR:
> +	  gcc_assert (probe == target);
> +	  vec_safe_push (refs, probe);
> +	  vec_safe_push (refs, TREE_TYPE (probe));
> +	  probe = TREE_OPERAND (probe, 0);
> +	  break;
> +
> +	case IMAGPART_EXPR:
> +	  gcc_assert (probe == target);
> +	  vec_safe_push (refs, probe);
> +	  vec_safe_push (refs, TREE_TYPE (probe));
> +	  probe = TREE_OPERAND (probe, 0);
> +	  break;
> +
>   	default:
>   	  if (evaluated)
>   	    object = probe;
> @@ -5764,7 +5798,8 @@ cxx_eval_store_expression (const constex
>     type = TREE_TYPE (object);
>     bool no_zero_init = true;
>   
> -  releasing_vec ctors, indexes;
> +  auto_vec<tree *> ctors;
> +  releasing_vec indexes;
>     auto_vec<int> index_pos_hints;
>     bool activated_union_member_p = false;
>     bool empty_base = false;
> @@ -5804,14 +5839,26 @@ cxx_eval_store_expression (const constex
>   	  *valp = ary_ctor;
>   	}
>   
> -      /* If the value of object is already zero-initialized, any new ctors for
> -	 subobjects will also be zero-initialized.  */
> -      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> -
>         enum tree_code code = TREE_CODE (type);
>         tree reftype = refs->pop();
>         tree index = refs->pop();
>   
> +      if (code == COMPLEX_TYPE)
> +	{
> +	  *valp = canonicalize_complex_to_complex_expr (*valp);
> +	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> +	  ctors.safe_push (valp);
> +	  vec_safe_push (indexes, index);
> +	  valp = &TREE_OPERAND (*valp, TREE_CODE (index) == IMAGPART_EXPR);
> +	  gcc_checking_assert (refs->is_empty ());
> +	  type = reftype;
> +	  break;
> +	}
> +
> +      /* If the value of object is already zero-initialized, any new ctors for
> +	 subobjects will also be zero-initialized.  */
> +      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> +
>         if (code == RECORD_TYPE && is_empty_field (index))
>   	/* Don't build a sub-CONSTRUCTOR for an empty base or field, as they
>   	   have no data and might have an offset lower than previously declared
> @@ -5854,7 +5901,7 @@ cxx_eval_store_expression (const constex
>   	  no_zero_init = true;
>   	}
>   
> -      vec_safe_push (ctors, *valp);
> +      ctors.safe_push (valp);
>         vec_safe_push (indexes, index);
>   
>         constructor_elt *cep
> @@ -5916,11 +5963,11 @@ cxx_eval_store_expression (const constex
>   	     semantics are not applied on an object under construction.
>   	     They come into effect when the constructor for the most
>   	     derived object ends."  */
> -	  for (tree elt : *ctors)
> +	  for (tree *elt : ctors)
>   	    if (same_type_ignoring_top_level_qualifiers_p
> -		(TREE_TYPE (const_object_being_modified), TREE_TYPE (elt)))
> +		(TREE_TYPE (const_object_being_modified), TREE_TYPE (*elt)))
>   	      {
> -		fail = TREE_READONLY (elt);
> +		fail = TREE_READONLY (*elt);
>   		break;
>   	      }
>   	}
> @@ -5961,6 +6008,16 @@ cxx_eval_store_expression (const constex
>         valp = ctx->global->values.get (object);
>         for (unsigned i = 0; i < vec_safe_length (indexes); i++)
>   	{
> +	  ctors[i] = valp;
> +	  if (TREE_CODE (indexes[i]) == REALPART_EXPR
> +	      || TREE_CODE (indexes[i]) == IMAGPART_EXPR)
> +	    {
> +	      *valp = canonicalize_complex_to_complex_expr (*valp);
> +	      gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> +	      valp = &TREE_OPERAND (*valp,
> +				    TREE_CODE (indexes[i]) == IMAGPART_EXPR);
> +	      break;
> +	    }
>   	  constructor_elt *cep
>   	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
>   	  valp = &cep->value;
> @@ -6023,17 +6080,45 @@ cxx_eval_store_expression (const constex
>        CONSTRUCTORs, if any.  */
>     bool c = TREE_CONSTANT (init);
>     bool s = TREE_SIDE_EFFECTS (init);
> +  if (!indexes->is_empty ())
> +    {
> +      tree last = indexes->last ();
> +      if (TREE_CODE (last) == REALPART_EXPR
> +	  || TREE_CODE (last) == IMAGPART_EXPR)
> +	{
> +	  /* And canonicalize COMPLEX_EXPR into COMPLEX_CST if
> +	     possible.  */
> +	  tree *cexpr = ctors.last ();
> +	  if (tree c = const_binop (COMPLEX_EXPR, TREE_TYPE (*cexpr),
> +				    TREE_OPERAND (*cexpr, 0),
> +				    TREE_OPERAND (*cexpr, 1)))
> +	    *cexpr = c;
> +	  else
> +	    {
> +	      TREE_CONSTANT (*cexpr)
> +		= (TREE_CONSTANT (TREE_OPERAND (*cexpr, 0))
> +		   & TREE_CONSTANT (TREE_OPERAND (*cexpr, 1)));
> +	      TREE_SIDE_EFFECTS (*cexpr)
> +		= (TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 0))
> +		   | TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 1)));
> +	    }
> +	  c = TREE_CONSTANT (*cexpr);
> +	  s = TREE_SIDE_EFFECTS (*cexpr);
> +	}
> +    }
>     if (!c || s || activated_union_member_p)
> -    for (tree elt : *ctors)
> +    for (tree *elt : ctors)
>         {
> +	if (TREE_CODE (*elt) != CONSTRUCTOR)
> +	  continue;
>   	if (!c)
> -	  TREE_CONSTANT (elt) = false;
> +	  TREE_CONSTANT (*elt) = false;
>   	if (s)
> -	  TREE_SIDE_EFFECTS (elt) = true;
> +	  TREE_SIDE_EFFECTS (*elt) = true;
>   	/* Clear CONSTRUCTOR_NO_CLEARING since we've activated a member of
>   	   this union.  */
> -	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
> -	  CONSTRUCTOR_NO_CLEARING (elt) = false;
> +	if (TREE_CODE (TREE_TYPE (*elt)) == UNION_TYPE)
> +	  CONSTRUCTOR_NO_CLEARING (*elt) = false;
>         }
>   
>     if (lval)
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C.jj	2022-07-04 17:30:47.884580998 +0200
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C	2022-07-04 17:30:47.884580998 +0200
> @@ -0,0 +1,24 @@
> +// PR c++/88174
> +// { dg-do compile { target c++14 } }
> +
> +constexpr bool
> +foo (double x, double y, double z, double w)
> +{
> +  __complex__ double a = 0;
> +  __real__ a = x;
> +  __imag__ a = y;
> +#if __cpp_constexpr >= 201907L
> +  __complex__ double b;
> +  __real__ b = z;
> +#else
> +  __complex__ double b = z;
> +#endif
> +  __imag__ b = w;
> +  a += b;
> +  a -= b;
> +  a *= b;
> +  a /= b;
> +  return __real__ a == x && __imag__ a == y;
> +}
> +
> +static_assert (foo (1.0, 2.0, 3.0, 4.0), "");
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] c++, v3: Add support for __real__/__imag__ modifications in constant expressions [PR88174]
  2022-07-05 20:44             ` Jason Merrill
@ 2022-07-05 20:57               ` Jakub Jelinek
  2022-07-27  9:09               ` [PATCH] c++, v4: " Jakub Jelinek
  1 sibling, 0 replies; 11+ messages in thread
From: Jakub Jelinek @ 2022-07-05 20:57 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Tue, Jul 05, 2022 at 04:44:41PM -0400, Jason Merrill wrote:
> On 7/4/22 11:50, Jakub Jelinek wrote:
> > On Mon, Jun 27, 2022 at 06:31:18PM +0200, Jakub Jelinek via Gcc-patches wrote:
> 
> > > > Hmm, why do we need to handle complex in the !preeval case?  I'd think we
> > > > want to preevaluate all complex values or components thereof.
> 
> > > Because the late evaluation of the initializer could have touched
> > > the destination, so we need to reevaluate it.
> > > Same reason why we call get_or_insert_ctor_field again in the second
> > > loop as we call it in the first loop.
> 
> But preeval should always be true, so we'd never reach the new handling in
> the if (!preeval) block.  Certainly the new testcase doesn't exercise this
> code.

Ah, you're right, in the complex case SCALAR_TYPE_P (type) has to be true
because it is COMPLEX_TYPE and so preeval must be true.
I'll rework the patch then.

	Jakub


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] c++, v4: Add support for __real__/__imag__ modifications in constant expressions [PR88174]
  2022-07-05 20:44             ` Jason Merrill
  2022-07-05 20:57               ` Jakub Jelinek
@ 2022-07-27  9:09               ` Jakub Jelinek
  2022-08-06 22:41                 ` Jason Merrill
  1 sibling, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2022-07-27  9:09 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Tue, Jul 05, 2022 at 04:44:41PM -0400, Jason Merrill wrote:
> But preeval should always be true, so we'd never reach the new handling in
> the if (!preeval) block.  Certainly the new testcase doesn't exercise this
> code.

Ok, changed now.

I had to keep the ctors[i] = valp; statement in the !preeval block, because
otherwise e.g. 20_util/optional/monadic/transform.cc test fails randomly.
It was wrong already before the patch, it would adjust
TREE_CONSTANT/TREE_SIDE_EFFECTS sometimes on no longer used trees,
but with the vector holding pointers to trees rather than trees it is even
more severe.

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

2022-07-27  Jakub Jelinek  <jakub@redhat.com>

	PR c++/88174
	* constexpr.cc (cxx_eval_store_expression): Handle REALPART_EXPR
	and IMAGPART_EXPR.  Change ctors from releasing_vec to
	auto_vec<tree *>, adjust all uses.  For !preeval, update ctors
	vector.

	* g++.dg/cpp1y/constexpr-complex1.C: New test.

--- gcc/cp/constexpr.cc.jj	2022-07-04 12:26:18.147053851 +0200
+++ gcc/cp/constexpr.cc	2022-07-26 17:35:53.100556949 +0200
@@ -5726,6 +5726,20 @@ cxx_eval_store_expression (const constex
 	  }
 	  break;
 
+	case REALPART_EXPR:
+	  gcc_assert (probe == target);
+	  vec_safe_push (refs, probe);
+	  vec_safe_push (refs, TREE_TYPE (probe));
+	  probe = TREE_OPERAND (probe, 0);
+	  break;
+
+	case IMAGPART_EXPR:
+	  gcc_assert (probe == target);
+	  vec_safe_push (refs, probe);
+	  vec_safe_push (refs, TREE_TYPE (probe));
+	  probe = TREE_OPERAND (probe, 0);
+	  break;
+
 	default:
 	  if (evaluated)
 	    object = probe;
@@ -5764,7 +5778,8 @@ cxx_eval_store_expression (const constex
   type = TREE_TYPE (object);
   bool no_zero_init = true;
 
-  releasing_vec ctors, indexes;
+  auto_vec<tree *> ctors;
+  releasing_vec indexes;
   auto_vec<int> index_pos_hints;
   bool activated_union_member_p = false;
   bool empty_base = false;
@@ -5804,14 +5819,36 @@ cxx_eval_store_expression (const constex
 	  *valp = ary_ctor;
 	}
 
-      /* If the value of object is already zero-initialized, any new ctors for
-	 subobjects will also be zero-initialized.  */
-      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
-
       enum tree_code code = TREE_CODE (type);
       tree reftype = refs->pop();
       tree index = refs->pop();
 
+      if (code == COMPLEX_TYPE)
+	{
+	  if (TREE_CODE (*valp) == COMPLEX_CST)
+	    *valp = build2 (COMPLEX_EXPR, type, TREE_REALPART (*valp),
+			    TREE_IMAGPART (*valp));
+	  else if (TREE_CODE (*valp) == CONSTRUCTOR
+		   && CONSTRUCTOR_NELTS (*valp) == 0
+		   && CONSTRUCTOR_NO_CLEARING (*valp))
+	    {
+	      tree r = build_constructor (reftype, NULL);
+	      CONSTRUCTOR_NO_CLEARING (r) = 1;
+	      *valp = build2 (COMPLEX_EXPR, type, r, r);
+	    }
+	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
+	  ctors.safe_push (valp);
+	  vec_safe_push (indexes, index);
+	  valp = &TREE_OPERAND (*valp, TREE_CODE (index) == IMAGPART_EXPR);
+	  gcc_checking_assert (refs->is_empty ());
+	  type = reftype;
+	  break;
+	}
+
+      /* If the value of object is already zero-initialized, any new ctors for
+	 subobjects will also be zero-initialized.  */
+      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
+
       if (code == RECORD_TYPE && is_empty_field (index))
 	/* Don't build a sub-CONSTRUCTOR for an empty base or field, as they
 	   have no data and might have an offset lower than previously declared
@@ -5854,7 +5891,7 @@ cxx_eval_store_expression (const constex
 	  no_zero_init = true;
 	}
 
-      vec_safe_push (ctors, *valp);
+      ctors.safe_push (valp);
       vec_safe_push (indexes, index);
 
       constructor_elt *cep
@@ -5916,11 +5953,11 @@ cxx_eval_store_expression (const constex
 	     semantics are not applied on an object under construction.
 	     They come into effect when the constructor for the most
 	     derived object ends."  */
-	  for (tree elt : *ctors)
+	  for (tree *elt : ctors)
 	    if (same_type_ignoring_top_level_qualifiers_p
-		(TREE_TYPE (const_object_being_modified), TREE_TYPE (elt)))
+		(TREE_TYPE (const_object_being_modified), TREE_TYPE (*elt)))
 	      {
-		fail = TREE_READONLY (elt);
+		fail = TREE_READONLY (*elt);
 		break;
 	      }
 	}
@@ -5961,6 +5998,7 @@ cxx_eval_store_expression (const constex
       valp = ctx->global->values.get (object);
       for (unsigned i = 0; i < vec_safe_length (indexes); i++)
 	{
+	  ctors[i] = valp;
 	  constructor_elt *cep
 	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
 	  valp = &cep->value;
@@ -6023,17 +6061,45 @@ cxx_eval_store_expression (const constex
      CONSTRUCTORs, if any.  */
   bool c = TREE_CONSTANT (init);
   bool s = TREE_SIDE_EFFECTS (init);
+  if (!indexes->is_empty ())
+    {
+      tree last = indexes->last ();
+      if (TREE_CODE (last) == REALPART_EXPR
+	  || TREE_CODE (last) == IMAGPART_EXPR)
+	{
+	  /* And canonicalize COMPLEX_EXPR into COMPLEX_CST if
+	     possible.  */
+	  tree *cexpr = ctors.last ();
+	  if (tree c = const_binop (COMPLEX_EXPR, TREE_TYPE (*cexpr),
+				    TREE_OPERAND (*cexpr, 0),
+				    TREE_OPERAND (*cexpr, 1)))
+	    *cexpr = c;
+	  else
+	    {
+	      TREE_CONSTANT (*cexpr)
+		= (TREE_CONSTANT (TREE_OPERAND (*cexpr, 0))
+		   & TREE_CONSTANT (TREE_OPERAND (*cexpr, 1)));
+	      TREE_SIDE_EFFECTS (*cexpr)
+		= (TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 0))
+		   | TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 1)));
+	    }
+	  c = TREE_CONSTANT (*cexpr);
+	  s = TREE_SIDE_EFFECTS (*cexpr);
+	}
+    }
   if (!c || s || activated_union_member_p)
-    for (tree elt : *ctors)
+    for (tree *elt : ctors)
       {
+	if (TREE_CODE (*elt) != CONSTRUCTOR)
+	  continue;
 	if (!c)
-	  TREE_CONSTANT (elt) = false;
+	  TREE_CONSTANT (*elt) = false;
 	if (s)
-	  TREE_SIDE_EFFECTS (elt) = true;
+	  TREE_SIDE_EFFECTS (*elt) = true;
 	/* Clear CONSTRUCTOR_NO_CLEARING since we've activated a member of
 	   this union.  */
-	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
-	  CONSTRUCTOR_NO_CLEARING (elt) = false;
+	if (TREE_CODE (TREE_TYPE (*elt)) == UNION_TYPE)
+	  CONSTRUCTOR_NO_CLEARING (*elt) = false;
       }
 
   if (lval)
--- gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C.jj	2022-07-04 17:30:47.884580998 +0200
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C	2022-07-04 17:30:47.884580998 +0200
@@ -0,0 +1,24 @@
+// PR c++/88174
+// { dg-do compile { target c++14 } }
+
+constexpr bool
+foo (double x, double y, double z, double w)
+{
+  __complex__ double a = 0;
+  __real__ a = x;
+  __imag__ a = y;
+#if __cpp_constexpr >= 201907L
+  __complex__ double b;
+  __real__ b = z;
+#else
+  __complex__ double b = z;
+#endif
+  __imag__ b = w;
+  a += b;
+  a -= b;
+  a *= b;
+  a /= b;
+  return __real__ a == x && __imag__ a == y;
+}
+
+static_assert (foo (1.0, 2.0, 3.0, 4.0), "");


	Jakub


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] c++, v4: Add support for __real__/__imag__ modifications in constant expressions [PR88174]
  2022-07-27  9:09               ` [PATCH] c++, v4: " Jakub Jelinek
@ 2022-08-06 22:41                 ` Jason Merrill
  0 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2022-08-06 22:41 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 7/27/22 02:09, Jakub Jelinek wrote:
> On Tue, Jul 05, 2022 at 04:44:41PM -0400, Jason Merrill wrote:
>> But preeval should always be true, so we'd never reach the new handling in
>> the if (!preeval) block.  Certainly the new testcase doesn't exercise this
>> code.
> 
> Ok, changed now.
> 
> I had to keep the ctors[i] = valp; statement in the !preeval block, because
> otherwise e.g. 20_util/optional/monadic/transform.cc test fails randomly.
> It was wrong already before the patch, it would adjust
> TREE_CONSTANT/TREE_SIDE_EFFECTS sometimes on no longer used trees,
> but with the vector holding pointers to trees rather than trees it is even
> more severe.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK, thanks.

> 2022-07-27  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/88174
> 	* constexpr.cc (cxx_eval_store_expression): Handle REALPART_EXPR
> 	and IMAGPART_EXPR.  Change ctors from releasing_vec to
> 	auto_vec<tree *>, adjust all uses.  For !preeval, update ctors
> 	vector.
> 
> 	* g++.dg/cpp1y/constexpr-complex1.C: New test.
> 
> --- gcc/cp/constexpr.cc.jj	2022-07-04 12:26:18.147053851 +0200
> +++ gcc/cp/constexpr.cc	2022-07-26 17:35:53.100556949 +0200
> @@ -5726,6 +5726,20 @@ cxx_eval_store_expression (const constex
>   	  }
>   	  break;
>   
> +	case REALPART_EXPR:
> +	  gcc_assert (probe == target);
> +	  vec_safe_push (refs, probe);
> +	  vec_safe_push (refs, TREE_TYPE (probe));
> +	  probe = TREE_OPERAND (probe, 0);
> +	  break;
> +
> +	case IMAGPART_EXPR:
> +	  gcc_assert (probe == target);
> +	  vec_safe_push (refs, probe);
> +	  vec_safe_push (refs, TREE_TYPE (probe));
> +	  probe = TREE_OPERAND (probe, 0);
> +	  break;
> +
>   	default:
>   	  if (evaluated)
>   	    object = probe;
> @@ -5764,7 +5778,8 @@ cxx_eval_store_expression (const constex
>     type = TREE_TYPE (object);
>     bool no_zero_init = true;
>   
> -  releasing_vec ctors, indexes;
> +  auto_vec<tree *> ctors;
> +  releasing_vec indexes;
>     auto_vec<int> index_pos_hints;
>     bool activated_union_member_p = false;
>     bool empty_base = false;
> @@ -5804,14 +5819,36 @@ cxx_eval_store_expression (const constex
>   	  *valp = ary_ctor;
>   	}
>   
> -      /* If the value of object is already zero-initialized, any new ctors for
> -	 subobjects will also be zero-initialized.  */
> -      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> -
>         enum tree_code code = TREE_CODE (type);
>         tree reftype = refs->pop();
>         tree index = refs->pop();
>   
> +      if (code == COMPLEX_TYPE)
> +	{
> +	  if (TREE_CODE (*valp) == COMPLEX_CST)
> +	    *valp = build2 (COMPLEX_EXPR, type, TREE_REALPART (*valp),
> +			    TREE_IMAGPART (*valp));
> +	  else if (TREE_CODE (*valp) == CONSTRUCTOR
> +		   && CONSTRUCTOR_NELTS (*valp) == 0
> +		   && CONSTRUCTOR_NO_CLEARING (*valp))
> +	    {
> +	      tree r = build_constructor (reftype, NULL);
> +	      CONSTRUCTOR_NO_CLEARING (r) = 1;
> +	      *valp = build2 (COMPLEX_EXPR, type, r, r);
> +	    }
> +	  gcc_assert (TREE_CODE (*valp) == COMPLEX_EXPR);
> +	  ctors.safe_push (valp);
> +	  vec_safe_push (indexes, index);
> +	  valp = &TREE_OPERAND (*valp, TREE_CODE (index) == IMAGPART_EXPR);
> +	  gcc_checking_assert (refs->is_empty ());
> +	  type = reftype;
> +	  break;
> +	}
> +
> +      /* If the value of object is already zero-initialized, any new ctors for
> +	 subobjects will also be zero-initialized.  */
> +      no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
> +
>         if (code == RECORD_TYPE && is_empty_field (index))
>   	/* Don't build a sub-CONSTRUCTOR for an empty base or field, as they
>   	   have no data and might have an offset lower than previously declared
> @@ -5854,7 +5891,7 @@ cxx_eval_store_expression (const constex
>   	  no_zero_init = true;
>   	}
>   
> -      vec_safe_push (ctors, *valp);
> +      ctors.safe_push (valp);
>         vec_safe_push (indexes, index);
>   
>         constructor_elt *cep
> @@ -5916,11 +5953,11 @@ cxx_eval_store_expression (const constex
>   	     semantics are not applied on an object under construction.
>   	     They come into effect when the constructor for the most
>   	     derived object ends."  */
> -	  for (tree elt : *ctors)
> +	  for (tree *elt : ctors)
>   	    if (same_type_ignoring_top_level_qualifiers_p
> -		(TREE_TYPE (const_object_being_modified), TREE_TYPE (elt)))
> +		(TREE_TYPE (const_object_being_modified), TREE_TYPE (*elt)))
>   	      {
> -		fail = TREE_READONLY (elt);
> +		fail = TREE_READONLY (*elt);
>   		break;
>   	      }
>   	}
> @@ -5961,6 +5998,7 @@ cxx_eval_store_expression (const constex
>         valp = ctx->global->values.get (object);
>         for (unsigned i = 0; i < vec_safe_length (indexes); i++)
>   	{
> +	  ctors[i] = valp;
>   	  constructor_elt *cep
>   	    = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
>   	  valp = &cep->value;
> @@ -6023,17 +6061,45 @@ cxx_eval_store_expression (const constex
>        CONSTRUCTORs, if any.  */
>     bool c = TREE_CONSTANT (init);
>     bool s = TREE_SIDE_EFFECTS (init);
> +  if (!indexes->is_empty ())
> +    {
> +      tree last = indexes->last ();
> +      if (TREE_CODE (last) == REALPART_EXPR
> +	  || TREE_CODE (last) == IMAGPART_EXPR)
> +	{
> +	  /* And canonicalize COMPLEX_EXPR into COMPLEX_CST if
> +	     possible.  */
> +	  tree *cexpr = ctors.last ();
> +	  if (tree c = const_binop (COMPLEX_EXPR, TREE_TYPE (*cexpr),
> +				    TREE_OPERAND (*cexpr, 0),
> +				    TREE_OPERAND (*cexpr, 1)))
> +	    *cexpr = c;
> +	  else
> +	    {
> +	      TREE_CONSTANT (*cexpr)
> +		= (TREE_CONSTANT (TREE_OPERAND (*cexpr, 0))
> +		   & TREE_CONSTANT (TREE_OPERAND (*cexpr, 1)));
> +	      TREE_SIDE_EFFECTS (*cexpr)
> +		= (TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 0))
> +		   | TREE_SIDE_EFFECTS (TREE_OPERAND (*cexpr, 1)));
> +	    }
> +	  c = TREE_CONSTANT (*cexpr);
> +	  s = TREE_SIDE_EFFECTS (*cexpr);
> +	}
> +    }
>     if (!c || s || activated_union_member_p)
> -    for (tree elt : *ctors)
> +    for (tree *elt : ctors)
>         {
> +	if (TREE_CODE (*elt) != CONSTRUCTOR)
> +	  continue;
>   	if (!c)
> -	  TREE_CONSTANT (elt) = false;
> +	  TREE_CONSTANT (*elt) = false;
>   	if (s)
> -	  TREE_SIDE_EFFECTS (elt) = true;
> +	  TREE_SIDE_EFFECTS (*elt) = true;
>   	/* Clear CONSTRUCTOR_NO_CLEARING since we've activated a member of
>   	   this union.  */
> -	if (TREE_CODE (TREE_TYPE (elt)) == UNION_TYPE)
> -	  CONSTRUCTOR_NO_CLEARING (elt) = false;
> +	if (TREE_CODE (TREE_TYPE (*elt)) == UNION_TYPE)
> +	  CONSTRUCTOR_NO_CLEARING (*elt) = false;
>         }
>   
>     if (lval)
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C.jj	2022-07-04 17:30:47.884580998 +0200
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-complex1.C	2022-07-04 17:30:47.884580998 +0200
> @@ -0,0 +1,24 @@
> +// PR c++/88174
> +// { dg-do compile { target c++14 } }
> +
> +constexpr bool
> +foo (double x, double y, double z, double w)
> +{
> +  __complex__ double a = 0;
> +  __real__ a = x;
> +  __imag__ a = y;
> +#if __cpp_constexpr >= 201907L
> +  __complex__ double b;
> +  __real__ b = z;
> +#else
> +  __complex__ double b = z;
> +#endif
> +  __imag__ b = w;
> +  a += b;
> +  a -= b;
> +  a *= b;
> +  a /= b;
> +  return __real__ a == x && __imag__ a == y;
> +}
> +
> +static_assert (foo (1.0, 2.0, 3.0, 4.0), "");
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-08-06 22:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-09  8:37 [PATCH] c++: Add support for __real__/__imag__ modifications in constant expressions [PR88174] Jakub Jelinek
2022-06-10 17:27 ` Jason Merrill
2022-06-10 19:57   ` Jakub Jelinek
2022-06-17 17:06     ` [PATCH] c++, v2: " Jakub Jelinek
2022-06-20 20:03       ` Jason Merrill
2022-06-27 16:31         ` Jakub Jelinek
2022-07-04 15:50           ` [PATCH] c++, v3: " Jakub Jelinek
2022-07-05 20:44             ` Jason Merrill
2022-07-05 20:57               ` Jakub Jelinek
2022-07-27  9:09               ` [PATCH] c++, v4: " Jakub Jelinek
2022-08-06 22:41                 ` Jason Merrill

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).