This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: mips 64-bit address generation is broken.
At Mon, 18 Feb 2002 04:00:35 +0000 (UTC), "Thiemo Seufer" wrote:
> > (actually, that's not quite true: right now it will emit code that
> > does 'la' instead of 'dla' -- but that's broken, right? to load a
> > 64-bit address, it should be using dla, right?
>
> AFAICS it should use 'dla' when using -mlong64.
Right, that was my thought. I've patched gcc to do that.
> [snip]
> > Any thoughts on this?
>
> I see three possibilities:
> - Make la and dla behaving identical, possibly plus some warnings.
> This is like the native IRIX as does, but this won't work for a
> dual 32/64 bit ABI like EABI.
This seems losing, yes. 8-)
> - Make la and dla always behaving different with the possible exception
> of 32 bit GPRS. This will lose compatibility to existing code.
> - Assume embedded mips is the only dual 32/64 bit ABI and expand
> for it according to the code. Do expansion to the ABI's definition
> in all other cases.
So, I _think_ what you want to do is:
(1) if you have 64-bit addresses, make dla and la behave identically.
(2) If you have 32-bit addresses, they need to behave (potentially --
depending on the exact calculation involved) differently,
depending on the addresses involved. Obviously, e.g. loads of
pointers should be done as 32-bit loads, but then the subsequent
operations (e.g. adding offsets) need to be done as 64-bit ops.
> I appended a patch implementing the third case. It passes the binutils
> testsuite for mips{,64}-linux without new regressions, gcc testsuite is
> currently running.
I don't thinks this is correct. Comments in-line.
> > (*sigh* The breakage caused by the HAVE_32BIT_ADDRESSES changes has
> > ... really caused us a fair bit of pain. I can only imagine what
> > it'll do to other random users who only use binutils releases, after
> > 2.12 is released...)
>
> I'm sorry for that, but the distinction between addresses and registers
> is necessary to support n64 ABI. I suspect there are some more cases
> outside the (d)la handling which gets addresses/registers/dbl wrong
> and nobody noticed yet.
Yeah. I don't think it's the wrong direction to move in. But, that
doesn't mean it's been painless. 8-)
> --- source-orig/gas/config/tc-mips.c Tue Feb 12 17:04:35 2002
> +++ source/gas/config/tc-mips.c Mon Feb 18 04:35:00 2002
> @@ -4616,11 +4640,11 @@ macro (ip)
> macro_build ((char *) NULL, &icnt, &offset_expr, "lui", "t,u",
> tempreg, (int) BFD_RELOC_PCREL_HI16_S);
> macro_build ((char *) NULL, &icnt, (expressionS *) NULL,
> - HAVE_32BIT_ADDRESSES ? "addu" : "daddu",
> + dbl ? "addu" : "daddu",
> "d,v,t", tempreg, tempreg, breg);
> }
> macro_build ((char *) NULL, &icnt, &offset_expr,
> - HAVE_32BIT_ADDRESSES ? "addiu" : "daddiu",
> + dbl ? "addiu" : "daddiu",
> "t,r,j", treg, tempreg, (int) BFD_RELOC_PCREL_LO16);
> if (! used_at)
> return;
because of the way the HAVE_32BIT_ADDRESSES macro works, actually,
these (the embedded-pic case) basically evaluate to HAVE_32BIT_GPRS.
The existing (embedded-pic) code here will always evaluate to using
daddu & daddiu if 64-bit GPRs.
Maybe this is the right thing, but it changes the current behaviour.
The test cases won't test it. 8-)
> @@ -4635,7 +4659,7 @@ macro (ip)
> }
>
> if (offset_expr.X_op == O_constant)
> - load_register (&icnt, tempreg, &offset_expr, dbl);
> + load_register (&icnt, tempreg, &offset_expr, HAVE_64BIT_GPRS);
> else if (mips_pic == NO_PIC)
> {
> /* If this is a reference to a GP relative symbol, we want
Hmm. Not sure about this one. If you have 64-bit GPRs and say 'la'
of a constant, do you want the sign-extended address or not?
> @@ -4760,7 +4782,8 @@ macro (ip)
> frag_grow (32);
> if (expr1.X_add_number == 0 && tempreg == PIC_CALL_REG)
> lw_reloc_type = (int) BFD_RELOC_MIPS_CALL16;
> - macro_build ((char *) NULL, &icnt, &offset_expr, dbl ? "ld" : "lw",
> + macro_build ((char *) NULL, &icnt, &offset_expr,
> + HAVE_32BIT_ADDRESSES ? "lw" : "ld",
> "t,o(b)", tempreg, lw_reloc_type, GP);
> if (expr1.X_add_number == 0)
> {
>
> @@ -4915,7 +4938,7 @@ macro (ip)
> HAVE_32BIT_ADDRESSES ? "addu" : "daddu",
> "d,v,t", tempreg, tempreg, GP);
> macro_build ((char *) NULL, &icnt, &offset_expr,
> - dbl ? "ld" : "lw",
> + HAVE_32BIT_ADDRESSES ? "lw" : "ld",
> "t,o(b)", tempreg, lw_reloc_type, tempreg);
> if (expr1.X_add_number == 0)
> {
> @@ -5018,7 +5041,7 @@ macro (ip)
> p += 4;
> }
> macro_build (p, &icnt, &offset_expr,
> - dbl ? "ld" : "lw",
> + HAVE_32BIT_ADDRESSES ? "lw" : "ld",
> "t,o(b)", tempreg, (int) BFD_RELOC_MIPS_GOT16, GP);
> p += 4;
> if (expr1.X_add_number >= -0x8000
These three have nothing to do with the embedded-ABI stuff, right?
> @@ -5072,8 +5095,16 @@ macro (ip)
> addiu $tempreg,$gp,<sym> (BFD_RELOC_GPREL16)
> */
> macro_build ((char *) NULL, &icnt, &offset_expr,
> - HAVE_32BIT_ADDRESSES ? "addiu" : "daddiu",
> + dbl ? "addiu" : "daddiu",
> "t,r,j", tempreg, GP, (int) BFD_RELOC_GPREL16);
> + if (breg != 0)
> + macro_build ((char *) NULL, &icnt, (expressionS *) NULL,
> + dbl ? "addu" : "daddu",
> + "d,v,t", treg, tempreg, breg);
> + if (! used_at)
> + return;
> +
> + break;
> }
> else
> abort ();
hmm. This is not sufficient, I don't think. You also need to do this
for the NO_PIC case. (Again note that for embedded-pic,
HAVE_32BIT_ADDRESS == HAVE_32BIT_GPRs, so for 64-bit embedded-pic, the
existing code will always select daddiu.)
The way I handled this change was to change the code at line 5081:
if (breg != 0)
macro_build ((char *) NULL, &icnt, (expressionS *) NULL,
HAVE_32BIT_ADDRESSES ? "addu" : "daddu",
"d,v,t", treg, tempreg, breg);
to select addu/daddu like:
(dbl || HAVE_64BIT_ADDRESSES) ? "daddu" : "addu",
I.e., this is _not_ just an issue for embedded-pic, it's also an issue
for no-pic.
chris