From: Robert Suchanek <Robert.Suchanek@imgtec.com>
To: "Moore, Catherine" <Catherine_Moore@mentor.com>,
Matthew Fortune <Matthew.Fortune@imgtec.com>,
Richard Sandiford <rdsandiford@googlemail.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture
Date: Tue, 24 May 2016 15:35:00 -0000 [thread overview]
Message-ID: <B5E67142681B53468FAF6B7C313565624F4EDC7A@hhmail02.hh.imgtec.org> (raw)
In-Reply-To: <FD3DCEAC5B03E9408544A1E416F112420192CBEC39@NA-MBX-04.mgc.mentorg.com>
Hi Catherine,
Apologies for the (very) late reply.
It appears that I never replied to the last message.
> > gcc/
> > * config/mips/mips-cpus.def: Replace PTF_AVOID_BRANCHLIKELY
> > with
> > PTF_AVOID_BRANCHLIKELY_ALWAYS for generic architecture and
> > with
> > PTF_AVOID_BRANCHLIKELY_SPEED for others.
> > (mips2, mips3, mips4): Add PTF_AVOID_BRANCHLIKELY_SIZE to tune
> > flags.
> > * config/mips/mips.c (mips_option_override): Enable the branch
> > likely
> > depending on the tune flags and optimization level.
> > * config/mips/mips.h (PTF_AVOID_BRANCHLIKELY): Remove.
> > (PTF_AVOID_BRANCHLIKELY_SPEED): Define.
> > (PTF_AVOID_BRANCHLIKELY_SIZE): Likewise.
> > (PTF_AVOID_BRANCHLIKELY_ALWAYS): Likewise.
> > ---
> > gcc/config/mips/mips-cpus.def | 56 +++++++++++++++++++++---------------
> > -------
> > gcc/config/mips/mips.c | 6 +++--
> > gcc/config/mips/mips.h | 20 ++++++++++++----
> > 3 files changed, 47 insertions(+), 35 deletions(-)
> >
> > a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 0e0ecf2..f8775c4
> > 100644
> > --- a/gcc/config/mips/mips.c
> > +++ b/gcc/config/mips/mips.c
> > @@ -17916,8 +17916,10 @@ mips_option_override (void)
> > if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
> > {
> > if (ISA_HAS_BRANCHLIKELY
> > - && (optimize_size
> > - || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY)
> > == 0))
> > + && ((optimize_size && (mips_tune_info->tune_flags
> > + & PTF_AVOID_BRANCHLIKELY_SIZE) == 0)
> > + || (!optimize_size && (mips_tune_info->tune_flags
> > + & PTF_AVOID_BRANCHLIKELY_SPEED) ==
> > 0)))
> > target_flags |= MASK_BRANCHLIKELY;
> > else
> > target_flags &= ~MASK_BRANCHLIKELY;
>
> Should this check be:
> Index: mips.c
> ===================================================================
> --- mips.c (revision 229138)
> +++ mips.c (working copy)
> @@ -17758,8 +17758,15 @@
> if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
> {
> if (ISA_HAS_BRANCHLIKELY
> - && (optimize_size
> - || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY) == 0))
> + && ((optimize_size
> + && (mips_tune_info->tune_flags
> + & PTF_AVOID_BRANCHLIKELY_SIZE) == 0)
> + || (!optimize_size
> + && optimize > 0
> + && ((mips_tune_info->tune_flags
> + & PTF_AVOID_BRANCHLIKELY_SPEED) == 0))
> + || (mips_tune_info->tune_flags
> + & PTF_AVOID_BRANCHLIKELY_ALWAYS) == 0))
> target_flags |= MASK_BRANCHLIKELY;
> else
> target_flags &= ~MASK_BRANCHLIKELY;
>
> Instead? I don't see a use of PTF_AVOID_BRANCH_ALWAYS in your patch, but it
> seems like it should be checked.
>
I did that on purpose at the time as the check looked redundant as it will be one
or the other. However, for easier reading and a potential redefinition of *_ALWAYS
e.g. to a unique value then the extra check is a must.
I'm happy to include this. Ok to commit with this change?
Regards,
Robert
next prev parent reply other threads:[~2016-05-24 14:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-14 13:14 Robert Suchanek
2015-08-20 20:15 ` Richard Sandiford
2015-08-20 21:52 ` Matthew Fortune
2015-09-04 14:29 ` Robert Suchanek
2015-10-22 19:09 ` Moore, Catherine
2016-05-24 15:35 ` Robert Suchanek [this message]
2016-09-21 13:32 ` Matthew Fortune
2016-10-11 8:00 ` Robert Suchanek
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=B5E67142681B53468FAF6B7C313565624F4EDC7A@hhmail02.hh.imgtec.org \
--to=robert.suchanek@imgtec.com \
--cc=Catherine_Moore@mentor.com \
--cc=Matthew.Fortune@imgtec.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=rdsandiford@googlemail.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).