[RFA] Altivec ABI patches

[RFA] Altivec ABI patches

[Elena Zannoni]

As promised, this patch adds the last missing bits of Altivec support.
It handles the new vector types for passing parameters and returning
values.

Elena

2002-04-26  Elena Zannoni  <ezannoni@redhat.com>

	* rs6000-tdep.c (rs6000_extract_return_value,
	rs6000_store_return_value): Handle returning vectors.
	(rs6000_gdbarch_init): Use
	ppc_sysv_abi_broken_use_struct_convention for native sysv cases.
	* ppc-linux-tdep.c (ppc_sysv_abi_broken_use_struct_convention):
	New function.
	(ppc_sysv_abi_use_struct_convention): Deal with functions returning
	vectors.
	(ppc_sysv_abi_push_arguments): Handle vector parameters.
	* ppc-tdep.h (ppc_sysv_abi_broken_use_struct_convention): Export.
        

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/uberbaum/gdb/rs6000-tdep.c,v
retrieving revision 1.60
diff -u -p -r1.60 rs6000-tdep.c
--- rs6000-tdep.c	26 Apr 2002 04:31:47 -0000	1.60
+++ rs6000-tdep.c	26 Apr 2002 20:34:10 -0000
@@ -1144,6 +1144,7 @@ static void
 rs6000_extract_return_value (struct type *valtype, char *regbuf, char *valbuf)
 {
   int offset = 0;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
 
   if (TYPE_CODE (valtype) == TYPE_CODE_FLT)
     {
@@ -1165,6 +1166,13 @@ rs6000_extract_return_value (struct type
 	  memcpy (valbuf, &ff, sizeof (float));
 	}
     }
+  else if (TYPE_CODE (valtype) == TYPE_CODE_ARRAY
+           && TYPE_LENGTH (valtype) == 16
+           && TYPE_VECTOR (valtype))
+    {
+      memcpy (valbuf, regbuf + REGISTER_BYTE (tdep->ppc_vr0_regnum + 2),
+	      TYPE_LENGTH (valtype));
+    }
   else
     {
       /* return value is copied starting from r3. */
@@ -1909,6 +1917,8 @@ rs6000_store_struct_return (CORE_ADDR ad
 static void
 rs6000_store_return_value (struct type *type, char *valbuf)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+
   if (TYPE_CODE (type) == TYPE_CODE_FLT)
 
     /* Floating point values are returned starting from FPR1 and up.
@@ -1917,6 +1927,13 @@ rs6000_store_return_value (struct type *
 
     write_register_bytes (REGISTER_BYTE (FP0_REGNUM + 1), valbuf,
 			  TYPE_LENGTH (type));
+  else if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+    {
+      if (TYPE_LENGTH (type) == 16
+          && TYPE_VECTOR (type))
+	write_register_bytes (REGISTER_BYTE (tdep->ppc_vr0_regnum + 2),
+			      valbuf, TYPE_LENGTH (type));
+    }
   else
     /* Everything else is returned in GPR3 and up. */
     write_register_bytes (REGISTER_BYTE (gdbarch_tdep (current_gdbarch)->ppc_gp0_regnum + 3),
@@ -2706,7 +2723,7 @@ rs6000_gdbarch_init (struct gdbarch_info
           || osabi == ELFOSABI_NETBSD
           || osabi == ELFOSABI_FREEBSD)
 	set_gdbarch_use_struct_convention (gdbarch,
-					   generic_use_struct_convention);
+					   ppc_sysv_abi_broken_use_struct_convention);
       else
 	set_gdbarch_use_struct_convention (gdbarch,
 					   ppc_sysv_abi_use_struct_convention);
Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/uberbaum/gdb/ppc-linux-tdep.c,v
retrieving revision 1.16
diff -u -p -r1.16 ppc-linux-tdep.c
--- ppc-linux-tdep.c	24 Apr 2002 16:28:16 -0000	1.16
+++ ppc-linux-tdep.c	26 Apr 2002 20:34:32 -0000
@@ -415,11 +415,29 @@ ppc_linux_frame_chain (struct frame_info
    it may be used generically by ports which use either the SysV ABI or
    the EABI */
 
+/* Until November 2001, gcc was not complying to the SYSV ABI for
+   returning structures less than or equal to 8 bytes in size.  It was
+   returning everything in memory.  When this was corrected, it wasn't
+   fixed for native platforms.  */
+int
+ppc_sysv_abi_broken_use_struct_convention (int gcc_p, struct type *value_type)
+{
+  if (TYPE_LENGTH (value_type) == 16
+      && TYPE_VECTOR (value_type))
+    return 0;
+
+  return generic_use_struct_convention (gcc_p, value_type);
+}
+
 /* Structures 8 bytes or less long are returned in the r3 & r4
    registers, according to the SYSV ABI. */
 int
 ppc_sysv_abi_use_struct_convention (int gcc_p, struct type *value_type)
 {
+  if (TYPE_LENGTH (value_type) == 16
+      && TYPE_VECTOR (value_type))
+    return 0;
+
   return (TYPE_LENGTH (value_type) > 8);
 }
 
@@ -445,7 +463,12 @@ ppc_sysv_abi_push_arguments (int nargs, 
 			     int struct_return, CORE_ADDR struct_addr)
 {
   int argno;
-  int greg, freg;
+  /* Next available general register for non-float, non-vector arguments. */
+  int greg;
+  /* Next available floating point register for float arguments. */
+  int freg;
+  /* Next available vector register for vector arguments. */
+  int vreg;
   int argstkspace;
   int structstkspace;
   int argoffset;
@@ -458,6 +481,7 @@ ppc_sysv_abi_push_arguments (int nargs, 
 
   greg = struct_return ? 4 : 3;
   freg = 1;
+  vreg = 2;
   argstkspace = 0;
   structstkspace = 0;
 
@@ -500,21 +524,38 @@ ppc_sysv_abi_push_arguments (int nargs, 
 	      greg += 2;
 	    }
 	}
-      else
-	{
+      else if (!TYPE_VECTOR (type))
+        {
 	  if (len > 4
 	      || TYPE_CODE (type) == TYPE_CODE_STRUCT
 	      || TYPE_CODE (type) == TYPE_CODE_UNION)
 	    {
 	      /* Rounding to the nearest multiple of 8 may not be necessary,
-	         but it is safe.  Particularly since we don't know the
-	         field types of the structure */
+		 but it is safe.  Particularly since we don't know the
+		 field types of the structure */
 	      structstkspace += round2 (len, 8);
 	    }
 	  if (greg <= 10)
 	    greg++;
 	  else
 	    argstkspace += 4;
+    	}
+      else
+        {
+          if (len == 16
+	      && TYPE_CODE (type) == TYPE_CODE_ARRAY
+	      && TYPE_VECTOR (type))
+	    {
+	      if (vreg <= 13)
+		vreg++;
+	      else
+		{
+		  /* Vector arguments must be aligned to 16 bytes on
+                     the stack. */
+		  argstkspace += round2 (argstkspace, 16);
+		  argstkspace += 16;
+		}
+	    }
 	}
     }
 
@@ -540,6 +581,7 @@ ppc_sysv_abi_push_arguments (int nargs, 
   structoffset = argoffset + argstkspace;
   freg = 1;
   greg = 3;
+  vreg = 2;
   /* Fill in r3 with the return structure, if any */
   if (struct_return)
     {
@@ -599,7 +641,7 @@ ppc_sysv_abi_push_arguments (int nargs, 
 	      greg += 2;
 	    }
 	}
-      else
+      else if (!TYPE_VECTOR (type))
 	{
 	  char val_buf[4];
 	  if (len > 4
@@ -627,6 +669,32 @@ ppc_sysv_abi_push_arguments (int nargs, 
 	      argoffset += 4;
 	    }
 	}
+      else
+	{
+	  if (len == 16
+	      && TYPE_CODE (type) == TYPE_CODE_ARRAY
+	      && TYPE_VECTOR (type))
+	    {
+	      struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+	      char *v_val_buf = alloca (16);
+	      memset (v_val_buf, 0, 16);
+	      memcpy (v_val_buf, VALUE_CONTENTS (arg), len);
+	      if (vreg <= 13)
+		{
+		  *(int *) &registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
+						     + vreg)] = 0;
+		  memcpy (&registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
+						    + vreg)],
+			  v_val_buf, 16);
+		  vreg++;
+		}
+	      else
+		{
+		  write_memory (sp + argoffset, v_val_buf, 16);
+		  argoffset += 16;
+		}
+	    }
+        }
     }
 
   target_store_registers (-1);
Index: ppc-tdep.h
===================================================================
RCS file: /cvs/uberbaum/gdb/ppc-tdep.h,v
retrieving revision 1.9
diff -u -p -r1.9 ppc-tdep.h
--- ppc-tdep.h	12 Apr 2002 19:48:36 -0000	1.9
+++ ppc-tdep.h	26 Apr 2002 20:34:41 -0000
@@ -32,6 +32,7 @@ int ppc_linux_frameless_function_invocat
 void ppc_linux_frame_init_saved_regs (struct frame_info *);
 CORE_ADDR ppc_linux_frame_chain (struct frame_info *);
 int ppc_sysv_abi_use_struct_convention (int, struct type *);
+int ppc_sysv_abi_broken_use_struct_convention (int, struct type *);
 CORE_ADDR ppc_sysv_abi_push_arguments (int, struct value **, CORE_ADDR, int,
 				       CORE_ADDR);
 int ppc_linux_memory_remove_breakpoint (CORE_ADDR addr, char *contents_cache);

Re: [RFA] Altivec ABI patches

[Kevin Buettner]

On Apr 26,  4:44pm, Elena Zannoni wrote:

> As promised, this patch adds the last missing bits of Altivec support.
> It handles the new vector types for passing parameters and returning
> values.
> 
> Elena
> 
> 2002-04-26  Elena Zannoni  <ezannoni@redhat.com>
> 
> 	* rs6000-tdep.c (rs6000_extract_return_value,
> 	rs6000_store_return_value): Handle returning vectors.
> 	(rs6000_gdbarch_init): Use
> 	ppc_sysv_abi_broken_use_struct_convention for native sysv cases.
> 	* ppc-linux-tdep.c (ppc_sysv_abi_broken_use_struct_convention):
> 	New function.
> 	(ppc_sysv_abi_use_struct_convention): Deal with functions returning
> 	vectors.
> 	(ppc_sysv_abi_push_arguments): Handle vector parameters.
> 	* ppc-tdep.h (ppc_sysv_abi_broken_use_struct_convention): Export.

These changes look reasonable except for the following change to
ppc_sysv_abi_push_arguments():

> +	      if (vreg <= 13)
> +		{
> +		  *(int *) &registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
> +						     + vreg)] = 0;
> +		  memcpy (&registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
> +						    + vreg)],
> +			  v_val_buf, 16);
> +		  vreg++;
> +		}

Specifically, I'm having trouble understanding the point of this
assignment:

> +		  *(int *) &registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
> +						     + vreg)] = 0;

Note that the subsequent memcpy() ends up overwriting the memory
zero'd by the assignment.  If the assignment does serve some useful
purpose, I'd prefer to see memset() used instead.

Your patch is approved so long as you address this point.

Thanks,

Kevin

Re: [RFA] Altivec ABI patches

[Elena Zannoni]

Kevin Buettner writes:
 > On Apr 26,  4:44pm, Elena Zannoni wrote:
 > 
 > > As promised, this patch adds the last missing bits of Altivec support.
 > > It handles the new vector types for passing parameters and returning
 > > values.
 > > 
 > > Elena
 > > 
 > > 2002-04-26  Elena Zannoni  <ezannoni@redhat.com>
 > > 
 > > 	* rs6000-tdep.c (rs6000_extract_return_value,
 > > 	rs6000_store_return_value): Handle returning vectors.
 > > 	(rs6000_gdbarch_init): Use
 > > 	ppc_sysv_abi_broken_use_struct_convention for native sysv cases.
 > > 	* ppc-linux-tdep.c (ppc_sysv_abi_broken_use_struct_convention):
 > > 	New function.
 > > 	(ppc_sysv_abi_use_struct_convention): Deal with functions returning
 > > 	vectors.
 > > 	(ppc_sysv_abi_push_arguments): Handle vector parameters.
 > > 	* ppc-tdep.h (ppc_sysv_abi_broken_use_struct_convention): Export.
 > 
 > These changes look reasonable except for the following change to
 > ppc_sysv_abi_push_arguments():
 > 
 > > +	      if (vreg <= 13)
 > > +		{
 > > +		  *(int *) &registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
 > > +						     + vreg)] = 0;
 > > +		  memcpy (&registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
 > > +						    + vreg)],
 > > +			  v_val_buf, 16);
 > > +		  vreg++;
 > > +		}
 > 
 > Specifically, I'm having trouble understanding the point of this
 > assignment:
 > 
 > > +		  *(int *) &registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
 > > +						     + vreg)] = 0;
 > 
 > Note that the subsequent memcpy() ends up overwriting the memory
 > zero'd by the assignment.  If the assignment does serve some useful
 > purpose, I'd prefer to see memset() used instead.
 > 

It does seem odd, I actually just cut and pasted the code from a few
lines above. I thought there was some reason for it, after all.

	    {
	      *(int *) &registers[REGISTER_BYTE (greg)] = 0;
	      memcpy (&registers[REGISTER_BYTE (greg)], val_buf, 4);
	      greg++;
	    }

Should this go as well, then? Unless it always writes 4 bytes, but the
size of the general register can be bigger?


 > Your patch is approved so long as you address this point.
 > 

Ok.


 > Thanks,
 > 
 > Kevin

Elena

Re: [RFA] Altivec ABI patches

[Kevin Buettner]

On Apr 26,  9:44pm, Elena Zannoni wrote:

>  > Note that the subsequent memcpy() ends up overwriting the memory
>  > zero'd by the assignment.  If the assignment does serve some useful
>  > purpose, I'd prefer to see memset() used instead.
>  > 
> 
> It does seem odd, I actually just cut and pasted the code from a few
> lines above. I thought there was some reason for it, after all.
> 
> 	    {
> 	      *(int *) &registers[REGISTER_BYTE (greg)] = 0;
> 	      memcpy (&registers[REGISTER_BYTE (greg)], val_buf, 4);
> 	      greg++;
> 	    }
> 
> Should this go as well, then?

Yes, definitely.  As written, this code could yield some very
unexpected results on certain hosts.

Thanks for pointing this out.  This is very likely a mistake that I
made a while back.

> Unless it always writes 4 bytes, but the
> size of the general register can be bigger?

Well, the code in question doesn't work for 64-bit ABIs, but then, as
I understand it, the 64-bit PowerPC SVR4 ABI resembles the old PowerOpen
ABI than it does the 32-bit SVR4 ABI.  (Actually, I don't know that
much about it.  If someone could give me a pointer to an ABI spec for
the 64-bit ABI, I would appreciate it very much.)

Kevin

Re: [RFA] Altivec ABI patches

[David Edelsohn]

>>>>> Kevin Buettner writes:

| Well, the code in question doesn't work for 64-bit ABIs, but then, as
| I understand it, the 64-bit PowerPC SVR4 ABI resembles the old PowerOpen
| ABI than it does the 32-bit SVR4 ABI.  (Actually, I don't know that
| much about it.  If someone could give me a pointer to an ABI spec for
| the 64-bit ABI, I would appreciate it very much.)

	A copy of the 64-bit PowerPC SVR4 ABI Supplement is available in
Alan Modra's Toolchain directory on the linuxppc64 website:

ftp://ftp.linuxppc64.org/pub/people/amodra/PPC-elf64abi.txt.gz

	The ABI is mostly a natural mapping of the PowerOpen ABI to the
ELF file format.

David

Re: [RFA] Altivec ABI patches

[Elena Zannoni]

Kevin Buettner writes:
 > On Apr 26,  9:44pm, Elena Zannoni wrote:
 > 
 > >  > Note that the subsequent memcpy() ends up overwriting the memory
 > >  > zero'd by the assignment.  If the assignment does serve some useful
 > >  > purpose, I'd prefer to see memset() used instead.
 > >  > 
 > > 
 > > It does seem odd, I actually just cut and pasted the code from a few
 > > lines above. I thought there was some reason for it, after all.
 > > 
 > > 	    {
 > > 	      *(int *) &registers[REGISTER_BYTE (greg)] = 0;
 > > 	      memcpy (&registers[REGISTER_BYTE (greg)], val_buf, 4);
 > > 	      greg++;
 > > 	    }
 > > 
 > > Should this go as well, then?
 > 
 > Yes, definitely.  As written, this code could yield some very
 > unexpected results on certain hosts.
 > 
 > Thanks for pointing this out.  This is very likely a mistake that I
 > made a while back.
 > 
 > > Unless it always writes 4 bytes, but the
 > > size of the general register can be bigger?
 > 
 > Well, the code in question doesn't work for 64-bit ABIs, but then, as
 > I understand it, the 64-bit PowerPC SVR4 ABI resembles the old PowerOpen
 > ABI than it does the 32-bit SVR4 ABI.  (Actually, I don't know that
 > much about it.  If someone could give me a pointer to an ABI spec for
 > the 64-bit ABI, I would appreciate it very much.)
 > 
 > Kevin


OK, I have removed those 2 statements and committed the rest.

Elena

Re: [RFA] Altivec ABI patches

[Jim Blandy]

I don't see anything here that prevents vectors from being coerced to
pointers to their first element.  Isn't there some change needed in
evaluate_subexp_with_coercion?

Re: [RFA] Altivec ABI patches

[Elena Zannoni]

Jim Blandy writes:
 > 
 > I don't see anything here that prevents vectors from being coerced to
 > pointers to their first element.  Isn't there some change needed in
 > evaluate_subexp_with_coercion?


You mean value_arg_coerce()?  There is no arithmetic you can do with
these types.  Anyway, you are right I forgot that. My tests didn't
catch it because I was doing the call by hand after having done a step
into and finish from the function. The registers still contained the
correct values from the gcc-initiated call. Moving the gdb-initiated
call to before the gcc-initiated one exposed the problem.

I'll make sure the registers are trashed between calls.

Elena

Re: [RFA] Altivec ABI patches

[Jim Blandy]

Elena Zannoni <ezannoni@redhat.com> writes:
> Jim Blandy writes:
>  > 
>  > I don't see anything here that prevents vectors from being coerced to
>  > pointers to their first element.  Isn't there some change needed in
>  > evaluate_subexp_with_coercion?
> 
> You mean value_arg_coerce()?

Err, ... yeah, that's what I meant.  :)

> There is no arithmetic you can do with
> these types.  Anyway, you are right I forgot that. My tests didn't
> catch it because I was doing the call by hand after having done a step
> into and finish from the function. The registers still contained the
> correct values from the gcc-initiated call. Moving the gdb-initiated
> call to before the gcc-initiated one exposed the problem.
> 
> I'll make sure the registers are trashed between calls.

Gruvi.

Re: [RFA] Altivec ABI patches

[Jim Blandy]

Jim Blandy <jimb@redhat.com> writes:
> Elena Zannoni <ezannoni@redhat.com> writes:
> > Jim Blandy writes:
> >  > 
> >  > I don't see anything here that prevents vectors from being coerced to
> >  > pointers to their first element.  Isn't there some change needed in
> >  > evaluate_subexp_with_coercion?
> > 
> > You mean value_arg_coerce()?
> 
> Err, ... yeah, that's what I meant.  :)

Wait, no.  If `v' is a vector variable, and the user types `print v +
2', that should be an error, right?  But somehow it'll get promoted to
a pointer to its first element.  value_add calls COERCE_NUMBER, which
calls COERCE_ARRAY.  I think that's what you need to fix.  Note that
value_subscript already handles arrays specially --- it doesn't depend
on C's array coercion --- so it should continue to work.  I think.

You may have already done this, but I'd grep for TYPE_CODE_ARRAY and
make sure each case is handled properly.