From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13144 invoked by alias); 26 May 2010 19:47:29 -0000 Received: (qmail 13124 invoked by uid 22791); 26 May 2010 19:47:27 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM X-Spam-Check-By: sourceware.org Received: from mail-wy0-f169.google.com (HELO mail-wy0-f169.google.com) (74.125.82.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 26 May 2010 19:47:21 +0000 Received: by wyb40 with SMTP id 40so249370wyb.0 for ; Wed, 26 May 2010 12:47:18 -0700 (PDT) Received: by 10.227.136.13 with SMTP id p13mr8884999wbt.193.1274903238416; Wed, 26 May 2010 12:47:18 -0700 (PDT) Received: from localhost (rsandifo.gotadsl.co.uk [82.133.89.107]) by mx.google.com with ESMTPS id u32sm2137645wbc.17.2010.05.26.12.47.15 (version=TLSv1/SSLv3 cipher=RC4-MD5); Wed, 26 May 2010 12:47:16 -0700 (PDT) From: Richard Sandiford To: "Fu\, Chao-Ying" Mail-Followup-To: "Fu\, Chao-Ying" ,"Maciej W. Rozycki" , , "Garbacea\, Ilie" , "Joseph Myers" , "Catherine Moore" , , rdsandiford@googlemail.com Cc: "Maciej W. Rozycki" , , "Garbacea\, Ilie" , "Joseph Myers" , "Catherine Moore" , Subject: Re: [PATCH] MIPS: microMIPS and MCU ASE instruction set support References: <87y6fa9u3t.fsf@firetop.home> <94BD67F8AF3ED34FA362C662BA1F12C502BB5F7F@MTVEXCHANGE.mips.com> Date: Wed, 26 May 2010 19:47:00 -0000 In-Reply-To: <94BD67F8AF3ED34FA362C662BA1F12C502BB5F7F@MTVEXCHANGE.mips.com> (Chao-Ying Fu's message of "Mon, 24 May 2010 15:24:13 -0700") Message-ID: <87eigyzbr4.fsf@firetop.home> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2010-05/txt/msg00364.txt.bz2 "Fu, Chao-Ying" 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