Message ID | FD3DCEAC5B03E9408544A1E416F112420192CBEC39@NA-MBX-04.mgc.mentorg.com |
---|---|
State | New |
Headers | show |
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
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
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
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;