public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: "Fu, Chao-Ying" <fu@mips.com>
To: "Richard Sandiford" <rdsandiford@googlemail.com>,
	       "Maciej W. Rozycki" <macro@codesourcery.com>
Cc: <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: Mon, 24 May 2010 22:25:00 -0000	[thread overview]
Message-ID: <94BD67F8AF3ED34FA362C662BA1F12C502BB5F7F@MTVEXCHANGE.mips.com> (raw)
In-Reply-To: <87y6fa9u3t.fsf@firetop.home>

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.
Maybe, we can do the same thing to *16_PCREL* by discarding address in
tc_gen_reloc() and md_pcrel_from(),
such that code looks cleaner and the same.
Thanks!

Regards,
Chao-ying

  reply	other threads:[~2010-05-24 22:25 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 [this message]
2010-05-26 19:47     ` Richard Sandiford
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=94BD67F8AF3ED34FA362C662BA1F12C502BB5F7F@MTVEXCHANGE.mips.com \
    --to=fu@mips.com \
    --cc=binutils@sourceware.org \
    --cc=clm@codesourcery.com \
    --cc=dan@codesourcery.com \
    --cc=ilie@mips.com \
    --cc=joseph@codesourcery.com \
    --cc=macro@codesourcery.com \
    --cc=rdsandiford@googlemail.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).