| Message ID | 1449463815-4150-1-git-send-email-patrick@parcs.ath.cx |
|---|---|
| State | New |
| Headers | show |
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.
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
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.
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.
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.
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
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" } } */