Re: [PATCH] Fix optimize_stmt (PR middle-end/37337)

["Richard Guenther" <richard dot guenther at gmail dot com>]

On Mon, Sep 8, 2008 at 5:43 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> This testcase ICEs because maybe_clean_or_replace_eh_stmt hasn't been
> called after fold_stmt changed a __fprintf_chk call into fputs.
> optimize_stmt has code to call it, but it calls it only if
> gimple_modified_p (stmt).  That's not true in this case, as fold_stmt
> called gsi_replace and that called update_modified_stmt.
> This patch fixes it by doing the maybe_clean_or_replace_eh_stmt call
> even in that case - remembering if gimple_modified_p has been already set
> once.  Additionally it removes 2 redundant may_have_exposed_new_symbols
> initializations.  We have:
>  bool may_have_exposed_new_symbols = false;
> ...
>  may_have_exposed_new_symbols = false;
> ...
>  may_have_exposed_new_symbols = cprop_into_stmt (stmt);
> and no stmt among the ... statements ever touch that variable.
>
> Ok for trunk?

Ok.

Thanks,
Richard.

> 2008-09-08  Jakub Jelinek  <jakub@redhat.com>
>
>        PR middle-end/37337
>        * tree-ssa-dom.c (optimize_stmt): Call maybe_clean_or_replace_eh_stmt
>        even when a stmt has been gimple_modified_p, but after fold_stmt is
>        not any longer.  Remove unneeded may_have_exposed_new_symbols
>        initializations.
>
>        * g++.dg/tree-ssa/pr37337.C: New test.
>
> --- gcc/tree-ssa-dom.c.jj       2008-09-05 12:56:32.000000000 +0200
> +++ gcc/tree-ssa-dom.c  2008-09-08 17:03:58.000000000 +0200
> @@ -2179,7 +2179,8 @@ optimize_stmt (struct dom_walk_data *wal
>  {
>   gimple stmt, old_stmt;
>   bool may_optimize_p;
> -  bool may_have_exposed_new_symbols = false;
> +  bool may_have_exposed_new_symbols;
> +  bool modified_p = false;
>
>   old_stmt = stmt = gsi_stmt (si);
>
> @@ -2188,7 +2189,6 @@ optimize_stmt (struct dom_walk_data *wal
>
>   update_stmt_if_modified (stmt);
>   opt_stats.num_stmts++;
> -  may_have_exposed_new_symbols = false;
>   push_stmt_changes (gsi_stmt_ptr (&si));
>
>   if (dump_file && (dump_flags & TDF_DETAILS))
> @@ -2237,6 +2237,10 @@ optimize_stmt (struct dom_walk_data *wal
>
>         Indicate we will need to rescan and rewrite the statement.  */
>       may_have_exposed_new_symbols = true;
> +      /* Indicate that maybe_clean_or_replace_eh_stmt needs to be called,
> +        even if fold_stmt updated the stmt already and thus cleared
> +        gimple_modified_p flag on it.  */
> +      modified_p = true;
>     }
>
>   /* Check for redundant computations.  Do this optimization only
> @@ -2285,7 +2289,7 @@ optimize_stmt (struct dom_walk_data *wal
>
>      Ultimately I suspect we're going to need to change the interface
>      into the SSA_NAME manager.  */
> -  if (gimple_modified_p (stmt))
> +  if (gimple_modified_p (stmt) || modified_p)
>     {
>       tree val = NULL;
>
> --- gcc/testsuite/g++.dg/tree-ssa/pr37337.C.jj  2008-09-08 17:27:10.000000000 +0200
> +++ gcc/testsuite/g++.dg/tree-ssa/pr37337.C     2008-09-08 17:22:58.000000000 +0200
> @@ -0,0 +1,37 @@
> +// PR middle-end/37337
> +// { dg-do compile }
> +// { dg-options "-O2" }
> +
> +extern "C"
> +{
> +  typedef struct _IO_FILE FILE;
> +  extern int __fprintf_chk (FILE *, int, const char *, ...);
> +  extern inline __attribute__ ((always_inline, gnu_inline, artificial))
> +  int fprintf (FILE *s, const char *f, ...)
> +  {
> +    return __fprintf_chk (s, 1, f, __builtin_va_arg_pack ());
> +  }
> +}
> +
> +extern int a;
> +struct A
> +{
> +  virtual ~A (void)
> +  {
> +  }
> +};
> +
> +struct B : public A
> +{
> +  B ();
> +  FILE *b;
> +};
> +
> +void f (int *);
> +B::B ()
> +{
> +  f (&a);
> +  for (int i = 0; i < 6; i++)
> +    fprintf (b, "%02x", 0xff);
> +  fprintf (b, "\n--\n");
> +}
>
>        Jakub
>