| Message ID | CAFiYyc2ZP-J8szZ9w9FHq0aJeGamnc2O_c-v44045vktZMg7DQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 07/22/2011 05:36 PM, Richard Guenther wrote: > That said - I'm reasonably happy with the pass now, but it's rather large > (this review took 40min again ...) so I appreciate a second look from > somebody else. > Ian, Do you have a moment to give a second look to a gimple CFG optimization? The optimization removes duplicate basic blocks and reduces code size by 1-2%. The latest patch is posted at http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01602.html. Thanks, - Tom
Tom de Vries <vries@codesourcery.com> writes: > Do you have a moment to give a second look to a gimple CFG optimization? The > optimization removes duplicate basic blocks and reduces code size by 1-2%. > > The latest patch is posted at > http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01602.html. I'm not really the best person to look at this patch, since it applies to areas of the compiler with which I am less familiar.. However, since you ask, I did read through the patch, and it looks OK to me. Since Richi OK'ed it, this patch is OK with the following changes. > +typedef struct same_succ *same_succ_t; > +typedef const struct same_succ *const_same_succ_t; Don't name new types ending with "_t". POSIX reserves names ending with "_t" when <sys/types.h> is #included. Name these something else. > +typedef struct bb_cluster *bb_cluster_t; > +typedef const struct bb_cluster *const_bb_cluster_t; Same here. > +@item -ftree-tail-merge > +Merges identical blocks with same successors. This flag is enabled by default > +at @option{-O2} and higher. The run time of this pass can be limited using > +@option{max-tail-merge-comparisons} parameter. I think this text can be improved to be more meaningful to compiler users. I suggest something like: Look for identical code sequences. When found, replace one with a jump to the other. This optimization is known as tail merging or cross jumping. This flag is enabled [now same as above] Thanks. Ian
On Wed, Aug 24, 2011 at 9:00 PM, Ian Lance Taylor <iant@google.com> wrote: > Tom de Vries <vries@codesourcery.com> writes: > >> Do you have a moment to give a second look to a gimple CFG optimization? The >> optimization removes duplicate basic blocks and reduces code size by 1-2%. >> >> The latest patch is posted at >> http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01602.html. > > > I'm not really the best person to look at this patch, since it applies > to areas of the compiler with which I am less familiar.. However, since > you ask, I did read through the patch, and it looks OK to me. Since > Richi OK'ed it, this patch is OK with the following changes. > > >> +typedef struct same_succ *same_succ_t; >> +typedef const struct same_succ *const_same_succ_t; > > Don't name new types ending with "_t". POSIX reserves names ending with > "_t" when <sys/types.h> is #included. Name these something else. > >> +typedef struct bb_cluster *bb_cluster_t; >> +typedef const struct bb_cluster *const_bb_cluster_t; > > Same here. > > >> +@item -ftree-tail-merge >> +Merges identical blocks with same successors. This flag is enabled by default >> +at @option{-O2} and higher. The run time of this pass can be limited using >> +@option{max-tail-merge-comparisons} parameter. > > I think this text can be improved to be more meaningful to compiler > users. I suggest something like: > > Look for identical code sequences. When found, replace one with a > jump to the other. This optimization is known as tail merging or > cross jumping. This flag is enabled [now same as above] Can you also add a --param for the maximum number of iterations you perform (16 sounds quite high for GCC bootstrap), I'd default it to 2 which seems to catch 99% of all cases. If you already committed the patch just do it as a followup please. Thanks, Richard. > > Thanks. > > Ian >
Index: gcc/tree-flow.h =================================================================== --- gcc/tree-flow.h (revision 175801) +++ gcc/tree-flow.h (working copy) @@ -806,6 +806,9 @@ bool multiplier_allowed_in_address_p (HO unsigned multiply_by_cost (HOST_WIDE_INT, enum machine_mode, bool); bool may_be_nonaddressable_p (tree expr); +/* In tree-ssa-tail-merge.c. */ +extern unsigned int tail_merge_optimize (unsigned int); Eh, tree-flow.h kitchen-sink ;) Please put it into tree-pass.h instead. That said - I'm reasonably happy with the pass now, but it's rather large