> -----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

Attachment: short-delay.cl
Description: short-delay.cl

Attachment: short-delay.patch
Description: short-delay.patch

Reply via email to