From: Richard Sandiford <rdsandiford@googlemail.com>
To: "Moore\, Catherine" <Catherine_Moore@mentor.com>
Cc: "gcc-patches\@gcc.gnu.org" <gcc-patches@gcc.gnu.org>, "Rozycki\,
Maciej" <Maciej_Rozycki@mentor.com>
Subject: Re: FW: [PATCH] [MIPS] microMIPS gcc support
Date: Tue, 19 Mar 2013 19:26:00 -0000 [thread overview]
Message-ID: <878v5jp4xl.fsf@talisman.default> (raw)
In-Reply-To: <FD3DCEAC5B03E9408544A1E416F11242987BE308@NA-MBX-01.mgc.mentorg.com> (Catherine Moore's message of "Fri, 15 Mar 2013 18:00:42 +0000")
"Moore, Catherine" <Catherine_Moore@mentor.com> writes:
>> >> -----Original Message-----
>> >> From: Richard Sandiford [mailto:rdsandiford@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.
Thanks,
Richard
next prev parent reply other threads:[~2013-03-19 19:26 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <FD3DCEAC5B03E9408544A1E416F1124211F9CC2E@NA-MBX-04.mgc.mentorg.com>
2012-07-19 18:51 ` Moore, Catherine
2012-07-20 0:46 ` Richard Sandiford
2013-01-22 20:23 ` Moore, Catherine
2013-01-22 20:27 ` Moore, Catherine
2013-01-23 20:05 ` Richard Sandiford
2013-01-26 3:52 ` Maciej W. Rozycki
2013-01-26 10:17 ` Richard Sandiford
2013-01-27 13:58 ` Maciej W. Rozycki
2013-01-28 10:14 ` Richard Sandiford
2013-02-12 19:00 ` Moore, Catherine
2013-02-13 11:43 ` Richard Sandiford
2013-02-19 16:31 ` Moore, Catherine
2013-02-19 18:25 ` Richard Sandiford
2013-02-19 22:27 ` Maciej W. Rozycki
2013-02-21 2:28 ` Moore, Catherine
2013-02-21 21:20 ` Richard Sandiford
2013-02-24 23:52 ` Moore, Catherine
2013-02-25 9:41 ` Richard Sandiford
2013-02-25 13:55 ` Moore, Catherine
2013-03-04 19:43 ` Moore, Catherine
2013-03-04 20:54 ` Richard Sandiford
2013-03-04 22:47 ` Moore, Catherine
2013-03-05 21:06 ` Richard Sandiford
2013-03-13 20:30 ` Moore, Catherine
2013-03-14 20:55 ` Richard Sandiford
2013-03-15 18:00 ` Moore, Catherine
2013-03-19 19:26 ` Richard Sandiford [this message]
2013-03-19 21:23 ` Moore, Catherine
2013-03-19 21:23 ` Richard Sandiford
2013-03-20 20:15 ` Moore, Catherine
2013-03-20 21:48 ` Richard Sandiford
2013-03-20 22:02 ` Moore, Catherine
2013-03-21 8:05 ` Richard Sandiford
2013-03-21 23:04 ` Moore, Catherine
2013-03-22 0:05 ` Richard Sandiford
2013-03-22 18:31 ` Moore, Catherine
2013-03-23 9:07 ` Richard Sandiford
2013-04-12 18:38 ` Many warnings in MIPS port (Was: [PATCH] [MIPS] microMIPS gcc support) David Daney
2013-04-12 20:35 ` Maciej W. Rozycki
2013-04-12 22:08 ` Moore, Catherine
2013-04-13 6:36 ` David Daney
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=878v5jp4xl.fsf@talisman.default \
--to=rdsandiford@googlemail.com \
--cc=Catherine_Moore@mentor.com \
--cc=Maciej_Rozycki@mentor.com \
--cc=gcc-patches@gcc.gnu.org \
/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).