[PATCH] Fix complex argument handling in ppc64 dummy function call

[PATCH] Fix complex argument handling in ppc64 dummy function call

[Tiago Stürmer Daitx]

Handle complex arguments for dummy function call on PPC64. I refactored the
code to extract the float logic into a function to reuse it for complex 
arguments as well - ABI defines that complex arguments are to be handled as 
if 2 float arguments were given.

This patch fixes the following PPC64 testcases:

  -FAIL: gdb.base/callfuncs.exp: p t_float_complex_values(fc1, fc2)
  -FAIL: gdb.base/callfuncs.exp: p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4)
  -FAIL: gdb.base/callfuncs.exp: p t_double_complex_values(dc1, dc2)
  -FAIL: gdb.base/callfuncs.exp: p t_double_complex_many_args(dc1, dc2, dc3, dc4, dc1, dc2, dc3, dc4, dc1, dc2, dc3, dc4, dc1, dc2, dc3, dc4)
  -FAIL: gdb.base/callfuncs.exp: p t_long_double_complex_values(ldc1, ldc2)
  -FAIL: gdb.base/callfuncs.exp: p t_long_double_complex_many_args(ldc1, ldc2, ldc3, ldc4, ldc1, ldc2, ldc3, ldc4, ldc1, ldc2, ldc3, ldc4, ldc1, ldc2, ldc3, ldc4)

No regressions detected on PPC32 and PPC64.

Cheers,
Tiago

gdb/ChangeLog
2013-02-05  Tiago Sturmer Daitx  <tdaitx@linux.vnet.ibm.com>

	* ppc-sysv-tdep.c (ppc64_sysv_abi_push_dummy_call): Handle complex 
	arguments.
	* ppc-sysv-tdep.c (ppc64_sysv_abi_push_float): New functions to handle 
	float arguments.
 

diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c
index 0ffeab9..690d65d 100644
--- a/gdb/ppc-sysv-tdep.c
+++ b/gdb/ppc-sysv-tdep.c
@@ -1101,6 +1101,76 @@ convert_code_addr_to_desc_addr (CORE_ADDR code_addr, CORE_ADDR *desc_addr)
   return 1;
 }
 
+static void
+ppc64_sysv_abi_push_float (struct gdbarch *gdbarch, struct regcache *regcache,
+			   struct gdbarch_tdep *tdep, struct type *type, 
+			   const bfd_byte *val, int freg, int greg,
+			   CORE_ADDR gparam)
+{
+  gdb_byte regval[MAX_REGISTER_SIZE];
+  const gdb_byte *p;
+
+  if (TYPE_LENGTH (type) <= 8)
+    {
+      /* Version 1.7 of the 64-bit PowerPC ELF ABI says:
+
+	 "Single precision floating point values are mapped to
+	 the first word in a single doubleword."
+
+	 And version 1.9 says:
+
+	 "Single precision floating point values are mapped to
+	 the second word in a single doubleword."
+
+	 GDB then writes single precision floating point values
+	 at both words in a doubleword, to support both ABIs.  */
+      if (TYPE_LENGTH (type) == 4)
+	{
+	  memcpy (regval, val, 4);
+	  memcpy (regval + 4, val, 4);
+	  p = regval;
+	}
+      else
+	p = val;
+
+      /* Write value in the stack's parameter save area.  */
+      write_memory (gparam, p, 8);
+
+      /* Floats and Doubles go in f1 .. f13.  They also consume a left aligned 
+	 GREG, and can end up in memory.  */
+      if (freg <= 13)
+	{
+	  struct type *regtype;
+	  regtype = register_type (gdbarch, tdep->ppc_fp0_regnum + freg);
+
+	  convert_typed_floating (val, type, regval, regtype);
+	  regcache_cooked_write (regcache, tdep->ppc_fp0_regnum + freg, regval);
+	}
+      if (greg <= 10)
+	regcache_cooked_write (regcache, tdep->ppc_gp0_regnum + greg, regval);
+    }
+  else
+    {
+      /* IBM long double stored in two doublewords of the
+	 parameter save area and corresponding registers.  */
+      if (!tdep->soft_float && freg <= 13)
+	{
+	  regcache_cooked_write (regcache, tdep->ppc_fp0_regnum + freg, val);
+	  if (freg <= 12)
+	    regcache_cooked_write (regcache, tdep->ppc_fp0_regnum + freg + 1,
+				   val + 8);
+	}
+      if (greg <= 10)
+	{
+	  regcache_cooked_write (regcache, tdep->ppc_gp0_regnum + greg, val);
+	  if (greg <= 9)
+	    regcache_cooked_write (regcache, tdep->ppc_gp0_regnum + greg + 1,
+				   val + 8);
+	}
+      write_memory (gparam, val, TYPE_LENGTH (type));
+    }
+}
+
 /* Pass the arguments in either registers, or in the stack.  Using the
    ppc 64 bit SysV ABI.
 
@@ -1218,53 +1288,9 @@ ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch,
 
 	  if (TYPE_CODE (type) == TYPE_CODE_FLT && TYPE_LENGTH (type) <= 8)
 	    {
-	      /* Floats and Doubles go in f1 .. f13.  They also
-	         consume a left aligned GREG,, and can end up in
-	         memory.  */
 	      if (write_pass)
-		{
-		  gdb_byte regval[MAX_REGISTER_SIZE];
-		  const gdb_byte *p;
-
-		  /* Version 1.7 of the 64-bit PowerPC ELF ABI says:
-
-		     "Single precision floating point values are mapped to
-		     the first word in a single doubleword."
-
-		     And version 1.9 says:
-
-		     "Single precision floating point values are mapped to
-		     the second word in a single doubleword."
-
-		     GDB then writes single precision floating point values
-		     at both words in a doubleword, to support both ABIs.  */
-		  if (TYPE_LENGTH (type) == 4)
-		    {
-		      memcpy (regval, val, 4);
-		      memcpy (regval + 4, val, 4);
-		      p = regval;
-		    }
-		  else
-		    p = val;
-
-		  /* Write value in the stack's parameter save area.  */
-		  write_memory (gparam, p, 8);
-
-		  if (freg <= 13)
-		    {
-		      struct type *regtype
-                        = register_type (gdbarch, tdep->ppc_fp0_regnum);
-
-		      convert_typed_floating (val, type, regval, regtype);
-		      regcache_cooked_write (regcache,
-                                             tdep->ppc_fp0_regnum + freg,
-					     regval);
-		    }
-		  if (greg <= 10)
-		    regcache_cooked_write (regcache,
-					   tdep->ppc_gp0_regnum + greg,
-					   regval);
-		}
+		  ppc64_sysv_abi_push_float (gdbarch, regcache, tdep, type,
+					     val, freg, greg, gparam);
 
 	      freg++;
 	      greg++;
@@ -1276,36 +1302,55 @@ ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch,
 		   && (gdbarch_long_double_format (gdbarch)
 		       == floatformats_ibm_long_double))
 	    {
-	      /* IBM long double stored in two doublewords of the
-		 parameter save area and corresponding registers.  */
 	      if (write_pass)
-		{
-		  if (!tdep->soft_float && freg <= 13)
-		    {
-		      regcache_cooked_write (regcache,
-                                             tdep->ppc_fp0_regnum + freg,
-					     val);
-		      if (freg <= 12)
-			regcache_cooked_write (regcache,
-					       tdep->ppc_fp0_regnum + freg + 1,
-					       val + 8);
-		    }
-		  if (greg <= 10)
-		    {
-		      regcache_cooked_write (regcache,
-					     tdep->ppc_gp0_regnum + greg,
-					     val);
-		      if (greg <= 9)
-			regcache_cooked_write (regcache,
-					       tdep->ppc_gp0_regnum + greg + 1,
-					       val + 8);
-		    }
-		  write_memory (gparam, val, TYPE_LENGTH (type));
-		}
+		ppc64_sysv_abi_push_float (gdbarch, regcache, tdep, type, 
+					   val, freg, greg, gparam);
 	      freg += 2;
 	      greg += 2;
 	      gparam = align_up (gparam + TYPE_LENGTH (type), tdep->wordsize);
 	    }
+          else if (TYPE_CODE (type) == TYPE_CODE_COMPLEX
+              && (TYPE_LENGTH (type) == 8 || TYPE_LENGTH (type) == 16))
+            {
+              int i;
+              for (i = 0; i < 2; i++)
+                {
+                  if (write_pass)
+		    {
+		      struct type *target_type;
+		      target_type = check_typedef (TYPE_TARGET_TYPE (type));
+		      ppc64_sysv_abi_push_float (gdbarch, regcache, tdep, 
+						 target_type, val + i * 
+						 TYPE_LENGTH (target_type),
+						 freg, greg, gparam);
+		    }
+                  freg++;
+                  greg++;
+                  /* Always consume parameter stack space.  */
+                  gparam = align_up(gparam + 8, tdep->wordsize);
+                }
+            }
+          else if (TYPE_CODE (type) == TYPE_CODE_COMPLEX
+                   && TYPE_LENGTH (type) == 32
+                   && (gdbarch_long_double_format (gdbarch)
+                       == floatformats_ibm_long_double))
+            {
+              int i;
+              for (i = 0; i < 2; i++)
+                {
+		  struct type *target_type;
+		  target_type = check_typedef (TYPE_TARGET_TYPE (type));
+                  if (write_pass)
+		    ppc64_sysv_abi_push_float (gdbarch, regcache, tdep, 
+					       target_type, val + i * 
+					       TYPE_LENGTH (target_type),
+					       freg, greg, gparam);
+                  freg += 2;
+                  greg += 2;
+                  gparam = align_up (gparam + TYPE_LENGTH (target_type),
+                                     tdep->wordsize);
+                }
+            }
 	  else if (TYPE_CODE (type) == TYPE_CODE_DECFLOAT
 		   && TYPE_LENGTH (type) <= 8)
 	    {

Re: [PATCH] Fix complex argument handling in ppc64 dummy function call

[Sergio Durigan Junior]

On Thursday, February 28 2013, Tiago Stürmer Daitx wrote:

> Handle complex arguments for dummy function call on PPC64. I refactored the
> code to extract the float logic into a function to reuse it for complex 
> arguments as well - ABI defines that complex arguments are to be handled as 
> if 2 float arguments were given.

Thanks for the patch, Tiago.  I have only formatting comments.

> gdb/ChangeLog
> 2013-02-05  Tiago Sturmer Daitx  <tdaitx@linux.vnet.ibm.com>

Don't forget to update the date if your patch is accepted :-).

> 	* ppc-sysv-tdep.c (ppc64_sysv_abi_push_dummy_call): Handle complex 
> 	arguments.
> 	* ppc-sysv-tdep.c (ppc64_sysv_abi_push_float): New functions to handle 
> 	float arguments.

You don't need multiple entries for the same file.  The order of the
functions is reverse (it's not a big issue, but I try to write my
ChangeLog entries following the order of the modifications in the
patch).  You can also just write "New function." for the
`ppc64_sysv_abi_push_float'.  So your ChangeLog would be something like:

	* ppc-sysv-tdep.c (ppc64_sysv_abi_push_float): New function.
	(ppc64_sysv_abi_push_dummy_call): Handle complex arguments.

> diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c
> index 0ffeab9..690d65d 100644
> --- a/gdb/ppc-sysv-tdep.c
> +++ b/gdb/ppc-sysv-tdep.c
> @@ -1101,6 +1101,76 @@ convert_code_addr_to_desc_addr (CORE_ADDR code_addr, CORE_ADDR *desc_addr)
>    return 1;
>  }
>  
> +static void
> +ppc64_sysv_abi_push_float (struct gdbarch *gdbarch, struct regcache *regcache,
> +			   struct gdbarch_tdep *tdep, struct type *type, 
> +			   const bfd_byte *val, int freg, int greg,
> +			   CORE_ADDR gparam)
> +{

Since this is a new function, it needs a comment describing it.  Oh, and
there should be a blank line between the comment and the function
declaration :-).

> +      /* Floats and Doubles go in f1 .. f13.  They also consume a left aligned 
> +	 GREG, and can end up in memory.  */
> +      if (freg <= 13)
> +	{
> +	  struct type *regtype;
> +	  regtype = register_type (gdbarch, tdep->ppc_fp0_regnum + freg);

I know you've just made a copy-and-paste here, but it's a standard to
put a blank line between the declaration of the variable(s) and the
actual code.

> @@ -1276,36 +1302,55 @@ ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch,
>  		   && (gdbarch_long_double_format (gdbarch)
>  		       == floatformats_ibm_long_double))
>  	    {
> -	      /* IBM long double stored in two doublewords of the
> -		 parameter save area and corresponding registers.  */
>  	      if (write_pass)
> -		{
> -		  if (!tdep->soft_float && freg <= 13)
> -		    {
> -		      regcache_cooked_write (regcache,
> -                                             tdep->ppc_fp0_regnum + freg,
> -					     val);
> -		      if (freg <= 12)
> -			regcache_cooked_write (regcache,
> -					       tdep->ppc_fp0_regnum + freg + 1,
> -					       val + 8);
> -		    }
> -		  if (greg <= 10)
> -		    {
> -		      regcache_cooked_write (regcache,
> -					     tdep->ppc_gp0_regnum + greg,
> -					     val);
> -		      if (greg <= 9)
> -			regcache_cooked_write (regcache,
> -					       tdep->ppc_gp0_regnum + greg + 1,
> -					       val + 8);
> -		    }
> -		  write_memory (gparam, val, TYPE_LENGTH (type));
> -		}
> +		ppc64_sysv_abi_push_float (gdbarch, regcache, tdep, type, 

Spurious space at the end of the line.

> +					   val, freg, greg, gparam);
[...]
> +          else if (TYPE_CODE (type) == TYPE_CODE_COMPLEX
> +              && (TYPE_LENGTH (type) == 8 || TYPE_LENGTH (type) == 16))
> +            {
> +              int i;

Same comment as above: blank line between variable declaration and code.

> +              for (i = 0; i < 2; i++)
> +                {
> +                  if (write_pass)
> +		    {
> +		      struct type *target_type;

Likewise.

> +		      target_type = check_typedef (TYPE_TARGET_TYPE (type));
> +		      ppc64_sysv_abi_push_float (gdbarch, regcache, tdep, 

Spurious space at the end of the line.

> +						 target_type, val + i * 

Likewise.

> +						 TYPE_LENGTH (target_type),
> +						 freg, greg, gparam);
> +		    }
> +                  freg++;
> +                  greg++;
> +                  /* Always consume parameter stack space.  */
> +                  gparam = align_up(gparam + 8, tdep->wordsize);

Space between function name and open parenthesis.

> +                }
> +            }
> +          else if (TYPE_CODE (type) == TYPE_CODE_COMPLEX
> +                   && TYPE_LENGTH (type) == 32
> +                   && (gdbarch_long_double_format (gdbarch)
> +                       == floatformats_ibm_long_double))
> +            {
> +              int i;
> +              for (i = 0; i < 2; i++)
> +                {
> +		  struct type *target_type;
> +		  target_type = check_typedef (TYPE_TARGET_TYPE (type));
> +                  if (write_pass)

This "if" is indented only with spaces (not using GNU style).

> +		    ppc64_sysv_abi_push_float (gdbarch, regcache, tdep, 
> +					       target_type, val + i * 

Spurious space at the end of both lines.

> +					       TYPE_LENGTH (target_type),
> +					       freg, greg, gparam);
> +                  freg += 2;
> +                  greg += 2;
> +                  gparam = align_up (gparam + TYPE_LENGTH (target_type),
> +                                     tdep->wordsize);
> +                }
> +            }
>  	  else if (TYPE_CODE (type) == TYPE_CODE_DECFLOAT
>  		   && TYPE_LENGTH (type) <= 8)
>  	    {

Well, I guess that's it.  Sorry for the nitpicking, and thanks a lot for
the code refactoring :-).

-- 
Sergio

Re: [PATCH] Fix complex argument handling in ppc64 dummy function call

[Tiago Stürmer Daitx]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 7705 bytes --]

Sergio, many thanks for the thorough review and the tips. There's no better 
way to get these rules to stick in my head. ^_^

Let me know if there is still something amiss.

Cheers,
Tiago

gdb/ChangeLog
2013-03-01  Tiago Stürmer Daitx  <tdaitx@linux.vnet.ibm.com>

	* ppc-sysv-tdep.c (ppc64_sysv_abi_push_float): New function.
	(ppc64_sysv_abi_push_dummy_call): Handle complex arguments. 

diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c
index 0ffeab9..cb9fe3b 100644
--- a/gdb/ppc-sysv-tdep.c
+++ b/gdb/ppc-sysv-tdep.c
@@ -1101,6 +1101,83 @@ convert_code_addr_to_desc_addr (CORE_ADDR code_addr, CORE_ADDR *desc_addr)
   return 1;
 }
 
+/* Push a float in either registers, or in the stack.  Using the ppc 64 bit
+   SysV ABI.
+
+   This implements a dumbed down version of the ABI.  It always writes
+   values to memory, GPR and FPR, even when not necessary.  Doing this
+   greatly simplifies the logic.  */
+
+static void
+ppc64_sysv_abi_push_float (struct gdbarch *gdbarch, struct regcache *regcache,
+			   struct gdbarch_tdep *tdep, struct type *type, 
+			   const bfd_byte *val, int freg, int greg,
+			   CORE_ADDR gparam)
+{
+  gdb_byte regval[MAX_REGISTER_SIZE];
+  const gdb_byte *p;
+
+  if (TYPE_LENGTH (type) <= 8)
+    {
+      /* Version 1.7 of the 64-bit PowerPC ELF ABI says:
+
+	 "Single precision floating point values are mapped to
+	 the first word in a single doubleword."
+
+	 And version 1.9 says:
+
+	 "Single precision floating point values are mapped to
+	 the second word in a single doubleword."
+
+	 GDB then writes single precision floating point values
+	 at both words in a doubleword, to support both ABIs.  */
+      if (TYPE_LENGTH (type) == 4)
+	{
+	  memcpy (regval, val, 4);
+	  memcpy (regval + 4, val, 4);
+	  p = regval;
+	}
+      else
+	p = val;
+
+      /* Write value in the stack's parameter save area.  */
+      write_memory (gparam, p, 8);
+
+      /* Floats and Doubles go in f1 .. f13.  They also consume a left aligned
+	 GREG, and can end up in memory.  */
+      if (freg <= 13)
+	{
+	  struct type *regtype;
+
+	  regtype = register_type (gdbarch, tdep->ppc_fp0_regnum + freg);
+	  convert_typed_floating (val, type, regval, regtype);
+	  regcache_cooked_write (regcache, tdep->ppc_fp0_regnum + freg, regval);
+	}
+      if (greg <= 10)
+	regcache_cooked_write (regcache, tdep->ppc_gp0_regnum + greg, regval);
+    }
+  else
+    {
+      /* IBM long double stored in two doublewords of the
+	 parameter save area and corresponding registers.  */
+      if (!tdep->soft_float && freg <= 13)
+	{
+	  regcache_cooked_write (regcache, tdep->ppc_fp0_regnum + freg, val);
+	  if (freg <= 12)
+	    regcache_cooked_write (regcache, tdep->ppc_fp0_regnum + freg + 1,
+				   val + 8);
+	}
+      if (greg <= 10)
+	{
+	  regcache_cooked_write (regcache, tdep->ppc_gp0_regnum + greg, val);
+	  if (greg <= 9)
+	    regcache_cooked_write (regcache, tdep->ppc_gp0_regnum + greg + 1,
+				   val + 8);
+	}
+      write_memory (gparam, val, TYPE_LENGTH (type));
+    }
+}
+
 /* Pass the arguments in either registers, or in the stack.  Using the
    ppc 64 bit SysV ABI.
 
@@ -1218,53 +1295,9 @@ ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch,
 
 	  if (TYPE_CODE (type) == TYPE_CODE_FLT && TYPE_LENGTH (type) <= 8)
 	    {
-	      /* Floats and Doubles go in f1 .. f13.  They also
-	         consume a left aligned GREG,, and can end up in
-	         memory.  */
 	      if (write_pass)
-		{
-		  gdb_byte regval[MAX_REGISTER_SIZE];
-		  const gdb_byte *p;
-
-		  /* Version 1.7 of the 64-bit PowerPC ELF ABI says:
-
-		     "Single precision floating point values are mapped to
-		     the first word in a single doubleword."
-
-		     And version 1.9 says:
-
-		     "Single precision floating point values are mapped to
-		     the second word in a single doubleword."
-
-		     GDB then writes single precision floating point values
-		     at both words in a doubleword, to support both ABIs.  */
-		  if (TYPE_LENGTH (type) == 4)
-		    {
-		      memcpy (regval, val, 4);
-		      memcpy (regval + 4, val, 4);
-		      p = regval;
-		    }
-		  else
-		    p = val;
-
-		  /* Write value in the stack's parameter save area.  */
-		  write_memory (gparam, p, 8);
-
-		  if (freg <= 13)
-		    {
-		      struct type *regtype
-                        = register_type (gdbarch, tdep->ppc_fp0_regnum);
-
-		      convert_typed_floating (val, type, regval, regtype);
-		      regcache_cooked_write (regcache,
-                                             tdep->ppc_fp0_regnum + freg,
-					     regval);
-		    }
-		  if (greg <= 10)
-		    regcache_cooked_write (regcache,
-					   tdep->ppc_gp0_regnum + greg,
-					   regval);
-		}
+		  ppc64_sysv_abi_push_float (gdbarch, regcache, tdep, type,
+					     val, freg, greg, gparam);
 
 	      freg++;
 	      greg++;
@@ -1276,36 +1309,58 @@ ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch,
 		   && (gdbarch_long_double_format (gdbarch)
 		       == floatformats_ibm_long_double))
 	    {
-	      /* IBM long double stored in two doublewords of the
-		 parameter save area and corresponding registers.  */
 	      if (write_pass)
-		{
-		  if (!tdep->soft_float && freg <= 13)
-		    {
-		      regcache_cooked_write (regcache,
-                                             tdep->ppc_fp0_regnum + freg,
-					     val);
-		      if (freg <= 12)
-			regcache_cooked_write (regcache,
-					       tdep->ppc_fp0_regnum + freg + 1,
-					       val + 8);
-		    }
-		  if (greg <= 10)
-		    {
-		      regcache_cooked_write (regcache,
-					     tdep->ppc_gp0_regnum + greg,
-					     val);
-		      if (greg <= 9)
-			regcache_cooked_write (regcache,
-					       tdep->ppc_gp0_regnum + greg + 1,
-					       val + 8);
-		    }
-		  write_memory (gparam, val, TYPE_LENGTH (type));
-		}
+		ppc64_sysv_abi_push_float (gdbarch, regcache, tdep, type,
+					   val, freg, greg, gparam);
 	      freg += 2;
 	      greg += 2;
 	      gparam = align_up (gparam + TYPE_LENGTH (type), tdep->wordsize);
 	    }
+          else if (TYPE_CODE (type) == TYPE_CODE_COMPLEX
+              && (TYPE_LENGTH (type) == 8 || TYPE_LENGTH (type) == 16))
+            {
+              int i;
+
+              for (i = 0; i < 2; i++)
+                {
+		  if (write_pass)
+		    {
+		      struct type *target_type;
+
+		      target_type = check_typedef (TYPE_TARGET_TYPE (type));
+		      ppc64_sysv_abi_push_float (gdbarch, regcache, tdep,
+						 target_type, val + i *
+						 TYPE_LENGTH (target_type),
+						 freg, greg, gparam);
+		    }
+                  freg++;
+                  greg++;
+                  /* Always consume parameter stack space.  */
+                  gparam = align_up(gparam + 8, tdep->wordsize);
+                }
+            }
+          else if (TYPE_CODE (type) == TYPE_CODE_COMPLEX
+                   && TYPE_LENGTH (type) == 32
+                   && (gdbarch_long_double_format (gdbarch)
+                       == floatformats_ibm_long_double))
+            {
+              int i;
+              for (i = 0; i < 2; i++)
+                {
+		  struct type *target_type;
+
+		  target_type = check_typedef (TYPE_TARGET_TYPE (type));
+                  if (write_pass)
+		    ppc64_sysv_abi_push_float (gdbarch, regcache, tdep,
+					       target_type, val + i *
+					       TYPE_LENGTH (target_type),
+					       freg, greg, gparam);
+                  freg += 2;
+                  greg += 2;
+                  gparam = align_up (gparam + TYPE_LENGTH (target_type),
+                                     tdep->wordsize);
+                }
+            }
 	  else if (TYPE_CODE (type) == TYPE_CODE_DECFLOAT
 		   && TYPE_LENGTH (type) <= 8)
 	    {

Re: [PATCH] Fix complex argument handling in ppc64 dummy function call

[Sergio Durigan Junior]

On Friday, March 01 2013, Tiago Stürmer Daitx wrote:

> Sergio, many thanks for the thorough review and the tips. There's no better 
> way to get these rules to stick in my head. ^_^

Thanks a lot!

> Let me know if there is still something amiss.

Just one little thing :-).

> +          else if (TYPE_CODE (type) == TYPE_CODE_COMPLEX
> +              && (TYPE_LENGTH (type) == 8 || TYPE_LENGTH (type) == 16))
> +            {
> +              int i;
> +
> +              for (i = 0; i < 2; i++)
> +                {
> +		  if (write_pass)
> +		    {
> +		      struct type *target_type;
> +
> +		      target_type = check_typedef (TYPE_TARGET_TYPE (type));
> +		      ppc64_sysv_abi_push_float (gdbarch, regcache, tdep,
> +						 target_type, val + i *
> +						 TYPE_LENGTH (target_type),
> +						 freg, greg, gparam);
> +		    }
> +                  freg++;
> +                  greg++;
> +                  /* Always consume parameter stack space.  */
> +                  gparam = align_up(gparam + 8, tdep->wordsize);

Space between the function name and its arguments.  It should read:

      align_up (gparam....);

But don't bother sending the patch again just for that little nit.  When
the patch gets reviewed/accepted, just commit it with the proper fix
:-).

Thanks again,

-- 
Sergio

Re: [PATCH] Fix complex argument handling in ppc64 dummy function call

[Tiago Stürmer Daitx]

On Fri, 2013-03-01 at 15:57 -0300, Sergio Durigan Junior wrote:
> On Friday, March 01 2013, Tiago Stürmer Daitx wrote:
> 
> Space between the function name and its arguments.  It should read:
> 
>       align_up (gparam....);
> 
> But don't bother sending the patch again just for that little nit.  When
> the patch gets reviewed/accepted, just commit it with the proper fix
> :-).
> 
> Thanks again,
> 

Fixed. Thanks for the heads up and the time doing the review, Sergio. I
will wait for someone to either point out some other issue or -
hopefully - approve the patch. =)



-- 
Tiago Stürmer Daitx
tdaitx@linux.vnet.ibm.com
IBM - Linux Technology Center

Re: [PATCH] Fix complex argument handling in ppc64 dummy function call

[Pedro Alves]

(not a real review, probably Ulrich would be
the best reviewer).

On 03/01/2013 05:08 PM, Tiago Stürmer Daitx wrote:
> +		  struct type *target_type;
> +
> +		  target_type = check_typedef (TYPE_TARGET_TYPE (type));
> +                  if (write_pass)
> +		    ppc64_sysv_abi_push_float (gdbarch, regcache, tdep,

While quickly skimming the patch I just noticed something wrong with
the indenting here.

> +					       target_type, val + i *
> +					       TYPE_LENGTH (target_type),
> +					       freg, greg, gparam);
> +                  freg += 2;
> +                  greg += 2;
> +                  gparam = align_up (gparam + TYPE_LENGTH (target_type),
> +                                     tdep->wordsize);
> +                }


-- 
Pedro Alves

Re: [PATCH] Fix complex argument handling in ppc64 dummy function call

[Tiago Stürmer Daitx]

On Fri, 2013-03-01 at 20:25 +0000, Pedro Alves wrote:
> (not a real review, probably Ulrich would be
> the best reviewer).
> 
> On 03/01/2013 05:08 PM, Tiago Stürmer Daitx wrote:
> > +		  struct type *target_type;
> > +
> > +		  target_type = check_typedef (TYPE_TARGET_TYPE (type));
> > +                  if (write_pass)
> > +		    ppc64_sysv_abi_push_float (gdbarch, regcache, tdep,
> 
> While quickly skimming the patch I just noticed something wrong with
> the indenting here.
> 
> > +					       target_type, val + i *
> > +					       TYPE_LENGTH (target_type),
> > +					       freg, greg, gparam);
> > +                  freg += 2;
> > +                  greg += 2;
> > +                  gparam = align_up (gparam + TYPE_LENGTH (target_type),
> > +                                     tdep->wordsize);
> > +                }
> 
> 

Pedro, thanks for catching this. =)



And just in case it might help somebody else, this is what I did in vim
to show tabs and end-of-lines:

:set listchars=tab:»\ ,eol:¬,trail:¶
:set list

I picked those chars because I wanted them to look very differently from
everything else in the code. Hopefully I'll be able to quickly notice
and correct wrong indentations and spurious spaces at eol from now on.

Re: [PATCH] Fix complex argument handling in ppc64 dummy function call

[Ulrich Weigand]

Tiago Stürmer Daitxwrote:

> 2013-03-01  Tiago Stürmer Daitx  <tdaitx@linux.vnet.ibm.com>
> 
> 	* ppc-sysv-tdep.c (ppc64_sysv_abi_push_float): New function.
> 	(ppc64_sysv_abi_push_dummy_call): Handle complex arguments. 

Sorry for the late reply, I must have missed this.

With the minor formatting issues noted by Sergio and Pedro fixed,
this is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Fix complex argument handling in ppc64 dummy function call

[Tiago Stürmer Daitx]

On Mon, 2013-03-11 at 17:11 +0100, Ulrich Weigand wrote:
> With the minor formatting issues noted by Sergio and Pedro fixed,
> this is OK.

Patch has been committed with the fixes. See
http://sourceware.org/ml/gdb-cvs/2013-04/msg00002.html

Err... I just noticed that it is appearing as April 1st in the cvs
mailing list and I set the ChangeLog date as 2013-03-31. My mind was
still on the wrong timezone. Please let me know if I need to fix that
ChangeLog entry.

I suppose it's ok to ignore the wrong date in the commit log message (it
says 2013-03-01, I forgot to update that), right?

Thank you all for the review. =)

Cheers,
Tiago Daitx

-- 
Tiago Stürmer Daitx
tdaitx@linux.vnet.ibm.com
IBM - Linux Technology Center

Re: [PATCH] Fix complex argument handling in ppc64 dummy function call

[Ulrich Weigand]

Tiago Stürmer Daitxwrote:
> On Mon, 2013-03-11 at 17:11 +0100, Ulrich Weigand wrote:
> > With the minor formatting issues noted by Sergio and Pedro fixed,
> > this is OK.
> 
> Patch has been committed with the fixes. See
> http://sourceware.org/ml/gdb-cvs/2013-04/msg00002.html

Thanks!

> Err... I just noticed that it is appearing as April 1st in the cvs
> mailing list and I set the ChangeLog date as 2013-03-31. My mind was
> still on the wrong timezone. Please let me know if I need to fix that
> ChangeLog entry.

No worries.  As long as the ChangeLog entries are not out of order
with each other, this is fine.

> I suppose it's ok to ignore the wrong date in the commit log message (it
> says 2013-03-01, I forgot to update that), right?

Yes, this doesn't really matter.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com