From: "Maciej W. Rozycki" <macro@codesourcery.com>
To: Richard Sandiford <rdsandiford@googlemail.com>
Cc: binutils@sourceware.org
Subject: [ping][PATCH] MIPS: Document the use of FP_D in MIPS16 mode
Date: Wed, 03 Aug 2011 14:55:00 -0000 [thread overview]
Message-ID: <alpine.DEB.1.10.1108031553110.4083@tp.orcam.me.uk> (raw)
In-Reply-To: <alpine.DEB.1.10.1107042309460.4083@tp.orcam.me.uk>
Hi Richard,
Where are we with the change below?
Maciej
On Mon, 4 Jul 2011, Maciej W. Rozycki wrote:
> On Mon, 4 Jul 2011, Richard Sandiford wrote:
>
> > > BTW, I've been wondering about this construct:
> > >
> > > return mask & ~0;
> > >
> > > you've introduced in a couple of places.
> >
> > Oops. Fixed below.
>
> Hmm, makes sense indeed, thanks. Except that it didn't pop up in testing
> which is worrisome and means that all this branch swapping stuff is not
> really terribly well covered. I think we should cook up something more
> systematic than the couple of scattered cases we already have.
>
> > > Also why "can not" instead of "cannot"? My understanding of the
> > > subtleties of English is this essentially means we "are allowed not to
> > > swap" rather than we "are not allowed to swap". The usual disclaimer
> > > applies of course. ;)
> >
> > Those comments were already there. I just moved them around.
>
> Fair enough. I got a notion of being otherwise, but now that I have
> double-checked you are right. Sorry about the confusion.
>
> In that case though I'm tempted to adjust the comments I'll be tweaking
> anyway with the microMIPS changes, if you don't mind that is.
>
> > > Finally I think the change below is needed as well. We shouldn't be
> > > relying on the meaning of FP_D for MIPS16 code as opcode flags are assumed
> > > to have a different meaning in the MIPS16 table. Granted, the location of
> > > the FP_D bit hasn't been assigned to anything (yet), so it's guaranteed to
> > > be zero and the check is harmless,
> >
> > Yes. I checked that before writing it this way. It's guaranteed to be
> > harmless anyway, because "until" MIPS16 gets hardware FP support, we'd
> > never have any set bits.
>
> Actually, I was more worried about this bit being reused for something
> else as a distinct MIPS16_* flag and then hitting this condition
> inadvertently. There is a place in append_insn() using the FPR mask
> unconditionally even in the MIPS16 mode (quite rightfully IMO). Granted,
> it's only used for the calculation of .fmask, that is less of importance
> now that we have DWARF-2, but still.
>
> How about making a suitable comment in include/opcode/mips.h, preferably
> following all the other MIPS16_* flags, that the bit position used for the
> FP_D flag is already reserved for MIPS16 code too? There's a terse
> description already there covering the shared flags. I think a change
> like below should be enough.
>
> I think INSN_ISA3 should be moved to the beginning and made clearly
> distinct. It took me a while to notice it's there. I have decided these
> changes are too trivial to be kept separate, so I've made them both at
> once.
>
> > I structured the code to be consistent with the GPR case. If you feel
> > strongly about it though, feel free to move the conditions inside the
> > existing "!mips_opts.mips16" check.
>
> I just feel no confidence that the notion of such an assumption will
> stand for the next half a dozen years or so. The chance for the MIPS16
> ASE being extended any further is probably slim, but the lifespan of
> binutils is long enough to be careful about any assumptions and about
> long-term maintenance of any piece of code. We seem to keep discovering
> obscure oddities all the time despite quite a strict maintenance regime.
>
> As long as include/opcode/mips.h is updated accordingly I don't insist on
> my previous change though.
>
> Maciej
>
> 2011-07-04 Maciej W. Rozycki <macro@codesourcery.com>
>
> include/opcode/
> * mips.h: Document the use of FP_D in MIPS16 mode. Adjust the
> order of flags documented.
>
> Index: include/opcode/mips.h
> ===================================================================
> RCS file: /cvs/src/src/include/opcode/mips.h,v
> retrieving revision 1.73
> diff -u -p -r1.73 mips.h
> --- include/opcode/mips.h 28 Feb 2011 16:06:51 -0000 1.73
> +++ include/opcode/mips.h 4 Jul 2011 22:57:01 -0000
> @@ -1132,6 +1132,9 @@ extern int bfd_mips_num_opcodes;
>
> /* The following flags have the same value for the mips16 opcode
> table:
> +
> + INSN_ISA3
> +
> INSN_UNCOND_BRANCH_DELAY
> INSN_COND_BRANCH_DELAY
> INSN_COND_BRANCH_LIKELY (never used)
> @@ -1140,7 +1143,7 @@ extern int bfd_mips_num_opcodes;
> INSN_WRITE_HI
> INSN_WRITE_LO
> INSN_TRAP
> - INSN_ISA3
> + FP_D (never used)
> */
>
> extern const struct mips_opcode mips16_opcodes[];
>
next prev parent reply other threads:[~2011-08-03 14:55 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-02-23 13:39 [PATCH] MIPS/GAS: Fix DWARF-2 with branch swapping for MIPS16 code Maciej W. Rozycki
2011-06-29 21:26 ` Richard Sandiford
2011-07-02 1:20 ` Maciej W. Rozycki
2011-07-02 7:33 ` Richard Sandiford
2011-07-04 20:30 ` Maciej W. Rozycki
2011-07-04 22:20 ` Richard Sandiford
2011-07-05 0:28 ` Maciej W. Rozycki
2011-07-05 11:29 ` Richard Sandiford
2011-08-03 14:55 ` Maciej W. Rozycki [this message]
2011-08-03 19:47 ` [ping][PATCH] MIPS: Document the use of FP_D in MIPS16 mode Richard Sandiford
2011-08-03 21:13 ` Maciej W. Rozycki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.1.10.1108031553110.4083@tp.orcam.me.uk \
--to=macro@codesourcery.com \
--cc=binutils@sourceware.org \
--cc=rdsandiford@googlemail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).