This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] microMIPS/GAS: Correct the handling of hazard clearance NOPs


On Tue, 9 Aug 2011, Richard Sandiford wrote:

> >  How about adding a comment to the effect of your explanation above where 
> > you've removed the ~INSN2_ALIAS mask -- ...
> 
> I think it's the other way round.  The INSN2_ALIAS check was far from
> obvious.  When I first saw it, I wondered why we were excluding branches
> that read from registers (since we have plenty of INSN2_READ_*s).  Then I
> realised that we weren't trying to exclude that as such; we were trying
> to exclude indirect jumps.  But JR is INSN2_UNCOND_BRANCH_DELAY for all
> of normal MIPS, MIPS16 and microMIPS, and has never had (or needed)
> special checks in the relaxation code.  AFAIK, noone has every been
> confused by that.  So...

 I mean not about INSN2_ALIAS (obviously?), which was just a way to avoid 
introducing further pinfo flags (that we've got none left now; even though 
for reasons that escape me we waste 2 * 32 bits for each entry on 64-bit 
platforms -- are "long long" types still a no-no in our tree, BTW?), but 
that we don't care here about JRC and JRADDIUSP that would accidentally 
match.

> >> @@ -4198,16 +4224,12 @@ append_insn (struct mips_cl_insn *ip, ex
> >>  	   && address_expr
> >>  	   && ((relax32 && *reloc_type == BFD_RELOC_16_PCREL_S2)
> >>  	       || *reloc_type > BFD_RELOC_UNUSED)
> >> -	   && (pinfo & INSN_UNCOND_BRANCH_DELAY
> >> -	       || pinfo & INSN_COND_BRANCH_DELAY
> >> -	       || (pinfo2 & ~INSN2_ALIAS) == INSN2_UNCOND_BRANCH
> >> -	       || pinfo2 & INSN2_COND_BRANCH))
> >> +	   && (delayed_branch_p (ip) || compact_branch_p (ip)))
> >>      {
> >>        bfd_boolean relax16 = *reloc_type > BFD_RELOC_UNUSED;
> >>        int type = relax16 ? *reloc_type - BFD_RELOC_UNUSED : 0;
> >
> >  ... specifically here?
> 
> no, I don't think that's a good idea.  The new form brings the microMIPS
> code in line with the normal MIPS and MIPS16 code, and is IMO what a
> reader is likely to expect.

 For MIPS16 code there's no check for branches made that would enable 
relaxation, hence no such problem (standard MIPS code checks, but there's 
no need for any exclusions).

 However you like, anyway...

  Maciej


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]