[PATCH] Disable -mbranch-likely for -Os when targetting generic architecture

[PATCH] Disable -mbranch-likely for -Os when targetting generic architecture

[Robert Suchanek]

Hi,

The patch below disables generation of the branch likely instructions for -Os
but only for generic architecture.  The branch likely may result in some
code size reduction but the cost of running the code on R6 core is significant.

Disabling this for generic architecture would therefore be desirable for now.

Ok to apply?

Regards,
Robert

gcc/
	* mips.c (mips_option_override): Enable branch likely for non-generic
	architecture.
---
 gcc/config/mips/mips.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index a9829bd..68def06 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19564,7 +19564,7 @@ mips_option_override (void)
   if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
     {
       if (ISA_HAS_BRANCHLIKELY
-	  && (optimize_size
+	  && ((optimize_size && strncmp (mips_arch_info->name, "mips", 4) != 0)
 	      || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY) == 0))
 	target_flags |= MASK_BRANCHLIKELY;
       else
-- 
2.4.5

Re: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture

[Richard Sandiford]

Robert Suchanek <Robert.Suchanek@imgtec.com> writes:
> The patch below disables generation of the branch likely instructions for -Os
> but only for generic architecture.  The branch likely may result in some
> code size reduction but the cost of running the code on R6 core is significant.

How about instead splitting PTF_AVOID_BRANCHLIKELY into
PTF_AVOID_BRANCHLIKELY_SPEED and PTF_AVOID_BRANCHLIKELY_SIZE?
We could have PTF_AVOID_BRANCHLIKELY_ALWAYS as an OR of the two.

Anything that does string ops on the architecture is suspicious :-)

Thanks,
Richard

RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture

[Matthew Fortune]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Robert Suchanek <Robert.Suchanek@imgtec.com> writes:
> > The patch below disables generation of the branch likely instructions for -Os
> > but only for generic architecture.  The branch likely may result in some
> > code size reduction but the cost of running the code on R6 core is significant.
> 
> How about instead splitting PTF_AVOID_BRANCHLIKELY into
> PTF_AVOID_BRANCHLIKELY_SPEED and PTF_AVOID_BRANCHLIKELY_SIZE?
> We could have PTF_AVOID_BRANCHLIKELY_ALWAYS as an OR of the two.

This sounds OK and is nicer.

> Anything that does string ops on the architecture is suspicious :-)

You can blame me for this.  I advocated the string comparison approach as I had to
do the same thing in gas IIRC for some feature and couldn't think of anything
better to suggest.

Thanks,
Matthew

RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture

[Robert Suchanek]

Hi,

> Richard Sandiford <rdsandiford@googlemail.com> writes:
> > Robert Suchanek <Robert.Suchanek@imgtec.com> writes:
> > > The patch below disables generation of the branch likely instructions for -
> Os
> > > but only for generic architecture.  The branch likely may result in some
> > > code size reduction but the cost of running the code on R6 core is
> significant.
> >
> > How about instead splitting PTF_AVOID_BRANCHLIKELY into
> > PTF_AVOID_BRANCHLIKELY_SPEED and PTF_AVOID_BRANCHLIKELY_SIZE?
> > We could have PTF_AVOID_BRANCHLIKELY_ALWAYS as an OR of the two.
> 
> This sounds OK and is nicer.
> 
> > Anything that does string ops on the architecture is suspicious :-)
> 
> You can blame me for this.  I advocated the string comparison approach as I had
> to
> do the same thing in gas IIRC for some feature and couldn't think of anything
> better to suggest.

Here is an updated version that use the suggested method. Ok to apply?

Regards,
Robert

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(-)

diff --git a/gcc/config/mips/mips-cpus.def b/gcc/config/mips/mips-cpus.def
index e0c77f8..52f9900 100644
--- a/gcc/config/mips/mips-cpus.def
+++ b/gcc/config/mips/mips-cpus.def
@@ -34,28 +34,28 @@ along with GCC; see the file COPYING3.  If not see
 
 /* Entries for generic ISAs.  */
 MIPS_CPU ("mips1", PROCESSOR_R3000, 1, 0)
-MIPS_CPU ("mips2", PROCESSOR_R6000, 2, 0)
-MIPS_CPU ("mips3", PROCESSOR_R4000, 3, 0)
-MIPS_CPU ("mips4", PROCESSOR_R10000, 4, 0)
+MIPS_CPU ("mips2", PROCESSOR_R6000, 2, PTF_AVOID_BRANCHLIKELY_SIZE)
+MIPS_CPU ("mips3", PROCESSOR_R4000, 3, PTF_AVOID_BRANCHLIKELY_SIZE)
+MIPS_CPU ("mips4", PROCESSOR_R10000, 4, PTF_AVOID_BRANCHLIKELY_SIZE)
 /* Prefer not to use branch-likely instructions for generic MIPS32rX
    and MIPS64rX code.  The instructions were officially deprecated
    in revisions 2 and earlier, but revision 3 is likely to downgrade
    that to a recommendation to avoid the instructions in code that
    isn't tuned to a specific processor.  */
-MIPS_CPU ("mips32", PROCESSOR_4KC, 32, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("mips32r2", PROCESSOR_74KF2_1, 33, PTF_AVOID_BRANCHLIKELY)
+MIPS_CPU ("mips32", PROCESSOR_4KC, 32, PTF_AVOID_BRANCHLIKELY_ALWAYS)
+MIPS_CPU ("mips32r2", PROCESSOR_74KF2_1, 33, PTF_AVOID_BRANCHLIKELY_ALWAYS)
 /* mips32r3 is micromips hense why it uses the M4K processor.
    mips32r5 should use the p5600 processor, but there is no definition 
    for this yet, so in the short term we will use the same processor entry 
    as mips32r2.  */
-MIPS_CPU ("mips32r3", PROCESSOR_M4K, 34, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("mips32r5", PROCESSOR_P5600, 36, PTF_AVOID_BRANCHLIKELY)
+MIPS_CPU ("mips32r3", PROCESSOR_M4K, 34, PTF_AVOID_BRANCHLIKELY_ALWAYS)
+MIPS_CPU ("mips32r5", PROCESSOR_P5600, 36, PTF_AVOID_BRANCHLIKELY_ALWAYS)
 MIPS_CPU ("mips32r6", PROCESSOR_I6400, 37, 0)
-MIPS_CPU ("mips64", PROCESSOR_5KC, 64, PTF_AVOID_BRANCHLIKELY)
+MIPS_CPU ("mips64", PROCESSOR_5KC, 64, PTF_AVOID_BRANCHLIKELY_ALWAYS)
 /* ??? For now just tune the generic MIPS64r2 and above for 5KC as well.   */
-MIPS_CPU ("mips64r2", PROCESSOR_5KC, 65, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("mips64r3", PROCESSOR_5KC, 66, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("mips64r5", PROCESSOR_5KC, 68, PTF_AVOID_BRANCHLIKELY)
+MIPS_CPU ("mips64r2", PROCESSOR_5KC, 65, PTF_AVOID_BRANCHLIKELY_ALWAYS)
+MIPS_CPU ("mips64r3", PROCESSOR_5KC, 66, PTF_AVOID_BRANCHLIKELY_ALWAYS)
+MIPS_CPU ("mips64r5", PROCESSOR_5KC, 68, PTF_AVOID_BRANCHLIKELY_ALWAYS)
 MIPS_CPU ("mips64r6", PROCESSOR_I6400, 69, 0)
 
 /* MIPS I processors.  */
@@ -80,8 +80,8 @@ MIPS_CPU ("r4650", PROCESSOR_R4650, 3, 0)
 MIPS_CPU ("r4700", PROCESSOR_R4700, 3, 0)
 MIPS_CPU ("r5900", PROCESSOR_R5900, 3, 0)
 /* ST Loongson 2E/2F processors.  */
-MIPS_CPU ("loongson2e", PROCESSOR_LOONGSON_2E, 3, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("loongson2f", PROCESSOR_LOONGSON_2F, 3, PTF_AVOID_BRANCHLIKELY)
+MIPS_CPU ("loongson2e", PROCESSOR_LOONGSON_2E, 3, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("loongson2f", PROCESSOR_LOONGSON_2F, 3, PTF_AVOID_BRANCHLIKELY_SPEED)
 
 /* MIPS IV processors. */
 MIPS_CPU ("r8000", PROCESSOR_R8000, 4, 0)
@@ -91,7 +91,7 @@ MIPS_CPU ("r14000", PROCESSOR_R10000, 4, 0)
 MIPS_CPU ("r16000", PROCESSOR_R10000, 4, 0)
 MIPS_CPU ("vr5000", PROCESSOR_R5000, 4, 0)
 MIPS_CPU ("vr5400", PROCESSOR_R5400, 4, 0)
-MIPS_CPU ("vr5500", PROCESSOR_R5500, 4, PTF_AVOID_BRANCHLIKELY)
+MIPS_CPU ("vr5500", PROCESSOR_R5500, 4, PTF_AVOID_BRANCHLIKELY_SPEED)
 MIPS_CPU ("rm7000", PROCESSOR_R7000, 4, 0)
 MIPS_CPU ("rm9000", PROCESSOR_R9000, 4, 0)
 
@@ -150,26 +150,26 @@ MIPS_CPU ("1004kf1_1", PROCESSOR_24KF1_1, 33, 0)
 MIPS_CPU ("interaptiv", PROCESSOR_24KF2_1, 33, 0)
 
 /* MIPS32 Release 5 processors.  */
-MIPS_CPU ("p5600", PROCESSOR_P5600, 36, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("m5100", PROCESSOR_M5100, 36, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("m5101", PROCESSOR_M5100, 36, PTF_AVOID_BRANCHLIKELY)
+MIPS_CPU ("p5600", PROCESSOR_P5600, 36, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("m5100", PROCESSOR_M5100, 36, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("m5101", PROCESSOR_M5100, 36, PTF_AVOID_BRANCHLIKELY_SPEED)
 
 /* MIPS64 processors.  */
 MIPS_CPU ("5kc", PROCESSOR_5KC, 64, 0)
 MIPS_CPU ("5kf", PROCESSOR_5KF, 64, 0)
-MIPS_CPU ("20kc", PROCESSOR_20KC, 64, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("sb1", PROCESSOR_SB1, 64, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("sb1a", PROCESSOR_SB1A, 64, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("sr71000", PROCESSOR_SR71000, 64, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("xlr", PROCESSOR_XLR, 64, PTF_AVOID_BRANCHLIKELY)
+MIPS_CPU ("20kc", PROCESSOR_20KC, 64, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("sb1", PROCESSOR_SB1, 64, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("sb1a", PROCESSOR_SB1A, 64, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("sr71000", PROCESSOR_SR71000, 64, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("xlr", PROCESSOR_XLR, 64, PTF_AVOID_BRANCHLIKELY_SPEED)
 
 /* MIPS64 Release 2 processors.  */
-MIPS_CPU ("loongson3a", PROCESSOR_LOONGSON_3A, 65, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("octeon", PROCESSOR_OCTEON, 65, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("octeon+", PROCESSOR_OCTEON, 65, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("octeon2", PROCESSOR_OCTEON2, 65, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("octeon3", PROCESSOR_OCTEON3, 65, PTF_AVOID_BRANCHLIKELY)
-MIPS_CPU ("xlp", PROCESSOR_XLP, 65, PTF_AVOID_BRANCHLIKELY)
+MIPS_CPU ("loongson3a", PROCESSOR_LOONGSON_3A, 65, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("octeon", PROCESSOR_OCTEON, 65, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("octeon+", PROCESSOR_OCTEON, 65, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("octeon2", PROCESSOR_OCTEON2, 65, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("octeon3", PROCESSOR_OCTEON3, 65, PTF_AVOID_BRANCHLIKELY_SPEED)
+MIPS_CPU ("xlp", PROCESSOR_XLP, 65, PTF_AVOID_BRANCHLIKELY_SPEED)
 
 /* MIPS64 Release 6 processors.  */
 MIPS_CPU ("i6400", PROCESSOR_I6400, 69, 0)
diff --git 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;
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 25a1e06..1d9075d 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -44,18 +44,28 @@ extern int target_flags_explicit;
 
 /* Masks that affect tuning.
 
-   PTF_AVOID_BRANCHLIKELY
+   PTF_AVOID_BRANCHLIKELY_SPEED
 	Set if it is usually not profitable to use branch-likely instructions
-	for this target, typically because the branches are always predicted
-	taken and so incur a large overhead when not taken.
+	for this target when optimizing code for speed, typically because
+	the branches are always predicted taken and so incur a large overhead
+	when not taken.
+
+   PTF_AVOID_BRANCHLIKELY_SIZE
+	As above but when optimizing for size.
+
+   PTF_AVOID_BRANCHLIKELY_ALWAYS
+	As above but regardless of whether we optimize for speed or size.
 
    PTF_AVOID_IMADD
 	Set if it is usually not profitable to use the integer MADD or MSUB
 	instructions because of the overhead of getting the result out of
 	the HI/LO registers.  */
 
-#define PTF_AVOID_BRANCHLIKELY	0x1
-#define PTF_AVOID_IMADD		0x2
+#define PTF_AVOID_BRANCHLIKELY_SPEED	0x1
+#define PTF_AVOID_BRANCHLIKELY_SIZE	0x2
+#define PTF_AVOID_BRANCHLIKELY_ALWAYS	(PTF_AVOID_BRANCHLIKELY_SPEED | \
+					 PTF_AVOID_BRANCHLIKELY_SIZE)
+#define PTF_AVOID_IMADD			0x4
 
 /* Information about one recognized processor.  Defined here for the
    benefit of TARGET_CPU_CPP_BUILTINS.  */
-- 
2.4.5

RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture

[Moore, Catherine]

> -----Original Message-----
> From: Robert Suchanek [mailto:Robert.Suchanek@imgtec.com]
> Sent: Friday, September 04, 2015 10:21 AM
> To: Matthew Fortune; Richard Sandiford
> Cc: Moore, Catherine; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic
> architecture
> 
> Hi,
> 
> > Richard Sandiford <rdsandiford@googlemail.com> writes:
> > > Robert Suchanek <Robert.Suchanek@imgtec.com> writes:
> > > > The patch below disables generation of the branch likely
> > > > instructions for -
> > Os
> > > > but only for generic architecture.  The branch likely may result
> > > > in some code size reduction but the cost of running the code on R6
> > > > core is
> > significant.
> > >
> > > How about instead splitting PTF_AVOID_BRANCHLIKELY into
> > > PTF_AVOID_BRANCHLIKELY_SPEED and
> PTF_AVOID_BRANCHLIKELY_SIZE?
> > > We could have PTF_AVOID_BRANCHLIKELY_ALWAYS as an OR of the two.
> >
> > This sounds OK and is nicer.
> >
> > > Anything that does string ops on the architecture is suspicious :-)
> >
> > You can blame me for this.  I advocated the string comparison approach
> > as I had to do the same thing in gas IIRC for some feature and
> > couldn't think of anything better to suggest.
> 
> Here is an updated version that use the suggested method. Ok to apply?
> 
> Regards,
> Robert
> 
> 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.

RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture

[Robert Suchanek]

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

RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture

[Matthew Fortune]

Robert Suchanek <Robert.Suchanek@imgtec.com> writes:
> 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?

This looks like it got lost at some point. I think this is a reasonable
change for safety.

Go ahead and commit.

Thanks,
Matthew

RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture

[Robert Suchanek]

Hi,

> > I'm happy to include this.  Ok to commit with this change?
> 
> This looks like it got lost at some point. I think this is a reasonable
> change for safety.
> 
> Go ahead and commit.
> 
> Thanks,
> Matthew

Committed as r240965.

Regards,
Robert