* [committed] small one liner in gas
@ 2004-10-04 23:25 Eric Christopher
2004-10-05 1:49 ` Daniel Jacobowitz
0 siblings, 1 reply; 11+ messages in thread
From: Eric Christopher @ 2004-10-04 23:25 UTC (permalink / raw)
To: binutils; +Cc: Zack Weinberg
Fixes a small bug Zack ran into with:
lw $3,%half($gp)
where he was getting an internal error in gas for this.
The fx_size was 4 because that's the size of the data worked on, not 2
which is the size in the howto. Anyhow, instead of changing the assert
to 4 I just removed it since none of the other relocations have asserts
in them.
-eric
--
Eric Christopher <echristo@redhat.com>
2004-10-04 Eric Christopher <echristo@redhat.com>
* config/tc-mips.c (md_apply_fix3): Remove erroneous assert.\
Index: config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.270
diff -u -p -w -r1.270 tc-mips.c
--- config/tc-mips.c 18 Aug 2004 15:58:11 -0000 1.270
+++ config/tc-mips.c 4 Oct 2004 23:16:13 -0000
@@ -11004,7 +11004,6 @@ md_apply_fix3 (fixS *fixP, valueT *valP,
case BFD_RELOC_16:
/* If we are deleting this reloc entry, we must fill in the
value now. */
- assert (fixP->fx_size == 2);
if (fixP->fx_done)
md_number_to_chars (buf, *valP, 2);
break;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [committed] small one liner in gas
2004-10-04 23:25 [committed] small one liner in gas Eric Christopher
@ 2004-10-05 1:49 ` Daniel Jacobowitz
2004-10-05 17:28 ` Eric Christopher
2007-10-12 15:59 ` MIPS %half " Daniel Jacobowitz
0 siblings, 2 replies; 11+ messages in thread
From: Daniel Jacobowitz @ 2004-10-05 1:49 UTC (permalink / raw)
To: Eric Christopher; +Cc: binutils, Zack Weinberg
On Mon, Oct 04, 2004 at 04:25:06PM -0700, Eric Christopher wrote:
> Fixes a small bug Zack ran into with:
>
> lw $3,%half($gp)
>
> where he was getting an internal error in gas for this.
>
> The fx_size was 4 because that's the size of the data worked on, not 2
> which is the size in the howto. Anyhow, instead of changing the assert
> to 4 I just removed it since none of the other relocations have asserts
> in them.
Waitasec...
> - assert (fixP->fx_size == 2);
> if (fixP->fx_done)
> md_number_to_chars (buf, *valP, 2);
Won't that get the wrong location for big-endian? The code for LO16
has an endian correction.
[Zack, I think that doesn't matter for us, since the only place we'll
be generating %half fx_done won't get set.]
--
Daniel Jacobowitz
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [committed] small one liner in gas
2004-10-05 1:49 ` Daniel Jacobowitz
@ 2004-10-05 17:28 ` Eric Christopher
2007-10-12 15:59 ` MIPS %half " Daniel Jacobowitz
1 sibling, 0 replies; 11+ messages in thread
From: Eric Christopher @ 2004-10-05 17:28 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: binutils, Zack Weinberg
> Won't that get the wrong location for big-endian? The code for LO16
> has an endian correction.
>
Doesn't look like the code for regular 16 does though. Of course, my ABI
manual is at home so if you can look... :)
-eric
--
Eric Christopher <echristo@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* MIPS %half in gas
2004-10-05 1:49 ` Daniel Jacobowitz
2004-10-05 17:28 ` Eric Christopher
@ 2007-10-12 15:59 ` Daniel Jacobowitz
2007-10-12 16:11 ` Maciej W. Rozycki
` (2 more replies)
1 sibling, 3 replies; 11+ messages in thread
From: Daniel Jacobowitz @ 2007-10-12 15:59 UTC (permalink / raw)
To: binutils
A message from so long ago that almost none of the addresses are still
valid... CC's trimmed :-)
On Mon, Oct 04, 2004 at 09:49:27PM -0400, Daniel Jacobowitz wrote:
> On Mon, Oct 04, 2004 at 04:25:06PM -0700, Eric Christopher wrote:
> > Fixes a small bug Zack ran into with:
> >
> > lw $3,%half($gp)
> >
> > where he was getting an internal error in gas for this.
> >
> > The fx_size was 4 because that's the size of the data worked on, not 2
> > which is the size in the howto. Anyhow, instead of changing the assert
> > to 4 I just removed it since none of the other relocations have asserts
> > in them.
>
> Waitasec...
>
> > - assert (fixP->fx_size == 2);
> > if (fixP->fx_done)
> > md_number_to_chars (buf, *valP, 2);
>
> Won't that get the wrong location for big-endian? The code for LO16
> has an endian correction.
I was right. Removing the assert was wrong, and the code has since
changed to get even wronger. Assemble this for MIPS big-endian or
little-endian:
lw $3,%half(x)($gp)
.set x, 0x1234
Either way you get:
00000000 <.text>:
0: 00001234 0x1234
If x is defined in another file, so that the linker has to do the
fixup, the right thing happens.
It got wronger here:
2006-08-01 Thiemo Seufer <ths@mips.com>
* config/tc-mips.c (macro_build_lui): Fix comment formatting.
(md_apply_fix): Likewise. Unify handling of BFD_RELOC_RVA,
BFD_RELOC_32 and BFD_RELOC_16.
(s_align, s_cpload, s_cplocal, s_cprestore, s_mips_stab,
md_convert_frag, md_obj_end): Fix comment formatting.
It looked as if fx_size was always 2 for BFD_RELOC_16, but that's not
true; it's always 4.
I came up with the attached patch and testcase. Does this look OK?
I've only tested it as below plus some manual verification of the
error messages; I don't have any use for %half at the moment, just
didn't want to leave it broken.
--
Daniel Jacobowitz
CodeSourcery
2007-10-12 Daniel Jacobowitz <dan@codesourcery.com>
* config/tc-mips.c (append_insn): Handle BFD_RELOC_16.
(md_apply_fix): Likewise.
2007-10-12 Daniel Jacobowitz <dan@codesourcery.com>
* gas/mips/half.d, gas/mips/half.s: New.
* gas/mips/mips.exp: Run %half test.
Index: config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.376
diff -u -p -r1.376 tc-mips.c
--- config/tc-mips.c 8 Oct 2007 16:09:34 -0000 1.376
+++ config/tc-mips.c 12 Oct 2007 15:41:19 -0000
@@ -2742,6 +2742,15 @@ append_insn (struct mips_cl_insn *ip, ex
ip->insn_opcode |= (address_expr->X_add_number >> 16) & 0xffff;
break;
+ case BFD_RELOC_16:
+ if (address_expr->X_add_number & ~0xffffull)
+ {
+ char value [32];
+
+ sprintf_vma (value, offset_expr.X_add_number);
+ as_bad (_("Number (0x%s) larger than 16 bits"), value);
+ }
+ /* FALLTHROUGH */
case BFD_RELOC_UNUSED:
case BFD_RELOC_LO16:
case BFD_RELOC_MIPS_GOT_DISP:
@@ -11925,7 +11934,6 @@ md_apply_fix (fixS *fixP, valueT *valP,
case BFD_RELOC_RVA:
case BFD_RELOC_32:
- case BFD_RELOC_16:
/* If we are deleting this reloc entry, we must fill in the
value now. This can happen if we have a .word which is not
resolved when it appears but is later defined. */
@@ -11933,6 +11941,27 @@ md_apply_fix (fixS *fixP, valueT *valP,
md_number_to_chars ((char *) buf, *valP, fixP->fx_size);
break;
+ case BFD_RELOC_16:
+ /* If we are deleting this reloc entry, we must fill in the
+ value now. This can happen if we have a .word which is not
+ resolved when it appears but is later defined. */
+ if (fixP->fx_done)
+ {
+ if (*valP & ~0xffffull)
+ {
+ char value [32];
+
+ sprintf_vma (value, *valP);
+ as_bad_where (fixP->fx_file, fixP->fx_line,
+ _("Number (0x%s) larger than 16 bits"), value);
+ }
+
+ if (target_big_endian)
+ buf += 2;
+ md_number_to_chars ((char *) buf, *valP, 2);
+ }
+ break;
+
case BFD_RELOC_LO16:
case BFD_RELOC_MIPS16_LO16:
/* FIXME: Now that embedded-PIC is gone, some of this code/comment
Index: testsuite/gas/mips/half.d
===================================================================
RCS file: testsuite/gas/mips/half.d
diff -N testsuite/gas/mips/half.d
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gas/mips/half.d 12 Oct 2007 15:41:20 -0000
@@ -0,0 +1,12 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS %half relocations
+#as: -32 -EB
+
+.*: +file format .*mips.*
+
+Disassembly of section .text:
+0+0000 <[^>]*> 8f831234 lw v1,4660\(gp\)
+0+0004 <[^>]*> 8f830000 lw v1,0\(gp\)
+ 4: R_MIPS_16 y
+0+0008 <[^>]*> 8f831234 lw v1,4660\(gp\)
+#pass
Index: testsuite/gas/mips/half.s
===================================================================
RCS file: testsuite/gas/mips/half.s
diff -N testsuite/gas/mips/half.s
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gas/mips/half.s 12 Oct 2007 15:41:20 -0000
@@ -0,0 +1,6 @@
+ .text
+ .set x, 0x1234
+ lw $3,%half(x)($gp)
+ lw $3,%half(y)($gp)
+ lw $3,%half(z)($gp)
+ .set z, 0x1234
Index: testsuite/gas/mips/mips.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips.exp,v
retrieving revision 1.132
diff -u -p -r1.132 mips.exp
--- testsuite/gas/mips/mips.exp 8 Oct 2007 16:41:34 -0000 1.132
+++ testsuite/gas/mips/mips.exp 12 Oct 2007 15:41:20 -0000
@@ -677,6 +677,8 @@ if { [istarget mips*-*-vxworks*] } {
run_list_test "tls-ill" "-32"
run_dump_test "tls-o32"
+
+ run_dump_test "half"
}
if $has_newabi {
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: MIPS %half in gas
2007-10-12 15:59 ` MIPS %half " Daniel Jacobowitz
@ 2007-10-12 16:11 ` Maciej W. Rozycki
2007-10-12 16:49 ` Daniel Jacobowitz
2007-10-12 17:48 ` Richard Sandiford
2007-10-12 17:58 ` Thiemo Seufer
2 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2007-10-12 16:11 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: binutils
On Fri, 12 Oct 2007, Daniel Jacobowitz wrote:
> A message from so long ago that almost none of the addresses are still
> valid... CC's trimmed :-)
Hmm, I find it nice to discover yet another MIPS relocation type/operator
I was not aware of. ;-) I can hardly see a use for this operator and I
have no idea as to where it is coming from, but your change looks
reasonable to me.
Though the test case may be confusing -- given the relocation operates on
an unsigned value, the use of "lw" in the test case is misleading. I
think "ori", "andi" or "xori" would be better. Although of these only
"ori" looks remotely useful and I am not sure if using this relocation on
code rather than data (".hword %half(foo)") makes sense at all.
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: MIPS %half in gas
2007-10-12 16:11 ` Maciej W. Rozycki
@ 2007-10-12 16:49 ` Daniel Jacobowitz
2007-10-12 17:48 ` Maciej W. Rozycki
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2007-10-12 16:49 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: binutils
On Fri, Oct 12, 2007 at 05:06:38PM +0100, Maciej W. Rozycki wrote:
> Hmm, I find it nice to discover yet another MIPS relocation type/operator
> I was not aware of. ;-) I can hardly see a use for this operator and I
> have no idea as to where it is coming from, but your change looks
> reasonable to me.
I've got no idea where it comes from either.
> Though the test case may be confusing -- given the relocation operates on
> an unsigned value, the use of "lw" in the test case is misleading. I
> think "ori", "andi" or "xori" would be better. Although of these only
> "ori" looks remotely useful and I am not sure if using this relocation on
> code rather than data (".hword %half(foo)") makes sense at all.
This is where my lack of knowing what things are for comes into play.
The example in Eric's original mail from long ago uses a gp-relative
address. GCC uses it for exactly that purpose, in a VxWorks PIC
model:
(define_split
[(unspec_volatile [(match_operand:P 0 "symbol_ref_operand")
(match_operand:P 1 "symbol_ref_operand")]
UNSPEC_LOADGP)]
"mips_current_loadgp_style () == LOADGP_RTP"
[(set (match_dup 2) (high:P (match_dup 3)))
(set (match_dup 2) (unspec:P [(match_dup 2)
(match_dup 3)] UNSPEC_LOAD_GOT))
(set (match_dup 2) (unspec:P [(match_dup 2)
(match_dup 4)] UNSPEC_LOAD_GOT))]
{
operands[2] = pic_offset_table_rtx;
operands[3] = mips_unspec_address (operands[0], SYMBOL_ABSOLUTE);
operands[4] = mips_unspec_address (operands[1], SYMBOL_HALF);
})
That expands to something like %half(__GOTT_INDEX__)($gp) which is a
magic symbol defined by the runtime to a small value, so the
signedness does not much matter.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: MIPS %half in gas
2007-10-12 15:59 ` MIPS %half " Daniel Jacobowitz
2007-10-12 16:11 ` Maciej W. Rozycki
@ 2007-10-12 17:48 ` Richard Sandiford
2007-10-12 18:08 ` Thiemo Seufer
2007-10-15 12:49 ` Maciej W. Rozycki
2007-10-12 17:58 ` Thiemo Seufer
2 siblings, 2 replies; 11+ messages in thread
From: Richard Sandiford @ 2007-10-12 17:48 UTC (permalink / raw)
To: binutils
Daniel Jacobowitz <drow@false.org> writes:
> + case BFD_RELOC_16:
> + /* If we are deleting this reloc entry, we must fill in the
> + value now. This can happen if we have a .word which is not
> + resolved when it appears but is later defined. */
Minor nit, but this comment is for BFD_RELOC_16 in isolation, which can
never come from a .word directive. I think you should just generalise
".word" to "instruction", or something like that.
> + if (fixP->fx_done)
> + {
> + if (*valP & ~0xffffull)
> + {
> + char value [32];
> +
> + sprintf_vma (value, *valP);
> + as_bad_where (fixP->fx_file, fixP->fx_line,
> + _("Number (0x%s) larger than 16 bits"), value);
> + }
> +
> + if (target_big_endian)
> + buf += 2;
> + md_number_to_chars ((char *) buf, *valP, 2);
> + }
> + break;
> +
Regarding the discussion downthread, the ABI specifically says that an
R_MIPS_16 addend is signed rather than an unsigned, so (a) it really should
be OK to use %half for ADDIU and LW, and (b), the *valP check above probably
needs to check a signed range rather than an unsigned one, or at least
allow both.
Richard
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: MIPS %half in gas
2007-10-12 16:49 ` Daniel Jacobowitz
@ 2007-10-12 17:48 ` Maciej W. Rozycki
0 siblings, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2007-10-12 17:48 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: binutils
On Fri, 12 Oct 2007, Daniel Jacobowitz wrote:
> > Though the test case may be confusing -- given the relocation operates on
> > an unsigned value, the use of "lw" in the test case is misleading. I
> > think "ori", "andi" or "xori" would be better. Although of these only
> > "ori" looks remotely useful and I am not sure if using this relocation on
> > code rather than data (".hword %half(foo)") makes sense at all.
>
> This is where my lack of knowing what things are for comes into play.
Me too, just trying to apply some common sense which may not always be
justified. ;-)
> That expands to something like %half(__GOTT_INDEX__)($gp) which is a
> magic symbol defined by the runtime to a small value, so the
> signedness does not much matter.
Well, such an abuse may work and may even be necessary for someone, but
it does not mean we should actively promote it, does it? ;-)
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: MIPS %half in gas
2007-10-12 15:59 ` MIPS %half " Daniel Jacobowitz
2007-10-12 16:11 ` Maciej W. Rozycki
2007-10-12 17:48 ` Richard Sandiford
@ 2007-10-12 17:58 ` Thiemo Seufer
2 siblings, 0 replies; 11+ messages in thread
From: Thiemo Seufer @ 2007-10-12 17:58 UTC (permalink / raw)
To: binutils
Daniel Jacobowitz wrote:
[snip]
> 2007-10-12 Daniel Jacobowitz <dan@codesourcery.com>
>
> * config/tc-mips.c (append_insn): Handle BFD_RELOC_16.
> (md_apply_fix): Likewise.
>
> 2007-10-12 Daniel Jacobowitz <dan@codesourcery.com>
>
> * gas/mips/half.d, gas/mips/half.s: New.
> * gas/mips/mips.exp: Run %half test.
>
> Index: config/tc-mips.c
> ===================================================================
> RCS file: /cvs/src/src/gas/config/tc-mips.c,v
> retrieving revision 1.376
> diff -u -p -r1.376 tc-mips.c
> --- config/tc-mips.c 8 Oct 2007 16:09:34 -0000 1.376
> +++ config/tc-mips.c 12 Oct 2007 15:41:19 -0000
> @@ -2742,6 +2742,15 @@ append_insn (struct mips_cl_insn *ip, ex
> ip->insn_opcode |= (address_expr->X_add_number >> 16) & 0xffff;
> break;
>
> + case BFD_RELOC_16:
> + if (address_expr->X_add_number & ~0xffffull)
> + {
> + char value [32];
> +
> + sprintf_vma (value, offset_expr.X_add_number);
> + as_bad (_("Number (0x%s) larger than 16 bits"), value);
> + }
> + /* FALLTHROUGH */
> case BFD_RELOC_UNUSED:
> case BFD_RELOC_LO16:
> case BFD_RELOC_MIPS_GOT_DISP:
> @@ -11925,7 +11934,6 @@ md_apply_fix (fixS *fixP, valueT *valP,
>
> case BFD_RELOC_RVA:
> case BFD_RELOC_32:
> - case BFD_RELOC_16:
> /* If we are deleting this reloc entry, we must fill in the
> value now. This can happen if we have a .word which is not
> resolved when it appears but is later defined. */
> @@ -11933,6 +11941,27 @@ md_apply_fix (fixS *fixP, valueT *valP,
> md_number_to_chars ((char *) buf, *valP, fixP->fx_size);
> break;
>
> + case BFD_RELOC_16:
> + /* If we are deleting this reloc entry, we must fill in the
> + value now. This can happen if we have a .word which is not
That's .hword or .short then, I think. Otherwise ok, thanks for
fixing this. :-)
Thiemo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: MIPS %half in gas
2007-10-12 17:48 ` Richard Sandiford
@ 2007-10-12 18:08 ` Thiemo Seufer
2007-10-15 12:49 ` Maciej W. Rozycki
1 sibling, 0 replies; 11+ messages in thread
From: Thiemo Seufer @ 2007-10-12 18:08 UTC (permalink / raw)
To: binutils, rsandifo
Richard Sandiford wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> > + case BFD_RELOC_16:
> > + /* If we are deleting this reloc entry, we must fill in the
> > + value now. This can happen if we have a .word which is not
> > + resolved when it appears but is later defined. */
>
> Minor nit, but this comment is for BFD_RELOC_16 in isolation, which can
> never come from a .word directive. I think you should just generalise
> ".word" to "instruction", or something like that.
>
> > + if (fixP->fx_done)
> > + {
> > + if (*valP & ~0xffffull)
> > + {
> > + char value [32];
> > +
> > + sprintf_vma (value, *valP);
> > + as_bad_where (fixP->fx_file, fixP->fx_line,
> > + _("Number (0x%s) larger than 16 bits"), value);
> > + }
> > +
> > + if (target_big_endian)
> > + buf += 2;
> > + md_number_to_chars ((char *) buf, *valP, 2);
> > + }
> > + break;
> > +
>
> Regarding the discussion downthread, the ABI specifically says that an
> R_MIPS_16 addend is signed rather than an unsigned, so (a) it really should
> be OK to use %half for ADDIU and LW, and (b), the *valP check above probably
> needs to check a signed range rather than an unsigned one, or at least
> allow both.
Indeed, I missed the signedness here.
Thiemo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: MIPS %half in gas
2007-10-12 17:48 ` Richard Sandiford
2007-10-12 18:08 ` Thiemo Seufer
@ 2007-10-15 12:49 ` Maciej W. Rozycki
1 sibling, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2007-10-15 12:49 UTC (permalink / raw)
To: Richard Sandiford; +Cc: binutils
On Fri, 12 Oct 2007, Richard Sandiford wrote:
> > + if (fixP->fx_done)
> > + {
> > + if (*valP & ~0xffffull)
> > + {
> > + char value [32];
> > +
> > + sprintf_vma (value, *valP);
> > + as_bad_where (fixP->fx_file, fixP->fx_line,
> > + _("Number (0x%s) larger than 16 bits"), value);
> > + }
> > +
> > + if (target_big_endian)
> > + buf += 2;
> > + md_number_to_chars ((char *) buf, *valP, 2);
> > + }
> > + break;
> > +
>
> Regarding the discussion downthread, the ABI specifically says that an
> R_MIPS_16 addend is signed rather than an unsigned, so (a) it really should
> be OK to use %half for ADDIU and LW, and (b), the *valP check above probably
> needs to check a signed range rather than an unsigned one, or at least
> allow both.
Fair enough. I only complained about the way of use in the test case,
because the code above clearly treats the addend as unsigned. I have had
a look at the ABI document now and it documents a half16 relocatable field
together with its associated R_MIPS_16 relocation like you say. Although
no indication there as to what it would be useful for.
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2007-10-15 12:40 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-10-04 23:25 [committed] small one liner in gas Eric Christopher
2004-10-05 1:49 ` Daniel Jacobowitz
2004-10-05 17:28 ` Eric Christopher
2007-10-12 15:59 ` MIPS %half " Daniel Jacobowitz
2007-10-12 16:11 ` Maciej W. Rozycki
2007-10-12 16:49 ` Daniel Jacobowitz
2007-10-12 17:48 ` Maciej W. Rozycki
2007-10-12 17:48 ` Richard Sandiford
2007-10-12 18:08 ` Thiemo Seufer
2007-10-15 12:49 ` Maciej W. Rozycki
2007-10-12 17:58 ` Thiemo Seufer
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).