Re: [rfa] mips argument passing fixes for o32

Re: [rfa] mips argument passing fixes for o32

[David B Anderson]

Andrew Cagney:
|(A+3, yes, sorry).
|
|The SGI compilers are big endian (correct?) so who knows what they would 
|do in the little endian case.


Correct. Years ago it was possible to emit little-endian
code, but on MIPS/IRIX that was irrelevant and is no longer 
supported.
davea@sgi.com

Re: [rfa] mips argument passing fixes for o32

[Andrew Cagney]

> Andrew Cagney:
> |(A+3, yes, sorry).
> |
> |The SGI compilers are big endian (correct?) so who knows what they would 
> |do in the little endian case.
> 
> 
> Correct. Years ago it was possible to emit little-endian
> code, but on MIPS/IRIX that was irrelevant and is no longer 
> supported.
> davea@sgi.com


You wouldn't have access to such a machine by any chance? :-)

I'm getting the feeling I'm wrong with this one - for some strange 
historical reason LE o32 really does left/right shift small parameters 
(I'm still mining the archives).

Assuming that is the case ...

>  if (!MIPS_EABI
>  		      && MIPS_SAVED_REGSIZE < 8
> -		      && TARGET_BYTE_ORDER == BIG_ENDIAN
> +		      && (TARGET_BYTE_ORDER == BIG_ENDIAN
> +			  || TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE)
>  		      && partial_len < MIPS_SAVED_REGSIZE
>  		      && (typecode == TYPE_CODE_STRUCT ||
>  			  typecode == TYPE_CODE_UNION))

I don't know that line being added should be

	

|| TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE


it should at least be guarded by ``ABI == o32''.  What does LE n32 do 
for instance?
If GCC, for o32, always left shifts the structs dregs (PARTIAL_LEN < 
MIPS_SAVED_REGSIZE) then is just the ABI test needed?

	Andrew

PS: That function is the official example of how to _not_ multi-arch an ABI.

Re: [rfa] mips argument passing fixes for o32

[Daniel Jacobowitz]

On Sun, Jul 15, 2001 at 05:50:16PM -0400, Andrew Cagney wrote:
> You wouldn't have access to such a machine by any chance? :-)
> 
> I'm getting the feeling I'm wrong with this one - for some strange 
> historical reason LE o32 really does left/right shift small parameters 
> (I'm still mining the archives).
> 
> Assuming that is the case ...
> 
> >  if (!MIPS_EABI
> >  		      && MIPS_SAVED_REGSIZE < 8
> > -		      && TARGET_BYTE_ORDER == BIG_ENDIAN
> > +		      && (TARGET_BYTE_ORDER == BIG_ENDIAN
> > +			  || TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE)
> >  		      && partial_len < MIPS_SAVED_REGSIZE
> >  		      && (typecode == TYPE_CODE_STRUCT ||
> >  			  typecode == TYPE_CODE_UNION))
> 
> I don't know that line being added should be
> 
> 	
> 
> || TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE
> 
> 
> it should at least be guarded by ``ABI == o32''.  What does LE n32 do 
> for instance?
> If GCC, for o32, always left shifts the structs dregs (PARTIAL_LEN < 
> MIPS_SAVED_REGSIZE) then is just the ABI test needed?

That's not what it does, though.  It only left-shifts the dregs if they
constitute the entire structure; thus the check I used.  Little-endian
will left-shift only small structs, while big-endian will left-shift
small structs or leftovers.

As for when it is done, GCC says:
      if (struct_p && int_size_in_bytes (type) < UNITS_PER_WORD
          && ! TARGET_64BIT && mips_abi != ABI_EABI)

However, there's a comment to the effect that the !TARGET_64BIT is
actually wrong, and being used only for efficiency.  So I'm not sure
precisely when this applies.  It seems that !MIPS_EABI and
MIPS_SAVED_REGSIZE < 8 would agree with at least the way that GCC does
things, but GCC is not a sterling example of n32 or n64 compatibility.

> PS: That function is the official example of how to _not_ multi-arch an ABI.

And does it ever show...

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] mips argument passing fixes for o32

[Andrew Cagney]

> That's not what it does, though.  It only left-shifts the dregs if they
> constitute the entire structure; thus the check I used.  Little-endian
> will left-shift only small structs, while big-endian will left-shift
> small structs or leftovers.


I which case you definitly need the o32 check.  DEC's compiler passes 
small structs in the LE part of the register.


> As for when it is done, GCC says:
>       if (struct_p && int_size_in_bytes (type) < UNITS_PER_WORD
>           && ! TARGET_64BIT && mips_abi != ABI_EABI)
> 
> However, there's a comment to the effect that the !TARGET_64BIT is
> actually wrong, and being used only for efficiency.  So I'm not sure
> precisely when this applies.  It seems that !MIPS_EABI and
> MIPS_SAVED_REGSIZE < 8 would agree with at least the way that GCC does
> things, but GCC is not a sterling example of n32 or n64 compatibility.


still digging,
	Andrew

Re: [rfa] mips argument passing fixes for o32

[David B Anderson]

Andrew Cagney:
|> Andrew Cagney:
|> |(A+3, yes, sorry).
|> |
|> |The SGI compilers are big endian (correct?) so who knows what they would 
|> |do in the little endian case.
|> 
|> 
|> Correct. Years ago it was possible to emit little-endian
|> code, but on MIPS/IRIX that was irrelevant and is no longer 
|> supported.
|> davea@sgi.com
|
|
|You wouldn't have access to such a machine by any chance? :-)
|
|I'm getting the feeling I'm wrong with this one - for some strange 
|historical reason LE o32 really does left/right shift small parameters 
|(I'm still mining the archives).
|
|Assuming that is the case ...

Oh boy.  I have to admit I ignored -EL (which 032 (cc -32)
still admits is a real option) back when it was
sort of current.  Big endian bigot :-) Sorry.

Today, the -EL sort of still works, o32, and generates 2LSB elf.
But it's not 'supported' so supported is here a weasel-word.

We don't test it.

But anyone with an IRIX cc -32 can look at the generated code.

Unfortunately, the MIPS disassembler (recent)
kind of barfs on little endian objects (I said we don't test -EL)...

davea

|>  if (!MIPS_EABI
|>  		      && MIPS_SAVED_REGSIZE < 8
|> -		      && TARGET_BYTE_ORDER == BIG_ENDIAN
|> +		      && (TARGET_BYTE_ORDER == BIG_ENDIAN
|> +			  || TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE)
|>  		      && partial_len < MIPS_SAVED_REGSIZE
|>  		      && (typecode == TYPE_CODE_STRUCT ||
|>  			  typecode == TYPE_CODE_UNION))
|
|I don't know that line being added should be


||| TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE
|
|
|it should at least be guarded by ``ABI == o32''.  What does LE n32 do 
|for instance?
|If GCC, for o32, always left shifts the structs dregs (PARTIAL_LEN < 
|MIPS_SAVED_REGSIZE) then is just the ABI test needed?
|
|	Andrew
|
|PS: That function is the official example of how to _not_ multi-arch an ABI.

And now I confess I don't quite understand the question.
Ah. mips_push_arguments().
The question is whether to shift the contents?

It's likely I don't really grasp the subtleties of the question
here at the moment.
struct mys {
        int a;
        char b;
};

as a struct argument, with cc -32 -EL, does not seem to do more than
load b into the second arg reg (r5) and a into r4  if one does
	struct mys m
	myfunc(m);
But I don't feel confident that answers anything, really.

I (and anyone with IRIX cc -32) can do -EL -32.
(With -n32 and -64,  -EL is ignored by IRIX cc.  )
If you had a sample you wondered about I could certainly
compile it -EL, send assembler to anyone. 
If it would help. As I said,
any IRIX cc -32 still sort of, unoficially, does try to honor -EL.

Hmm. Well hope this helps a little....
davea@sgi.com

Re: [rfa] mips argument passing fixes for o32

[Andrew Cagney]

> |> if (!MIPS_EABI
> |> && MIPS_SAVED_REGSIZE < 8
> |> -		      && TARGET_BYTE_ORDER == BIG_ENDIAN
> |> +		      && (TARGET_BYTE_ORDER == BIG_ENDIAN
> |> +			  || TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE)
> |> && partial_len < MIPS_SAVED_REGSIZE
> |> && (typecode == TYPE_CODE_STRUCT ||
> |> typecode == TYPE_CODE_UNION))
> |
> |I don't know that line being added should be
> 
> 
> ||| TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE
> |
> |
> |it should at least be guarded by ``ABI == o32''.  What does LE n32 do 
> |for instance?
> |If GCC, for o32, always left shifts the structs dregs (PARTIAL_LEN < |MIPS_SAVED_REGSIZE) then is just the ABI test needed?
> |
> |	Andrew
> |
> |PS: That function is the official example of how to _not_ multi-arch an ABI.
> 
> And now I confess I don't quite understand the question.
> Ah. mips_push_arguments().
> The question is whether to shift the contents?
> 
> It's likely I don't really grasp the subtleties of the question
> here at the moment.
> struct mys {
>         int a;
>         char b;
> };
> 
> as a struct argument, with cc -32 -EL, does not seem to do more than
> load b into the second arg reg (r5) and a into r4  if one does
> 	struct mys m
> 	myfunc(m);
> But I don't feel confident that answers anything, really.


The debate os over a possible exception to this rule.  Given a very 
small structure and the code segment:

	struct c1 { char c; } s1;
	foo (struct c1);
	...
	foo (s1);

GCC is always left shifting s1 in a register (regardless of -EL or EB).


> I (and anyone with IRIX cc -32) can do -EL -32.
> (With -n32 and -64,  -EL is ignored by IRIX cc.  )
> If you had a sample you wondered about I could certainly
> compile it -EL, send assembler to anyone. 
> If it would help. As I said,
> any IRIX cc -32 still sort of, unoficially, does try to honor -EL.


gdb/testsuite/gdb.base/structs.c is pretty good but just the above 
segment is :-)

	Andrew

Re: [rfa] mips argument passing fixes for o32

[David B Anderson]

>
>The debate os over a possible exception to this rule.  Given a very 
>small structure and the code segment:
>
>        struct c1 { char c; } s1;
>        foo (struct c1);
>        ...
>        foo (s1);
>
>GCC is always left shifting s1 in a register (regardless of -EL or EB).


Well.... 
proton 25% cc -c -32 t3.c -show
/usr/lib/cc -DEFAULT:abi=n32:isa=mips4:proc=r10k -c -32 t3.c -show
/usr/lib/cfe -D_MIPS_FPSET=16 -D_MIPS_ISA=2 -D_ABIO32=1 -D_MIPS_SIM=_ABIO32 -D_MIPS_SZINT=32 -D_MIPS_SZLONG=32 -D_MIPS_SZPTR=32 -D_SIZE_INT=32 -D_SIZE_LONG=32 -D_SIZE_PTR=32 -D__EXTENSIONS__ -DLANGUAGE_C -D_LANGUAGE_C -D__INLINE_INTRINSICS -Dsgi -D__sgi -Dunix -Dmips -Dhost_mips -D__unix -D__host_mips -D_SVR4_SOURCE -D_MODERN_C -D_SGI_SOURCE -D_PIC -D__DSO__ -DSYSTYPE_SVR4 -D_SYSTYPE_SVR4 -D_LONGLONG -D__mips=2 -I -D_MIPSEB -DMIPSEB -D__STDC__=1 -I/usr/include t3.c -Xv -D_CFE -Amachine(mips) -Asystem(unix) -call_shared -G 0 -std -XS/tmp/ctmsta000Ss -mips2 -EB -Xg0 -O1 > /tmp/ctmfa000Ss 
cfe: main /usr/lib/ugen -v -G 0 -pic2 -mips2 -EB -g0 -O1 /tmp/ctmfa000Ss -o /tmp/ctmca000Ss -t /tmp/ctmsta000Ss -temp /tmp/ctmgta000Ss 
ugen: main
/usr/lib/as1 -t5_ll_sc_bug -elf -pic2 -v -G 0 -p0 -mips2 -EB -g0 -O1 /tmp/ctmca000Ss -o t3.o -t /tmp/ctmsta000Ss 
as1: main 
proton 26% dis t3.o
                ****   DISASSEMBLER  ****


disassembly for t3.o

section .text
main:
[   7] 0x   0:  3c 1c 00 00    lui gp,0
[   7] 0x   4:  27 9c 00 00    addiu gp,gp,0
[   7] 0x   8:  03 99 e0 21    addu gp,gp,t9
[   8] 0x   c:  8f 8e 00 00    lw t2,0(gp)
[   8] 0x  10:  8f 99 00 00    lw t9,0(gp)
[   7] 0x  14:  27 bd ff e0    addiu sp,sp,-32
[   8] 0x  18:  91 ce 00 00    lbu t2,0(t2)
[   7] 0x  1c:  af bf 00 1c    sw ra,28(sp)
[   7] 0x  20:  af bc 00 18    sw gp,24(sp)
[   8] 0x  24:  a3 ae 00 00    sb t2,0(sp)
[   8] 0x  28:  03 20 f8 09    jalr t9
[   8] 0x  2c:  8f a4 00 00    lw a0,0(sp)
[   9] 0x  30:  8f bf 00 1c    lw ra,28(sp)
[   8] 0x  34:  8f bc 00 18    lw gp,24(sp)
[   9] 0x  38:  27 bd 00 20    addiu sp,sp,32
[   9] 0x  3c:  03 e0 00 08    jr ra
[   9] 0x  40:  00 00 10 25    move v0,zero
[   9] 0x  44:  00 00 00 00    nop
[   9] 0x  48:  00 00 00 00    nop
[   9] 0x  4c:  00 00 00 00    nop
proton 27% cat t3.c

struct c1 { char c; } s1;

int foo (struct c1);

int main()
{
        foo (s1);
        return 0;
}

And -EL seems to generate the same instructions (but I don't quite
trust -EL).

Regards,
David B. Anderson davea@sgi.com danderson@acm.org 
< http://reality.sgi.com/davea > 
(reality.sgi.com to be permanently turned off --  around Aug 15, 2001)

Re: [rfa] mips argument passing fixes for o32

[Andrew Cagney]

>>GCC is always left shifting s1 in a register (regardless of -EL or EB).
> 
> 
> 
> Well.... 
> proton 25% cc -c -32 t3.c -show
> /usr/lib/cc -DEFAULT:abi=n32:isa=mips4:proc=r10k -c -32 t3.c -show
> /usr/lib/cfe -D_MIPS_FPSET=16 -D_MIPS_ISA=2 -D_ABIO32=1 -D_MIPS_SIM=_ABIO32 -D_MIPS_SZINT=32 -D_MIPS_SZLONG=32 -D_MIPS_SZPTR=32 -D_SIZE_INT=32 -D_SIZE_LONG=32 -D_SIZE_PTR=32 -D__EXTENSIONS__ -DLANGUAGE_C -D_LANGUAGE_C -D__INLINE_INTRINSICS -Dsgi -D__sgi -Dunix -Dmips -Dhost_mips -D__unix -D__host_mips -D_SVR4_SOURCE -D_MODERN_C -D_SGI_SOURCE -D_PIC -D__DSO__ -DSYSTYPE_SVR4 -D_SYSTYPE_SVR4 -D_LONGLONG -D__mips=2 -I -D_MIPSEB -DMIPSEB -D__STDC__=1 -I/usr/include t3.c -Xv -D_CFE -Amachine(mips) -Asystem(unix) -call_shared -G 0 -std -XS/tmp/ctmsta000Ss -mips2 -EB -Xg0 -O1 > /tmp/ctmfa000Ss 
> cfe: main /usr/lib/ugen -v -G 0 -pic2 -mips2 -EB -g0 -O1 /tmp/ctmfa000Ss -o /tmp/ctmca000Ss -t /tmp/ctmsta000Ss -temp /tmp/ctmgta000Ss 
> ugen: main
> /usr/lib/as1 -t5_ll_sc_bug -elf -pic2 -v -G 0 -p0 -mips2 -EB -g0 -O1 /tmp/ctmca000Ss -o t3.o -t /tmp/ctmsta000Ss 
> as1: main 
> proton 26% dis t3.o

> [   7] 0x   0:  3c 1c 00 00    lui gp,0
> [   7] 0x   4:  27 9c 00 00    addiu gp,gp,0
> [   7] 0x   8:  03 99 e0 21    addu gp,gp,t9
> [   8] 0x   c:  8f 8e 00 00    lw t2,0(gp)
> [   8] 0x  10:  8f 99 00 00    lw t9,0(gp)
> [   7] 0x  14:  27 bd ff e0    addiu sp,sp,-32
> [   8] 0x  18:  91 ce 00 00    lbu t2,0(t2)
> [   7] 0x  1c:  af bf 00 1c    sw ra,28(sp)
> [   7] 0x  20:  af bc 00 18    sw gp,24(sp)
> [   8] 0x  24:  a3 ae 00 00    sb t2,0(sp)
> [   8] 0x  28:  03 20 f8 09    jalr t9
> [   8] 0x  2c:  8f a4 00 00    lw a0,0(sp)

> 
> And -EL seems to generate the same instructions (but I don't quite
> trust -EL).


Now I'm really confused, that is -EB -o32 and no shift.  I think I'll try to scrounge up an IRIX box and have a play.


	Andrew

[rfa] mips argument passing fixes for o32

[Daniel Jacobowitz]

These are based on testsuite failures (call-*-st, if I remember correctly)
and reading the argument passing code in GCC.  The struct alignment fix
definitely agrees with the ABI, though it's not always clear on this point -
it becomes necessary for us when the return value is a struct and thus there
is a hidden pointer as the first argument.  The shift fix matches this
comment in GCC and is not really specified by the ABI document:

      /* The following is a hack in order to pass 1 byte structures
         the same way that the MIPS compiler does (namely by passing
         the structure in the high byte or half word of the register).
         This also makes varargs work.  If we have such a structure,
         we save the adjustment RTL, and the call define expands will
         emit them.  For the VOIDmode argument (argument after the
         last real argument), pass back a parallel vector holding each
         of the adjustments.  */

      /* ??? function_arg can be called more than once for each argument.
         As a result, we compute more adjustments than we need here.
         See the CUMULATIVE_ARGS definition in mips.h.  */

      /* ??? This scheme requires everything smaller than the word size to
         shifted to the left, but when TARGET_64BIT and ! TARGET_INT64,
         that would mean every int needs to be shifted left, which is very
         inefficient.  Let's not carry this compatibility to the 64 bit
         calling convention for now.  */

      if (struct_p && int_size_in_bytes (type) < UNITS_PER_WORD
          && ! TARGET_64BIT && mips_abi != ABI_EABI)
        { 
          rtx amount = GEN_INT (BITS_PER_WORD
                                - int_size_in_bytes (type) * BITS_PER_UNIT);
          rtx reg = gen_rtx_REG (word_mode, regbase + *arg_words + bias);

          if (TARGET_64BIT)
            cum->adjust[cum->num_adjusts++] = gen_ashldi3 (reg, reg, amount);
          else
            cum->adjust[cum->num_adjusts++] = gen_ashlsi3 (reg, reg, amount);
        }


OK?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-07-06  Daniel Jacobowitz  <drow@mvista.com>

	* mips-tdep.c (mips_type_needs_double_align): New function.
	(mips_push_arguments): Align o32 structs to even argument
	registers if necessary.  Shift small structures to the left
	in registers even if the target is little endian.

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.56
diff -u -r1.56 mips-tdep.c
--- mips-tdep.c	2001/07/06 05:35:17	1.56
+++ mips-tdep.c	2001/07/06 18:03:57
@@ -2126,6 +2133,36 @@
 	   && MIPS_FPU_TYPE != MIPS_FPU_NONE);
 }
 
+/* On o32, argument passing in GPRs depends on the alignment of the type being
+   passed.  Return 1 if this type must be aligned to a doubleword boundary. */
+
+static int
+mips_type_needs_double_align (struct type *type)
+{
+  enum type_code typecode = TYPE_CODE (type);
+  
+  if (typecode == TYPE_CODE_FLT && TYPE_LENGTH (type) == 8)
+    return 1;
+  else if (typecode == TYPE_CODE_STRUCT)
+    {
+      if (TYPE_NFIELDS (type) < 1)
+	return 0;
+      return mips_type_needs_double_align (TYPE_FIELD_TYPE (type, 0));
+    }
+  else if (typecode == TYPE_CODE_UNION)
+    {
+      int i, n;    
+
+      n = TYPE_NFIELDS (type);
+      for (i = 0; i < n; i++)
+	if (mips_type_needs_double_align (TYPE_FIELD_TYPE (type, i)))
+	  return 1;
+      return 0;
+    }
+  return 0;
+}
+
+
 CORE_ADDR
 mips_push_arguments (int nargs,
 		     value_ptr *args,
@@ -2312,6 +2349,13 @@
 	     compatibility, we will put them in both places.  */
 	  int odd_sized_struct = ((len > MIPS_SAVED_REGSIZE) &&
 				  (len % MIPS_SAVED_REGSIZE != 0));
+	  /* Structures should be aligned to eight bytes (even arg registers)
+	     on MIPS_ABI_O32 if their first member has double precision. */
+	  if (gdbarch_tdep (current_gdbarch)->mips_abi == MIPS_ABI_O32
+	      && mips_type_needs_double_align (arg_type))
+	    {
+	      argreg += argreg & 1;
+	    }
 	  /* Note: Floating-point values that didn't fit into an FP
              register are only written to memory. */
 	  while (len > 0)
@@ -2393,7 +2437,8 @@
 
 		  if (!MIPS_EABI
 		      && MIPS_SAVED_REGSIZE < 8
-		      && TARGET_BYTE_ORDER == BIG_ENDIAN
+		      && (TARGET_BYTE_ORDER == BIG_ENDIAN
+			  || TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE)
 		      && partial_len < MIPS_SAVED_REGSIZE
 		      && (typecode == TYPE_CODE_STRUCT ||
 			  typecode == TYPE_CODE_UNION))

Re: [rfa] mips argument passing fixes for o32

[Andrew Cagney]

FYI,

> These are based on testsuite failures (call-*-st, if I remember correctly)
> and reading the argument passing code in GCC.  The struct alignment fix
> definitely agrees with the ABI, though it's not always clear on this point -
> it becomes necessary for us when the return value is a struct and thus there
> is a hidden pointer as the first argument.  The shift fix matches this
> comment in GCC and is not really specified by the ABI document:


Reviewing this could take a long time.

	Andrew

Re: [rfa] mips argument passing fixes for o32

[Andrew Cagney]

> These are based on testsuite failures (call-*-st, if I remember correctly)
> and reading the argument passing code in GCC.  The struct alignment fix
> definitely agrees with the ABI, though it's not always clear on this point -
> it becomes necessary for us when the return value is a struct and thus there
> is a hidden pointer as the first argument.  The shift fix matches this
> comment in GCC and is not really specified by the ABI document:


I get the feeling that these two changes fix two separate bugs:

	o	botched struct parameter pass

	o	botched struct return

Looking at just the first one:

> +	  /* Structures should be aligned to eight bytes (even arg registers)
> +	     on MIPS_ABI_O32 if their first member has double precision. */
> +	  if (gdbarch_tdep (current_gdbarch)->mips_abi == MIPS_ABI_O32
> +	      && mips_type_needs_double_align (arg_type))
> +	    {
> +	      argreg += argreg & 1;
> +	    }

this is approved.  However could you please change the expression:


	argreg += argreg & 1;


to something like:


	if ((argreg & 1))
	  argreg++;

your original expression makes my head hurt :-)

	Andrew

Re: [rfa] mips argument passing fixes for o32

[Daniel Jacobowitz]

On Thu, Jul 12, 2001 at 03:44:18AM -0400, Andrew Cagney wrote:
> > These are based on testsuite failures (call-*-st, if I remember correctly)
> > and reading the argument passing code in GCC.  The struct alignment fix
> > definitely agrees with the ABI, though it's not always clear on this point -
> > it becomes necessary for us when the return value is a struct and thus there
> > is a hidden pointer as the first argument.  The shift fix matches this
> > comment in GCC and is not really specified by the ABI document:
> 
> 
> I get the feeling that these two changes fix two separate bugs:
> 
> 	o	botched struct parameter pass
> 
> 	o	botched struct return

Actually, they're both struct parameter passing.  But yes, two separate
bugs.

> Looking at just the first one:
> 
> > +	  /* Structures should be aligned to eight bytes (even arg registers)
> > +	     on MIPS_ABI_O32 if their first member has double precision. */
> > +	  if (gdbarch_tdep (current_gdbarch)->mips_abi == MIPS_ABI_O32
> > +	      && mips_type_needs_double_align (arg_type))
> > +	    {
> > +	      argreg += argreg & 1;
> > +	    }
> 
> this is approved.  However could you please change the expression:

> to something like:
> 
> 
> 	if ((argreg & 1))
> 	  argreg++;

Changed, and committed.  Thanks.

> your original expression makes my head hurt :-)

Got it from GCC :)

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] mips argument passing fixes for o32

[Andrew Cagney]

> The shift fix matches this
> comment in GCC and is not really specified by the ABI document:


> -		      && TARGET_BYTE_ORDER == BIG_ENDIAN
> +		      && (TARGET_BYTE_ORDER == BIG_ENDIAN
> +			  || TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE)
> 

I don't think this change is right.  In little endian mode, a one byte 
struct will end up being stored at A+4 instead of A.

Which failures did it appear to fix?  Which endian is MIPS/Linux?

	Andrew

Re: [rfa] mips argument passing fixes for o32

[Daniel Jacobowitz]

On Thu, Jul 12, 2001 at 04:23:49AM -0400, Andrew Cagney wrote:
> > The shift fix matches this
> > comment in GCC and is not really specified by the ABI document:
> 
> 
> > -		      && TARGET_BYTE_ORDER == BIG_ENDIAN
> > +		      && (TARGET_BYTE_ORDER == BIG_ENDIAN
> > +			  || TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE)
> > 
> 
> I don't think this change is right.  In little endian mode, a one byte 
> struct will end up being stored at A+4 instead of A.
> 
> Which failures did it appear to fix?  Which endian is MIPS/Linux?

MIPS/Linux can be either big or little endian; the failure was on
little endian.

The effect is to store a one byte struct at A+3 (not A+4); this matches
what at least GCC does from what I can tell, and the comment in GCC
says that SGI compilers do the same.

The testcase was the passing of a three_char_t in call-ar-st (calling
print_three_chars from the gdb prompt).

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] mips argument passing fixes for o32

[Andrew Cagney]

>> I don't think this change is right.  In little endian mode, a one byte 
>> struct will end up being stored at A+4 instead of A.
>> 
>> Which failures did it appear to fix?  Which endian is MIPS/Linux?
> 
> 
> MIPS/Linux can be either big or little endian; the failure was on
> little endian.
> 
> The effect is to store a one byte struct at A+3 (not A+4); this matches
> what at least GCC does from what I can tell, and the comment in GCC
> says that SGI compilers do the same.


(A+3, yes, sorry).

The SGI compilers are big endian (correct?) so who knows what they would 
do in the little endian case.


> The testcase was the passing of a three_char_t in call-ar-st (calling
> print_three_chars from the gdb prompt).


This is going to take more digging.  It smells like a GCC bug.  It is 
definitly inconsistent with other ABIs where the struct is always stored 
at A and not A+3.

	Andrew

Re: [rfa] mips argument passing fixes for o32

[Daniel Jacobowitz]

On Thu, Jul 12, 2001 at 02:59:53PM -0400, Andrew Cagney wrote:
> The SGI compilers are big endian (correct?) so who knows what they would 
> do in the little endian case.

There are other MIPS little-endian compilers, and:

> > The testcase was the passing of a three_char_t in call-ar-st (calling
> > print_three_chars from the gdb prompt).
> 
> 
> This is going to take more digging.  It smells like a GCC bug.  It is 
> definitly inconsistent with other ABIs where the struct is always stored 
> at A and not A+3.

If you read the original message at:
 < http://sources.redhat.com/ml/gdb-patches/2001-07/msg00156.html >
you'll see the comment in GCC about this behavior.  It says that
structures of less than word size will be passed in the high end of
registers, as a hack for compatibility with other MIPS compilers.

It would be interesting to see how other compilers actually handle
this, but I don't have access to any other little-endian MIPS
toolchains.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] mips argument passing fixes for o32

[Andrew Cagney]

> 
> If you read the original message at:
>  < http://sources.redhat.com/ml/gdb-patches/2001-07/msg00156.html >
> you'll see the comment in GCC about this behavior.  It says that
> structures of less than word size will be passed in the high end of
> registers, as a hack for compatibility with other MIPS compilers.
> 
> It would be interesting to see how other compilers actually handle
> this, but I don't have access to any other little-endian MIPS
> toolchains.


Yes, but as far as I know, that hack is for big endian and not little 
endian mips.  I need to look at a few of the 60 odd MIPS ABI 
combinations that gdb can debug and check if I've got my story straight :-/

	Andrew

Re: [rfa] mips argument passing fixes for o32

[Andrew Cagney]

> 
> What should I do about the test in the meantime?  I don't have access
> to any other LE o32 compilers besides GCC, so I can't help you with
> that, and just leaving the FAIL seems inadequate.  I suppose I could
> XFAIL it.


Just leave it fail.  It is still a fixable bug.  Just need to figure out 
where it is.

	Andrew

Re: [rfa] mips argument passing fixes for o32

[Elena Zannoni]

Andrew Cagney writes:
 > > 
 > > What should I do about the test in the meantime?  I don't have access
 > > to any other LE o32 compilers besides GCC, so I can't help you with
 > > that, and just leaving the FAIL seems inadequate.  I suppose I could
 > > XFAIL it.
 > 
 > 
 > Just leave it fail.  It is still a fixable bug.  Just need to figure out 
 > where it is.
 > 
 > 	Andrew
 > 
 > 

Please, stick a comment somewhere in the code/testfile. Or do you want
to go through this all over again next time? :-) (I can just see, with
my luck it'll be me having to refigure this out).

Elena

Re: [rfa] mips argument passing fixes for o32

[Andrew Cagney]

> Andrew Cagney writes:
>  > > > > What should I do about the test in the meantime?  I don't have access
>  > > to any other LE o32 compilers besides GCC, so I can't help you with
>  > > that, and just leaving the FAIL seems inadequate.  I suppose I could
>  > > XFAIL it.
>  > > > Just leave it fail.  It is still a fixable bug.  Just need to figure out 
>  > where it is.
>  > > Andrew
>  > > Please, stick a comment somewhere in the code/testfile. Or do you want
> to go through this all over again next time? :-) (I can just see, with
> my luck it'll be me having to refigure this out).


Good point. I've also created a PR.

	Andrew

Re: [rfa] mips argument passing fixes for o32

[Daniel Jacobowitz]

On Fri, Jul 20, 2001 at 06:20:46PM -0400, Andrew Cagney wrote:
> Daniel,
> 
> Just to close this current thread.  For the moment I'm rejecting the change:
> 
> >if (!MIPS_EABI
> >               && MIPS_SAVED_REGSIZE < 8
> >-              && TARGET_BYTE_ORDER == BIG_ENDIAN
> >+              && (TARGET_BYTE_ORDER == BIG_ENDIAN
> >+              || TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE)
> >               && partial_len < MIPS_SAVED_REGSIZE
> >               && (typecode == TYPE_CODE_STRUCT ||
> >               typecode == TYPE_CODE_UNION))
> 
> I need hard evidence (not comments from GCC) that LE o32 does indead 
> left shift small register structs.  I'll probably eventually get that 
> evidence however, until then I'd prefer to drop this change.

Meanwhile, to really close the current thread, I have adequately
convinced myself and the current MIPS GCC maintainer that GCC is in
error.  Hopefully be fixed soon in the dev snapshots, after he tests my
patch a little more.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] mips argument passing fixes for o32

[Daniel Jacobowitz]

On Fri, Jul 20, 2001 at 06:20:46PM -0400, Andrew Cagney wrote:
> Daniel,
> 
> Just to close this current thread.  For the moment I'm rejecting the change:
> 
> >if (!MIPS_EABI
> >               && MIPS_SAVED_REGSIZE < 8
> >-              && TARGET_BYTE_ORDER == BIG_ENDIAN
> >+              && (TARGET_BYTE_ORDER == BIG_ENDIAN
> >+              || TYPE_LENGTH (arg_type) < MIPS_SAVED_REGSIZE)
> >               && partial_len < MIPS_SAVED_REGSIZE
> >               && (typecode == TYPE_CODE_STRUCT ||
> >               typecode == TYPE_CODE_UNION))
> 
> I need hard evidence (not comments from GCC) that LE o32 does indead 
> left shift small register structs.  I'll probably eventually get that 
> evidence however, until then I'd prefer to drop this change.

Meanwhile, to really close the current thread, I have adequately
convinced myself and the current MIPS GCC maintainer that GCC is in
error.  Hopefully be fixed soon in the dev snapshots, after he tests my
patch a little more.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer