Home Original page

mips 64-bit address generation is broken.

Thiemo Seufer ica2_ts@csv.ica.uni-stuttgart.de
Sun Feb 17 20:31:00 GMT 2002 
cgd@broadcom.com wrote:
[snip]
> For this, mips64-elf-gcc -mlong64 -O2 will emit code like:
> 
> 	pointer_test:
> 	        .frame  $sp,40,$31              # vars= 0, regs= 1/0, args= 32, extra= 0
> 	        .mask   0x80000000,-8
> 	        .fmask  0x00000000,0
> 	        ld      $2,q
> 	        ld      $3,p
> 	        dsubu   $sp,$sp,40
> 	        dla $2,131072($2)               # leadi
> 	        sltu    $2,$2,$3
> 		[ ... ]
> 
> (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.

[snip]
> While that's probably the right thing for 'la' in a 32-bit address
> environment, or when 64-bit GPRs aren't available it doesn't seem to
> be the right thing if using 'dla' like the above, even if your object
> formats have only 32-bit addresses.  (otherwise, what's the real diff
> between dla and la?)

The whole (d)la thing is a unfortunate mixture following two different
principles:
  - The "ABI way", which determines the necessary expansion by it's
    definition and ignores/warns about usage which doesn't fit in.
  - The "code way", where a dla is always supposed to use up 64 bit
    and a la will always fit in 32 bit.

[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.
  - 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.

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've not gone through and actually tested a change to do this yet...
> but i'll need to do so soon.
> 
> 
> (*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.


Thiemo


2002-02-18  Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>

	/gas/ChangeLog
	* config/tc-mips.c (macro): Fix la/dla address expansions.


--- 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;
@@ -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
@@ -4707,8 +4731,7 @@ macro (ip)
 		&& ! nopic_need_relax (offset_expr.X_add_symbol, 1))
 	      {
 		frag_grow (20);
-		macro_build ((char *) NULL, &icnt, &offset_expr,
-			     HAVE_32BIT_ADDRESSES ? "addiu" : "daddiu",
+		macro_build ((char *) NULL, &icnt, &offset_expr, "addiu",
 			     "t,r,j", tempreg, GP, (int) BFD_RELOC_GPREL16);
 		p = frag_var (rs_machine_dependent, 8, 0,
 			      RELAX_ENCODE (4, 8, 0, 4, 0,
@@ -4718,8 +4741,7 @@ macro (ip)
 	    macro_build_lui (p, &icnt, &offset_expr, tempreg);
 	    if (p != NULL)
 	      p += 4;
-	    macro_build (p, &icnt, &offset_expr,
-			 HAVE_32BIT_ADDRESSES ? "addiu" : "daddiu",
+	    macro_build (p, &icnt, &offset_expr, "addiu",
 			 "t,r,j", tempreg, tempreg, (int) BFD_RELOC_LO16);
 	  }
 	}
@@ -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
@@ -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 ();
More information about the Binutils mailing list