> -----Original Message----- > From: Richard Sandiford [mailto:rdsandif...@googlemail.com] > Sent: Tuesday, March 19, 2013 4:38 PM > To: Moore, Catherine > Cc: gcc-patches@gcc.gnu.org; Rozycki, Maciej > Subject: Re: FW: [PATCH] [MIPS] microMIPS gcc support > > "Moore, Catherine" <catherine_mo...@mentor.com> writes: > >> -----Original Message----- > >> From: Richard Sandiford [mailto:rdsandif...@googlemail.com] > >> Sent: Tuesday, March 19, 2013 3:26 PM > >> To: Moore, Catherine > >> Cc: gcc-patches@gcc.gnu.org; Rozycki, Maciej > >> Subject: Re: FW: [PATCH] [MIPS] microMIPS gcc support > >> > >> "Moore, Catherine" <catherine_mo...@mentor.com> writes: > >> >> >> -----Original Message----- > >> >> >> From: Richard Sandiford [mailto:rdsandif...@googlemail.com] > >> >> >> Sent: Tuesday, March 05, 2013 4:06 PM > >> >> >> To: Moore, Catherine > >> >> >> Cc: gcc-patches@gcc.gnu.org; Rozycki, Maciej > >> >> >> Subject: Re: FW: [PATCH] [MIPS] microMIPS gcc support: > >> >> >> > >> >> >> We have a few internal-only undocumented constraints that > >> >> >> aren't used much, so we should be able to move them to the "Y" > >> >> >> space > >> instead. > >> >> >> The patch below does this for "T" and "U". Then we could use "U" > >> >> >> for new, longer constraints. > >> >> >> > >> >> >> > >> >> >> U<type><factor><bits> > >> >> >> > >> >> >> where <type> is: > >> >> >> > >> >> >> s for signed > >> >> >> u for unsigned > >> >> >> d for decremented unsigned (-1 ... N) > >> >> >> i for incremented unsigned (1 ... N) > >> >> >> > >> >> >> where <factor> is: > >> >> >> > >> >> >> b for "byte" (*1) > >> >> >> h for "halfwords" (*2) > >> >> >> w for "words" (*4) > >> >> >> d for "doublewords" (*8) -- useful for 64-bit MIPS16 but probably > not > >> >> >> needed for 32-bit microMIPS > >> >> >> > >> >> >> and where <bits> is the number of bits. <type> and <factor> > >> >> >> could be replaced with an ad-hoc two-letter combination for special > cases. > >> >> >> E.g. "Uas9" ("add stack") for ADDISUP. > >> >> >> > >> >> >> Just a suggestion though. I'm not saying these names are > >> >> >> totally intuitive or anything, but they should at least be > >> >> >> better than arbitrary > >> >> letters. > >> >> >> > >> >> >> Also, <bits> could be two digits if necessary, or we could just > >> >> >> use hex > >> >> digits. > >> >> > > >> >> > I extended this proposal a bit by: > >> >> > 1. Adding a <type> e for encoded. The constraint will start > >> >> > with Ue, when the operand is an encoded value. > >> >> > 2. I decided to use two digits for <bits>. > >> >> > 3. The ad-hoc combination is used for anything else. > >> >> > >> >> First of all, thanks for coming up with a counter-suggestion. I'm > >> >> hopeless at naming things, so I was hoping there would be at least > >> >> some > >> pushback. > >> >> > >> >> "e" for "encoded" sounds good. I'm less keen on the mixture of > >> >> single- and double-digit widths though (single digit for some > >> >> "Ue"s, double digits for other "U"s.) I think we should either: > >> >> > >> >> (a) use the same number of digits for all "U" constraints. That leaves > >> >> one character for the "Ue" type, which isn't as mnemonic, but is in > >> >> line with what we do elsewhere. > >> >> > >> >> (b) avoid digits in the "Ue" forms and just have an ad-hoc letter > >> combination. > >> > >> FWIW, as far as this point goes, the patch still has "Uea4". > >> > >> >> > +/* Return true if X is a legitimate address that conforms to > >> >> > +the > >> >> requirements > >> >> > + for any of the short microMIPS LOAD or STORE instructions > >> >> > + except > >> LWSP > >> >> > + or SWSP. */ > >> >> > + > >> >> > +bool > >> >> > +umips_address_p (rtx x, bool load, enum machine_mode mode) { > >> >> > + > >> >> > + struct mips_address_info addr; > >> >> > + bool ok = mips_classify_address (&addr, x, mode, false) > >> >> > + && addr.type == ADDRESS_REG > >> >> > + && M16_REG_P (REGNO (addr.reg)) > >> >> > + && CONST_INT_P (addr.offset); > >> >> > + > >> >> > + if (!ok) > >> >> > + return false; > >> >> > + > >> >> > + if (mips_unsigned_immediate_p (INTVAL (addr.offset), 4, 2) > >> >> > + || mips_unsigned_immediate_p (INTVAL (addr.offset), 4, 1)) > >> >> > + return true; > >> >> > + > >> >> > + if (load && IN_RANGE (INTVAL (addr.offset), -1, 14)) > >> >> > + return true; > >> >> > + > >> >> > + if (!load && IN_RANGE (INTVAL (addr.offset), 0, 15)) > >> >> > + return true; > >> >> > + > >> >> > + return false; > >> >> > + > >> >> > +} > >> >> > >> >> No blank lines after "{" and before "}". > >> >> > >> >> More importantly, what cases are these range tests covering? > >> >> Both binutils and qemu seem to think that LW and SW need offsets > >> >> that exactly match: > >> >> > >> >> mips_unsigned_immediate_p (INTVAL (addr.offset), 4, 2) > >> >> > >> > > >> > Those range tests are for the LBU16 and SB16 instructions. LBU16 > >> > supports offsets from -1 to 14 (encodes -1 as 15) while the SB16 > >> > insn supports offsets from 0-15. > >> > >> They need to use separate constraints in that case. The patch as > >> written allows -1 offsets for LW16 too. (In rare cases, offsets like > >> -1 can be used even with aligned word loads.) > >> > >> E.g. we could have: > >> > >> /* Return true if X is legitimate for accessing values of mode MODE, > >> if it is based on a MIPS16 register, and if the offset satisfies > >> OFFSET_PREDICATE. */ > >> > >> bool > >> m16_based_address_p (rtx x, enum machine_mode mode, > >> insn_operand_predicate_fn predicate) { > >> struct mips_address_info addr; > >> return (mips_classify_address (&addr, x, mode, false) > >> && addr.type == ADDRESS_REG > >> && M16_REG_P (REGNO (addr.reg)) > >> && offset_predicate (addr.offset)); } > >> > >> Perhaps if there are so many of these, we should define "T???" to be > >> a memory constraint in which the base register is an M16_REG and in > >> which "???" has the same format as for "U". E.g. "Tuw4" for LW and SW. > >> That's just a suggestion though. > >> > > > > I'd just as soon leave them as "Z" constraints, if that's okay with you. > > Yeah, that's fine. > > > I do see the need for separate constraints. I'll fix that up. > > Adding the new microMIPS memory constraints here would take ZS-ZZ. > > The base microMIPS patch used ZC and ZD and ZR has been used. > > That leaves ZA, ZB, and ZD to ZQ. > > Sounds good, thanks. > HI Richard, Now done. New patch and ChangeLog attached. Thanks for reviewing. Catherine
short-delay.cl
Description: short-delay.cl
short-delay.patch
Description: short-delay.patch
