[PATCH] [C FE] Fold trivial exprs that refer to const vars

[PATCH] [C FE] Fold trivial exprs that refer to const vars

[Patrick Palka]

There is a minor inconsistency in the folding behavior within the C
frontend.  The C frontend does not currently fold the expression "x",
where x is a const int, yet the FE does fold the expression "x + 0".

This happens because decl_constant_value is called in c_fully_fold only
while recursing over the operands of the expression being folded, i.e.
there is no top-level call to decl_constant_value to handle the case
where the expression being folded happens to be a singular expression
such as "x", as opposed to "x + 5" (where x is a const variable).

To fix this inconsistency, this patch calls decl_constant_value in
c_fully fold after folding the given expression.

Bootstrap + regtest in progress on x86_64-pc-linux-gnu, OK to commit if
testing succeeds?

gcc/c/ChangeLog:

	* c-fold.c (c_fully_fold): Call
	decl_constant_value_for_optimization after folding
	the given expression.
	* c-typeck.c (digest_init): Remove redundant call to
	decl_constant_value_for_optimization.

gcc/testsuite/ChangeLog:

	* gcc.dg/fold-const-1.c: New test.
---
 gcc/c/c-fold.c                      |  1 +
 gcc/c/c-typeck.c                    |  1 -
 gcc/testsuite/gcc.dg/fold-const-1.c | 23 +++++++++++++++++++++++
 3 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/fold-const-1.c

diff --git a/gcc/c/c-fold.c b/gcc/c/c-fold.c
index c554e17..ab0b37f 100644
--- a/gcc/c/c-fold.c
+++ b/gcc/c/c-fold.c
@@ -88,6 +88,7 @@ c_fully_fold (tree expr, bool in_init, bool *maybe_const)
     }
   ret = c_fully_fold_internal (expr, in_init, maybe_const,
 			       &maybe_const_itself, false);
+  ret = decl_constant_value_for_optimization (ret);
   if (eptype)
     ret = fold_convert_loc (loc, eptype, ret);
   *maybe_const &= maybe_const_itself;
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index b691072..4886fc2 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -6791,7 +6791,6 @@ digest_init (location_t init_loc, tree type, tree init, tree origtype,
       inside_init = TREE_OPERAND (inside_init, 0);
     }
   inside_init = c_fully_fold (inside_init, require_constant, &maybe_const);
-  inside_init = decl_constant_value_for_optimization (inside_init);
 
   /* Initialization of an array of chars from a string constant
      optionally enclosed in braces.  */
diff --git a/gcc/testsuite/gcc.dg/fold-const-1.c b/gcc/testsuite/gcc.dg/fold-const-1.c
new file mode 100644
index 0000000..9f043b8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-const-1.c
@@ -0,0 +1,23 @@
+/* { dg-options "-O -fdump-tree-gimple -fdump-tree-ccp1" } */
+
+extern void dummy (const void *);
+
+int
+f1 (void)
+{
+  const int x = 7;
+  dummy (&x);
+  return x;
+}
+
+void foo (int);
+
+void
+f2 (void)
+{
+  const int x = 7;
+  foo (x);
+}
+
+/* { dg-final { scan-tree-dump "foo \\(7\\);" "gimple" } } */
+/* { dg-final { scan-tree-dump-times "return 7;" 2 "ccp1" } } */
-- 
2.6.3.517.g5b116b4.dirty

Re: [PATCH] [C FE] Fold trivial exprs that refer to const vars

[Patrick Palka]

On Sun, Dec 6, 2015 at 11:50 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> There is a minor inconsistency in the folding behavior within the C
> frontend.  The C frontend does not currently fold the expression "x",
> where x is a const int, yet the FE does fold the expression "x + 0".
>
> This happens because decl_constant_value is called in c_fully_fold only
> while recursing over the operands of the expression being folded, i.e.
> there is no top-level call to decl_constant_value to handle the case
> where the expression being folded happens to be a singular expression
> such as "x", as opposed to "x + 5" (where x is a const variable).
>
> To fix this inconsistency, this patch calls decl_constant_value in
> c_fully fold after folding the given expression.
>
> Bootstrap + regtest in progress on x86_64-pc-linux-gnu, OK to commit if
> testing succeeds?
>
> gcc/c/ChangeLog:
>
>         * c-fold.c (c_fully_fold): Call
>         decl_constant_value_for_optimization after folding
>         the given expression.
>         * c-typeck.c (digest_init): Remove redundant call to
>         decl_constant_value_for_optimization.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/fold-const-1.c: New test.
> ---
>  gcc/c/c-fold.c                      |  1 +
>  gcc/c/c-typeck.c                    |  1 -
>  gcc/testsuite/gcc.dg/fold-const-1.c | 23 +++++++++++++++++++++++
>  3 files changed, 24 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/fold-const-1.c
>
> diff --git a/gcc/c/c-fold.c b/gcc/c/c-fold.c
> index c554e17..ab0b37f 100644
> --- a/gcc/c/c-fold.c
> +++ b/gcc/c/c-fold.c
> @@ -88,6 +88,7 @@ c_fully_fold (tree expr, bool in_init, bool *maybe_const)
>      }
>    ret = c_fully_fold_internal (expr, in_init, maybe_const,
>                                &maybe_const_itself, false);
> +  ret = decl_constant_value_for_optimization (ret);
>    if (eptype)
>      ret = fold_convert_loc (loc, eptype, ret);
>    *maybe_const &= maybe_const_itself;
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index b691072..4886fc2 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -6791,7 +6791,6 @@ digest_init (location_t init_loc, tree type, tree init, tree origtype,
>        inside_init = TREE_OPERAND (inside_init, 0);
>      }
>    inside_init = c_fully_fold (inside_init, require_constant, &maybe_const);
> -  inside_init = decl_constant_value_for_optimization (inside_init);
>
>    /* Initialization of an array of chars from a string constant
>       optionally enclosed in braces.  */
> diff --git a/gcc/testsuite/gcc.dg/fold-const-1.c b/gcc/testsuite/gcc.dg/fold-const-1.c
> new file mode 100644
> index 0000000..9f043b8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/fold-const-1.c
> @@ -0,0 +1,23 @@
> +/* { dg-options "-O -fdump-tree-gimple -fdump-tree-ccp1" } */
> +
> +extern void dummy (const void *);
> +
> +int
> +f1 (void)
> +{
> +  const int x = 7;
> +  dummy (&x);
> +  return x;
> +}
> +
> +void foo (int);
> +
> +void
> +f2 (void)
> +{
> +  const int x = 7;
> +  foo (x);
> +}
> +
> +/* { dg-final { scan-tree-dump "foo \\(7\\);" "gimple" } } */
> +/* { dg-final { scan-tree-dump-times "return 7;" 2 "ccp1" } } */

Oops, this last dg-final line should say { scan-tree-dump "return 7;"
"ccp1" } of course.

Re: [PATCH] [C FE] Fold trivial exprs that refer to const vars

[Marek Polacek]

On Sun, Dec 06, 2015 at 11:50:15PM -0500, Patrick Palka wrote:
> diff --git a/gcc/c/c-fold.c b/gcc/c/c-fold.c
> index c554e17..ab0b37f 100644
> --- a/gcc/c/c-fold.c
> +++ b/gcc/c/c-fold.c
> @@ -88,6 +88,7 @@ c_fully_fold (tree expr, bool in_init, bool *maybe_const)
>      }
>    ret = c_fully_fold_internal (expr, in_init, maybe_const,
>  			       &maybe_const_itself, false);
> +  ret = decl_constant_value_for_optimization (ret);

Sorry, I don't think you can just do this.  Because for e.g.
  const int x = 7;
  x++;
we'd turn this into
  7++;
, right?  And I'm sure that's going to ICE in gimplifier.

	Marek

Re: [PATCH] [C FE] Fold trivial exprs that refer to const vars

[Patrick Palka]

On Sun, Dec 6, 2015 at 11:50 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> There is a minor inconsistency in the folding behavior within the C
> frontend.  The C frontend does not currently fold the expression "x",
> where x is a const int, yet the FE does fold the expression "x + 0".
>
> This happens because decl_constant_value is called in c_fully_fold only
> while recursing over the operands of the expression being folded, i.e.
> there is no top-level call to decl_constant_value to handle the case
> where the expression being folded happens to be a singular expression
> such as "x", as opposed to "x + 5" (where x is a const variable).
>
> To fix this inconsistency, this patch calls decl_constant_value in
> c_fully fold after folding the given expression.
>
> Bootstrap + regtest in progress on x86_64-pc-linux-gnu, OK to commit if
> testing succeeds?

It just occurred to me that this change is not completely safe because
calling c_fully_fold on an lvalue can now return an rvalue. Callers of
c_fully_fold are not prepared to handle this.  Indeed, this patch
causes a couple of regressions in the handling asm() memory operands
due to this implicit lvalue-rvalue conversion.

Re: [PATCH] [C FE] Fold trivial exprs that refer to const vars

[Joseph Myers]

On Mon, 7 Dec 2015, Patrick Palka wrote:

> To fix this inconsistency, this patch calls decl_constant_value in
> c_fully fold after folding the given expression.

The aim should be to eliminate decl_constant_value use here once all 
folding optimizations are also done on GIMPLE (and generally reduce the 
amount of folding done in the front end), not to use it in more cases.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] [C FE] Fold trivial exprs that refer to const vars

[Patrick Palka]

On Mon, Dec 7, 2015 at 7:20 AM, Marek Polacek <polacek@redhat.com> wrote:
> On Sun, Dec 06, 2015 at 11:50:15PM -0500, Patrick Palka wrote:
>> diff --git a/gcc/c/c-fold.c b/gcc/c/c-fold.c
>> index c554e17..ab0b37f 100644
>> --- a/gcc/c/c-fold.c
>> +++ b/gcc/c/c-fold.c
>> @@ -88,6 +88,7 @@ c_fully_fold (tree expr, bool in_init, bool *maybe_const)
>>      }
>>    ret = c_fully_fold_internal (expr, in_init, maybe_const,
>>                              &maybe_const_itself, false);
>> +  ret = decl_constant_value_for_optimization (ret);
>
> Sorry, I don't think you can just do this.  Because for e.g.
>   const int x = 7;
>   x++;
> we'd turn this into
>   7++;
> , right?  And I'm sure that's going to ICE in gimplifier.

Yes, looks like it.

Re: [PATCH] [C FE] Fold trivial exprs that refer to const vars

[Patrick Palka]

On Mon, Dec 7, 2015 at 7:23 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Mon, 7 Dec 2015, Patrick Palka wrote:
>
>> To fix this inconsistency, this patch calls decl_constant_value in
>> c_fully fold after folding the given expression.
>
> The aim should be to eliminate decl_constant_value use here once all
> folding optimizations are also done on GIMPLE (and generally reduce the
> amount of folding done in the front end), not to use it in more cases.

I see, that makes sense. For now I filed PR 68764 to track this
particular issue.

>
> --
> Joseph S. Myers
> joseph@codesourcery.com