From: Richard Guenther <richard.guenther@gmail.com>
To: Ian Lance Taylor <iant@google.com>
Cc: Tom de Vries <vries@codesourcery.com>,
Steven Bosscher <stevenb.gcc@gmail.com>,
gcc-patches@gcc.gnu.org
Subject: Re: [PATCH, PR43864] Gimple level duplicate block cleanup.
Date: Thu, 25 Aug 2011 09:30:00 -0000 [thread overview]
Message-ID: <CAFiYyc2086tVOdHctw6M5K2n11-9uvUd3HT5gEVaKoa+bEYzsQ@mail.gmail.com> (raw)
In-Reply-To: <mcrmxey8wio.fsf@coign.corp.google.com>
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
>
next prev parent reply other threads:[~2011-08-25 7:31 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-08 9:49 Tom de Vries
2011-06-08 9:55 ` [PATCH, PR43864] Gimple level duplicate block cleanup - test cases Tom de Vries
2011-07-18 2:54 ` Tom de Vries
2011-08-19 18:38 ` Tom de Vries
2011-08-25 10:09 ` Richard Guenther
2011-06-08 10:09 ` [PATCH, PR43864] Gimple level duplicate block cleanup Richard Guenther
2011-06-08 10:40 ` Steven Bosscher
2011-06-10 17:16 ` Tom de Vries
2011-06-14 15:12 ` Richard Guenther
2011-07-12 12:21 ` Tom de Vries
2011-07-12 14:37 ` Richard Guenther
2011-07-18 0:41 ` Tom de Vries
2011-07-22 15:54 ` Richard Guenther
2011-08-19 18:33 ` Tom de Vries
2011-08-24 9:00 ` Tom de Vries
2011-08-25 1:07 ` Ian Lance Taylor
2011-08-25 9:30 ` Richard Guenther [this message]
2011-06-10 18:43 ` Jeff Law
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFiYyc2086tVOdHctw6M5K2n11-9uvUd3HT5gEVaKoa+bEYzsQ@mail.gmail.com \
--to=richard.guenther@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=iant@google.com \
--cc=stevenb.gcc@gmail.com \
--cc=vries@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).