Re: [C++ PATCH] Don't clear TREE_CONSTANT on ADDR_EXPRs (PR c++/83993)

[Jason Merrill]

On Wed, Jan 31, 2018 at 11:21 AM, Jakub Jelinek  wrote:
> On Wed, Jan 31, 2018 at 11:05:17AM -0500, Jason Merrill wrote:
>> > 2018-01-24  Jakub Jelinek  
>> >
>> > PR c++/83993
>> > * constexpr.c (cxx_eval_outermost_constant_expr): Don't clear
>> > TREE_CONSTANT on ADDR_EXPRs.
>> >
>> > * g++.dg/init/pr83993-2.C: New test.
>> >
>> > --- gcc/cp/constexpr.c.jj   2018-01-24 13:38:40.572913190 +0100
>> > +++ gcc/cp/constexpr.c  2018-01-24 17:03:16.821440047 +0100
>> > @@ -4832,7 +4832,7 @@ cxx_eval_outermost_constant_expr (tree t
>> >
>> >if (non_constant_p && !allow_non_constant)
>> >  return error_mark_node;
>> > -  else if (non_constant_p && TREE_CONSTANT (r))
>> > +  else if (non_constant_p && TREE_CONSTANT (r) && TREE_CODE (r) != 
>> > ADDR_EXPR)
>> >  {
>> >/* This isn't actually constant, so unset TREE_CONSTANT.  */
>> >if (EXPR_P (r))
>>
>> Hmm, what if we check for ADDR_EXPR in the EXPR_P test, so we build a
>> non-TREE_CONSTANT NOP_EXPR around the ADDR_EXPR instead?
>
> That looks like a good idea and works on the testcase in question,
> ok for trunk if it passes bootstrap/regtest?
>
> 2018-01-31  Jason Merrill  
> Jakub Jelinek  
>
> PR c++/83993
> * constexpr.c (cxx_eval_outermost_constant_expr): Build NOP_EXPR
> around non-constant ADDR_EXPRs rather than clearing TREE_CONSTANT
> on ADDR_EXPR.
>
> * g++.dg/init/pr83993-2.C: New test.
>
> --- gcc/cp/constexpr.c.jj   2018-01-30 12:30:24.279362188 +0100
> +++ gcc/cp/constexpr.c  2018-01-31 17:18:03.316302653 +0100
> @@ -4824,7 +4824,7 @@ cxx_eval_outermost_constant_expr (tree t
>else if (non_constant_p && TREE_CONSTANT (r))
>  {
>/* This isn't actually constant, so unset TREE_CONSTANT.  */
> -  if (EXPR_P (r))
> +  if (EXPR_P (r) && TREE_CODE (r) != ADDR_EXPR)

OK with an added comment about the middle end needing TREE_CONSTANT to
stay set on ADDR_EXPR.

Jason

Re: [C++ PATCH] Don't clear TREE_CONSTANT on ADDR_EXPRs (PR c++/83993)

[Jakub Jelinek]

On Wed, Jan 31, 2018 at 11:05:17AM -0500, Jason Merrill wrote:
> > 2018-01-24  Jakub Jelinek  
> >
> > PR c++/83993
> > * constexpr.c (cxx_eval_outermost_constant_expr): Don't clear
> > TREE_CONSTANT on ADDR_EXPRs.
> >
> > * g++.dg/init/pr83993-2.C: New test.
> >
> > --- gcc/cp/constexpr.c.jj   2018-01-24 13:38:40.572913190 +0100
> > +++ gcc/cp/constexpr.c  2018-01-24 17:03:16.821440047 +0100
> > @@ -4832,7 +4832,7 @@ cxx_eval_outermost_constant_expr (tree t
> >
> >if (non_constant_p && !allow_non_constant)
> >  return error_mark_node;
> > -  else if (non_constant_p && TREE_CONSTANT (r))
> > +  else if (non_constant_p && TREE_CONSTANT (r) && TREE_CODE (r) != 
> > ADDR_EXPR)
> >  {
> >/* This isn't actually constant, so unset TREE_CONSTANT.  */
> >if (EXPR_P (r))
> 
> Hmm, what if we check for ADDR_EXPR in the EXPR_P test, so we build a
> non-TREE_CONSTANT NOP_EXPR around the ADDR_EXPR instead?

That looks like a good idea and works on the testcase in question,
ok for trunk if it passes bootstrap/regtest?

2018-01-31  Jason Merrill  
Jakub Jelinek  

PR c++/83993
* constexpr.c (cxx_eval_outermost_constant_expr): Build NOP_EXPR
around non-constant ADDR_EXPRs rather than clearing TREE_CONSTANT
on ADDR_EXPR.

* g++.dg/init/pr83993-2.C: New test.

--- gcc/cp/constexpr.c.jj   2018-01-30 12:30:24.279362188 +0100
+++ gcc/cp/constexpr.c  2018-01-31 17:18:03.316302653 +0100
@@ -4824,7 +4824,7 @@ cxx_eval_outermost_constant_expr (tree t
   else if (non_constant_p && TREE_CONSTANT (r))
 {
   /* This isn't actually constant, so unset TREE_CONSTANT.  */
-  if (EXPR_P (r))
+  if (EXPR_P (r) && TREE_CODE (r) != ADDR_EXPR)
r = copy_node (r);
   else if (TREE_CODE (r) == CONSTRUCTOR)
r = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (r), r);
--- gcc/testsuite/g++.dg/init/pr83993-2.C.jj2018-01-31 17:17:04.755290928 
+0100
+++ gcc/testsuite/g++.dg/init/pr83993-2.C   2018-01-31 17:17:04.755290928 
+0100
@@ -0,0 +1,14 @@
+// PR c++/83993
+// { dg-do compile }
+// { dg-options "-w" }
+
+int a[5];
+extern int b[];
+int *const c = [6];
+int *const d = [1];
+
+int
+foo ()
+{
+  return c[-4] + d[-1];
+}


Jakub

Re: [C++ PATCH] Don't clear TREE_CONSTANT on ADDR_EXPRs (PR c++/83993)

[Jason Merrill]

On Wed, Jan 24, 2018 at 6:22 PM, Jakub Jelinek  wrote:
> Hi!
>
> cxx_eval_outermost_constant_expr clears TREE_CONSTANT on ADDR_EXPRs that
> aren't considered by C++ constant expressions, but that breaks middle-end
> which relies on TREE_CONSTANT being set on ADDR_EXPR where the address
> is constant.
>
> The following patch just special cases ADDR_EXPR not to clear it.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> As I wrote in the PR, another option would be to restore TREE_CONSTANT
> during genericization if it was cleared earlier in the FE.
>
> 2018-01-24  Jakub Jelinek  
>
> PR c++/83993
> * constexpr.c (cxx_eval_outermost_constant_expr): Don't clear
> TREE_CONSTANT on ADDR_EXPRs.
>
> * g++.dg/init/pr83993-2.C: New test.
>
> --- gcc/cp/constexpr.c.jj   2018-01-24 13:38:40.572913190 +0100
> +++ gcc/cp/constexpr.c  2018-01-24 17:03:16.821440047 +0100
> @@ -4832,7 +4832,7 @@ cxx_eval_outermost_constant_expr (tree t
>
>if (non_constant_p && !allow_non_constant)
>  return error_mark_node;
> -  else if (non_constant_p && TREE_CONSTANT (r))
> +  else if (non_constant_p && TREE_CONSTANT (r) && TREE_CODE (r) != ADDR_EXPR)
>  {
>/* This isn't actually constant, so unset TREE_CONSTANT.  */
>if (EXPR_P (r))

Hmm, what if we check for ADDR_EXPR in the EXPR_P test, so we build a
non-TREE_CONSTANT NOP_EXPR around the ADDR_EXPR instead?

Jason

[C++ PATCH] Don't clear TREE_CONSTANT on ADDR_EXPRs (PR c++/83993)

[Jakub Jelinek]

Hi!

cxx_eval_outermost_constant_expr clears TREE_CONSTANT on ADDR_EXPRs that
aren't considered by C++ constant expressions, but that breaks middle-end
which relies on TREE_CONSTANT being set on ADDR_EXPR where the address
is constant.

The following patch just special cases ADDR_EXPR not to clear it.

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

As I wrote in the PR, another option would be to restore TREE_CONSTANT
during genericization if it was cleared earlier in the FE.

2018-01-24  Jakub Jelinek  

PR c++/83993
* constexpr.c (cxx_eval_outermost_constant_expr): Don't clear
TREE_CONSTANT on ADDR_EXPRs.

* g++.dg/init/pr83993-2.C: New test.

--- gcc/cp/constexpr.c.jj   2018-01-24 13:38:40.572913190 +0100
+++ gcc/cp/constexpr.c  2018-01-24 17:03:16.821440047 +0100
@@ -4832,7 +4832,7 @@ cxx_eval_outermost_constant_expr (tree t
 
   if (non_constant_p && !allow_non_constant)
 return error_mark_node;
-  else if (non_constant_p && TREE_CONSTANT (r))
+  else if (non_constant_p && TREE_CONSTANT (r) && TREE_CODE (r) != ADDR_EXPR)
 {
   /* This isn't actually constant, so unset TREE_CONSTANT.  */
   if (EXPR_P (r))
--- gcc/testsuite/g++.dg/init/pr83993-2.C.jj2018-01-24 17:04:18.823456178 
+0100
+++ gcc/testsuite/g++.dg/init/pr83993-2.C   2018-01-24 17:04:39.593454636 
+0100
@@ -0,0 +1,14 @@
+// PR c++/83993
+// { dg-do compile }
+// { dg-options "-w" }
+
+int a[5];
+extern int b[];
+int *const c = [6];
+int *const d = [1];
+
+int
+foo ()
+{
+  return c[-4] + d[-1];
+}

Jakub