This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] MIPS/GAS: Clean-up no-delay slot instruction annotation
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: "Maciej W. Rozycki" <macro at codesourcery dot com>
- Cc: binutils at sourceware dot org
- Date: Sat, 26 Feb 2011 10:04:45 +0000
- Subject: Re: [PATCH] MIPS/GAS: Clean-up no-delay slot instruction annotation
- References: <alpine.DEB.1.10.1102250106270.20460@tp.orcam.me.uk>
"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> The MIPS architecture defines a set of instructions that are not allowed
> to be placed in a branch delay slot or unpredictable behaviour will
> happen. Additionally GAS deliberately does not reorder some other
> instructions that are actually allowed in a branch delay slot, but their
> intent is to trigger an exception under some circumstances and handling
> such an exception, although architecturally supported, is sometimes
> problematic if coming from a branch delay slot.
>
> Currently we do not distinguish between the two cases and furthermore we
> correctly annotate some instructions to keep them out of delay slots, we
> annotate some instructions unnecessarily, and we fail to annotate some,
> which we handle by checking their opcodes explicitly. Ah, and we have
> this honourable SYNC instruction that got awarded with its own flag for
> this very purpose.
>
> I believe this all is unnecessary. We can do with a single flag, if set
> correctly for all the affected instructions, and no explicit opcode
> checks. Here's a change that implements my idea. I have renamed the
> INSN_TRAP flag to INSN_NO_DELAY_SLOT, to reflect its meaning more
> accurately. I have kept the TRAP alias though for instructions that GAS
> deliberately chooses not to schedule into delay slots even they are
> otherwise permitted there. I have added a new alias, NODS, for these
> instructions that are truly forbidden there to make it easy to distinguish
> between the two classes. I have used the new flag for SYNC, the ERET and
> DERET instructions that were checked explicitly and lacked any annotation,
> and updated all the other instructions of this kind including, but not
> limited to MIPS16 compact jumps. I haven't added this flag to branches
> that are already excluded implicitly by means of some obscure conditions
> (like being relaxed or having a fixup attached); though frankly perhaps we
> should use this flag on them too, to make it explicit they cannot be
> scheduled into delay slots. WDYT?
By including INSN_NO_DELAY_SLOT in UBD, etc? Sounds like a good idea.
I like the way you've kept the TRAP vs. NODS distinction in the opcode
tables, so that the short macros give semantic information, and the
#defines map that information to flags. On that basis, if property
X inherently stops the instruction being placed in a delay slot,
it seems better to include INSN_NO_DELAY_SLOT in the #define for
X than to add "|NODS" to each X entry.
Richard