Robert Suchanek <robert.sucha...@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
