This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] microMIPS/GAS: Correct the handling of hazard clearance NOPs
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: binutils at sourceware dot org, Chao-ying Fu <fu at mips dot com>, Rich Fuhler <rich at mips dot com>, David Lau <davidlau at mips dot com>, Kevin Mills <kevinm at mips dot com>, Ilie Garbacea <ilie at mips dot com>, Catherine Moore <clm at codesourcery dot com>, Nathan Sidwell <nathan at codesourcery dot com>, Joseph Myers <joseph at codesourcery dot com>
- Date: Tue, 9 Aug 2011 00:27:25 +0100 (BST)
- Subject: Re: [PATCH] microMIPS/GAS: Correct the handling of hazard clearance NOPs
- References: <alpine.DEB.1.10.1108021611410.4083@tp.orcam.me.uk> <877h6qrh69.fsf@firetop.home>
On Sat, 6 Aug 2011, Richard Sandiford wrote:
> These sorts of test are becoming a maintenance and readability problem,
> so it seemed like a good time to abstract them away. Perhaps the only
> noteworthy part is that the microMIPS relaxation code tested:
>
> || (pinfo2 & ~INSN2_ALIAS) == INSN2_UNCOND_BRANCH
>
> instead of plain:
>
> || (pinfo2 & INSN2_UNCOND_BRANCH)
>
> The first form excludes JRC and JRADDIUSP, which should of course not be
> relaxed. But those instructions wouldn't have address expressions or
> fixups anyway, so both conditions ought to be equivalent.
Yes, fair enough as it stands, but things may change and then someone
will scratch their head ten years on or suchlike -- as we do about 1990s
stuff these days sometimes.
How about adding a comment to the effect of your explanation above where
you've removed the ~INSN2_ALIAS mask -- ...
> @@ -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?
Maciej