Re: [PATCH] Fix some further gimple_clobber_p fallout (PR middle-end/51644)

[Richard Guenther <rguenther at suse dot de>]

On Wed, 21 Dec 2011, Jakub Jelinek wrote:

> Hi!
> 
> The following testcases fail on the trunk, because at -O0
> we don't decide to copy the finally stmts if there is more than one
> destination.  That is of course desirable for debugging reasons if
> there are any real stmts, but if the try/finally has been created only to
> hold gimple_clobber_p stmts that will not result in any code generated
> in those spots, we really should duplicate those.
> The second hunk is to use the right costs for the clobber stmts, they
> are expanded to nothing.  Without that for -O2 and perhaps many clobber
> stmts we could still decide not to duplicate.  I guess it should help
> inlining too if the costs much better the reality.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok

Thanks,
Richard.

> 2011-12-21  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/51644
> 	PR middle-end/51647
> 	* tree-eh.c (decide_copy_try_finally): At -O0, return true
> 	even when ndests is not 1, if there are only gimple_clobber_p
> 	(or debug) stmts in the finally sequence.
> 	* tree-inline.c (estimate_num_insns): Return 0 for gimple_clobber_p
> 	stmts.
> 
> 	* gcc.dg/pr51644.c: New test.
> 	* g++.dg/warn/Wreturn-4.C: New test.
> 
> --- gcc/tree-eh.c.jj	2011-12-14 08:11:03.000000000 +0100
> +++ gcc/tree-eh.c	2011-12-21 12:15:49.749301513 +0100
> @@ -1538,7 +1538,20 @@ decide_copy_try_finally (int ndests, boo
>      }
>  
>    if (!optimize)
> -    return ndests == 1;
> +    {
> +      gimple_stmt_iterator gsi;
> +
> +      if (ndests == 1)
> +        return true;
> +
> +      for (gsi = gsi_start (finally); !gsi_end_p (gsi); gsi_next (&gsi))
> +	{
> +	  gimple stmt = gsi_stmt (gsi);
> +	  if (!is_gimple_debug (stmt) && !gimple_clobber_p (stmt))
> +	    return false;
> +	}
> +      return true;
> +    }
>  
>    /* Finally estimate N times, plus N gotos.  */
>    f_estimate = count_insns_seq (finally, &eni_size_weights);
> --- gcc/tree-inline.c.jj	2011-12-14 08:11:03.000000000 +0100
> +++ gcc/tree-inline.c	2011-12-21 11:17:04.708413203 +0100
> @@ -3482,6 +3482,9 @@ estimate_num_insns (gimple stmt, eni_wei
>  	 likely be a real store, so the cost of the GIMPLE_ASSIGN is the cost
>  	 of moving something into "a", which we compute using the function
>  	 estimate_move_cost.  */
> +      if (gimple_clobber_p (stmt))
> +	return 0;	/* ={v} {CLOBBER} stmt expands to nothing.  */
> +
>        lhs = gimple_assign_lhs (stmt);
>        rhs = gimple_assign_rhs1 (stmt);
>  
> --- gcc/testsuite/gcc.dg/pr51644.c.jj	2011-12-21 12:24:22.185537200 +0100
> +++ gcc/testsuite/gcc.dg/pr51644.c	2011-12-21 12:32:01.493946505 +0100
> @@ -0,0 +1,33 @@
> +/* PR middle-end/51644 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wall -fexceptions" } */
> +
> +#include <stdarg.h>
> +
> +extern void baz (int, va_list) __attribute__ ((__noreturn__));
> +
> +__attribute__ ((__noreturn__))
> +void
> +foo (int s, ...)
> +{
> +  va_list ap;
> +  va_start (ap, s);
> +  baz (s, ap);
> +  va_end (ap);
> +}		/* { dg-bogus "function does return" } */
> +
> +__attribute__ ((__noreturn__))
> +void
> +bar (int s, ...)
> +{
> +  va_list ap1;
> +  va_start (ap1, s);
> +  {
> +    va_list ap2;
> +    va_start (ap2, s);
> +    baz (s, ap1);
> +    baz (s, ap2);
> +    va_end (ap2);
> +  }
> +  va_end (ap1);
> +}		/* { dg-bogus "function does return" } */
> --- gcc/testsuite/g++.dg/warn/Wreturn-4.C.jj	2011-12-21 12:33:33.328428381 +0100
> +++ gcc/testsuite/g++.dg/warn/Wreturn-4.C	2011-12-21 12:32:44.000000000 +0100
> @@ -0,0 +1,48 @@
> +// PR middle-end/51647
> +// { dg-do compile }
> +// { dg-options "-Wall" }
> +
> +enum PropertyAttributes { NONE = 1 };
> +enum PropertyType { NORMAL = 0, FIELD = 1 };
> +class LookupResult;
> +
> +template <typename T>
> +struct Handle
> +{
> +  inline explicit Handle (T *obj) __attribute__ ((always_inline)) {}
> +  inline T *operator-> () const __attribute__ ((always_inline)) { return 0; }
> +};
> +
> +struct JSObject
> +{
> +  bool IsGlobalObject () { return false; }
> +};
> +
> +struct Isolate
> +{
> +  LookupResult *top_lookup_result () { return 0; }
> +};
> +
> +struct LookupResult
> +{
> +  explicit LookupResult (Isolate *isolate) {}
> +  JSObject *holder () { return 0; }
> +  PropertyType type () { return NORMAL; }
> +};
> +
> +int
> +test (LookupResult *lookup)
> +{
> +  Handle <JSObject> holder (lookup->holder ());
> +  switch (lookup->type ())
> +    {
> +    case NORMAL:
> +      if (holder->IsGlobalObject ())
> +	return 2;
> +      else
> +	return 3;
> +      break;
> +    default:
> +      return 4;
> +    }
> +}	// { dg-bogus "control reaches end of non-void function" }
> 
> 	Jakub
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer