[ping][PATCH] MIPS: Document the use of FP_D in MIPS16 mode

["Maciej W. Rozycki" <macro at codesourcery dot com>]

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[];
>