From: Richard Sandiford <rdsandiford@googlemail.com>
To: "Fu\, Chao-Ying" <fu@mips.com>
Cc: "Maciej W. Rozycki" <macro@codesourcery.com>,
<binutils@sourceware.org>, "Garbacea\, Ilie" <ilie@mips.com>,
"Joseph Myers" <joseph@codesourcery.com>,
"Catherine Moore" <clm@codesourcery.com>,
<dan@codesourcery.com>
Subject: Re: [PATCH] MIPS: microMIPS and MCU ASE instruction set support
Date: Wed, 26 May 2010 19:47:00 -0000 [thread overview]
Message-ID: <87eigyzbr4.fsf@firetop.home> (raw)
In-Reply-To: <94BD67F8AF3ED34FA362C662BA1F12C502BB5F7F@MTVEXCHANGE.mips.com> (Chao-Ying Fu's message of "Mon, 24 May 2010 15:24:13 -0700")
"Fu, Chao-Ying" <fu@mips.com> writes:
> Richard Sandiford wrote:
>> > (mips_relax_frag): Handle microMIPS.
>>
>> + gas_assert (fixp->fx_r_type == BFD_RELOC_16_PCREL_S2
>> + || fixp->fx_r_type == BFD_RELOC_MICROMIPS_7_PCREL_S1
>> + || fixp->fx_r_type == BFD_RELOC_MICROMIPS_10_PCREL_S1
>> + || fixp->fx_r_type == BFD_RELOC_MICROMIPS_16_PCREL_S1);
>> +
>> + /* For 7/10 PCREL_S1, we just need to use
>> fixp->fx_addnumber. */
>> + if (fixp->fx_r_type == BFD_RELOC_MICROMIPS_7_PCREL_S1
>> + || fixp->fx_r_type == BFD_RELOC_MICROMIPS_10_PCREL_S1)
>> + reloc->addend = fixp->fx_addnumber;
>> + else
>> + /* At this point, fx_addnumber is "symbol offset -
>> pcrel address".
>> + Relocations want only the symbol offset. */
>> + reloc->addend = fixp->fx_addnumber + reloc->address;
>>
>> A better comment is needed. _Why_ do you just need fx_addnumber?
>>
>
> Thanks for the review! The explanation is in another place as
> follows.
> Maybe we need to copy the comment to tc_gen_reloc from md_pcrel_from.
> Ex:
> long
> md_pcrel_from (fixS *fixP)
> {
> valueT addr = fixP->fx_where + fixP->fx_frag->fr_address;
> switch (fixP->fx_r_type)
> {
> /* We don't add addr, because it will cause the error checking of
> "addnumber" fail in write.c for *7/10_PCREL_S1.
> In tc_gen_reloc, we just use fixp->fx_addnumber. */
> case BFD_RELOC_MICROMIPS_7_PCREL_S1:
> case BFD_RELOC_MICROMIPS_10_PCREL_S1:
> /* Return the beginning of the delay slot from the current insn.
> */
> return 2;
>
> case BFD_RELOC_MICROMIPS_16_PCREL_S1:
> case BFD_RELOC_MICROMIPS_JMP:
> case BFD_RELOC_16_PCREL_S2:
> case BFD_RELOC_MIPS_JMP:
> /* Return the address of the delay slot. */
> return addr + 4;
> ...
> The field of *7/10_PCREL_S1 is limited in the 16-bit instructions.
> If we add the "address", write.c will fail to check these two
> relocations due to overflow or something (I kind of forgot). From
> debugging, adding "address" is no use at all, because later "address" is
> subtracted.
Ah, thanks, that's a good explanation. Yeah, at least a cross-reference
would be useful if we keep things as they are. However...
...I think you mean this bit of write.c:
if (fixP->fx_size < sizeof (valueT) && 0)
{
valueT mask;
mask = 0;
mask--; /* Set all bits to one. */
mask <<= fixP->fx_size * 8 - (fixP->fx_signed ? 1 : 0);
if ((add_number & mask) != 0 && (add_number & mask) != mask)
{
char buf[50], buf2[50];
sprint_value (buf, fragP->fr_address + fixP->fx_where);
if (add_number > 1000)
sprint_value (buf2, add_number);
else
sprintf (buf2, "%ld", (long) add_number);
as_bad_where (fixP->fx_file, fixP->fx_line,
_("value of %s too large for field of %d bytes at %s"),
buf2, fixP->fx_size, buf);
} /* Generic error checking. */
}
That check's bogus for these relocations anyway, since it doesn't take
the implied shift into account. I think there's an argument to say
we should set fx_no_overflow for these relocations and leave the
overflow checking to bfd. You'll still get a "relocation overflow"
error if the final in-place addend really is too big.
Richard
next prev parent reply other threads:[~2010-05-26 19:47 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-18 18:19 Maciej W. Rozycki
2010-05-23 21:38 ` Richard Sandiford
2010-05-24 22:25 ` Fu, Chao-Ying
2010-05-26 19:47 ` Richard Sandiford [this message]
2010-06-01 14:21 ` Maciej W. Rozycki
2010-06-01 14:39 ` Catherine Moore
2010-06-01 22:04 ` Richard Sandiford
2010-06-01 22:47 ` Fu, Chao-Ying
2010-06-05 9:17 ` Richard Sandiford
2010-07-26 10:56 ` [PATCH] MIPS: microMIPS ASE support Maciej W. Rozycki
2010-07-26 13:25 ` Nathan Froyd
2010-07-26 13:53 ` Maciej W. Rozycki
2010-07-26 19:03 ` Richard Sandiford
2010-12-07 1:13 ` Maciej W. Rozycki
2010-12-12 14:59 ` Richard Sandiford
2010-12-14 13:30 ` Maciej W. Rozycki
2010-12-14 14:51 ` Richard Sandiford
2010-12-16 11:54 ` Maciej W. Rozycki
2010-12-18 10:26 ` Richard Sandiford
2010-12-14 17:56 ` Joseph S. Myers
2010-12-16 15:28 ` Maciej W. Rozycki
2010-12-17 20:56 ` Fu, Chao-Ying
2010-12-18 10:09 ` Richard Sandiford
2011-01-02 11:36 ` Richard Sandiford
2011-02-21 15:35 ` Maciej W. Rozycki
2011-02-22 20:12 ` Fu, Chao-Ying
2011-02-22 20:19 ` Fu, Chao-Ying
2011-02-24 10:46 ` Maciej W. Rozycki
2011-02-26 11:41 ` Richard Sandiford
2011-02-28 16:41 ` Maciej W. Rozycki
2011-02-26 0:00 ` Maciej W. Rozycki
2011-03-13 9:23 ` Richard Sandiford
2011-07-25 7:49 ` Richard Sandiford
2011-07-26 2:01 ` Maciej W. Rozycki
2011-07-29 0:58 ` Maciej W. Rozycki
2011-07-29 11:30 ` Richard Sandiford
2011-07-29 22:52 ` Maciej W. Rozycki
2011-02-26 11:36 ` Richard Sandiford
2011-07-26 14:00 ` Maciej W. Rozycki
2010-05-26 20:19 ` [PATCH] MIPS: microMIPS and MCU ASE instruction set support Richard Sandiford
2010-05-27 21:39 ` Richard Sandiford
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=87eigyzbr4.fsf@firetop.home \
--to=rdsandiford@googlemail.com \
--cc=binutils@sourceware.org \
--cc=clm@codesourcery.com \
--cc=dan@codesourcery.com \
--cc=fu@mips.com \
--cc=ilie@mips.com \
--cc=joseph@codesourcery.com \
--cc=macro@codesourcery.com \
/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).