From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6232 invoked by alias); 3 Aug 2011 14:55:51 -0000 Received: (qmail 6222 invoked by uid 22791); 3 Aug 2011 14:55:49 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 03 Aug 2011 14:55:31 +0000 Received: (qmail 19410 invoked from network); 3 Aug 2011 14:55:31 -0000 Received: from unknown (HELO tp.orcam.me.uk) (macro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 3 Aug 2011 14:55:31 -0000 Date: Wed, 03 Aug 2011 14:55:00 -0000 From: "Maciej W. Rozycki" To: Richard Sandiford cc: binutils@sourceware.org Subject: [ping][PATCH] MIPS: Document the use of FP_D in MIPS16 mode In-Reply-To: Message-ID: References: <87boxg5nyo.fsf@firetop.home> <87boxdcf2b.fsf@firetop.home> <87ei254wmb.fsf@firetop.home> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2011-08/txt/msg00028.txt.bz2 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 > > 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[]; >