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 <ja...@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 <rguent...@suse.de> SUSE / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer