Re: [PATCH] Fix passing/returning of complex data for PowerPC 32-bit

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> Date: Thu, 26 Jun 2014 06:54:59 +0100
> From: Luis Machado <lgustavo@codesourcery.com>
> 
> The PowerPC 32-bit unified ABI states that there are two ways of passing 
> and returning complex type data:
> 
> - Pointer, in a register, to a memory area.
> - Data in registers.
> 
> The problem is that it is not clear how to detect which variation a 
> program is using. GDB currently does a bit of both. It uses the first 
> mechanism for passing parameters and uses both to return data, depending 
> on the size of the data type. It is a bit messy because GDB is not 
> handling complex types explicitly.
> 
> Checking the gdb.base/callfuncs.exp testcase for a PowerPC 32-bit 
> target, with code built with GCC, showed a few failures related to 
> complex types.
> 
> This patch steers GDB towards what GCC seems to generate for PowerPC 
> 32-bit and handles complex type passing/return via general registers 
> (the second option). All failures are gone.
> 
> The problem here is if some other target/compiler is using the other 
> variation. So, for those that have a PowerPC 32-bit handy, can you 
> confirm it works reliably? I'm thinking AIX, Darwin or some other eabi 
> target.

AIX uses its own inplementations (rs6000_push_dummy_call and
rs6000_return_value).  And we don't support Darwin on PowerPC.

> Otherwise, does this look reasonable?

I agree that the "System V" support code should support the
ATR-PASS-COMPLEX-IN-GPRS ABI Attribute.  This is what the Linux ABI
uses (it is included in ATR-LINUX) which pretty much is the direct
succssor of the System V ABI (which didn't specify anything about
complex floating-point support).

If somebody really wants to support complex numbers on an embedded
system that uses ATR-PASS-COMPLEX-AS-STRUCT, they'll have to implement
an osabi sniffer for it and override the appropriate methods.

Code generally looks good.  Some nits below.  The comments are a bit
elaborate though.  I'd cut them down a bit; see my suggestion below.


> 2014-06-26  Luis Machado  <lgustavo@codesourcery.com>
> 
> 	* ppc-sysv-tdep.c (ppc_sysv_abi_push_dummy_call): Explicitly
> 	handle passing of complex arguments.
> 	(do_ppc_sysv_return_value): Explicitly handle return of
> 	complex types.
> 
> diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c
> index 1a880a6..2ea7796 100644
> --- a/gdb/ppc-sysv-tdep.c
> +++ b/gdb/ppc-sysv-tdep.c
> @@ -269,6 +269,57 @@ ppc_sysv_abi_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>  		  greg += 4;
>  		}
>  	    }
> +	  else if (TYPE_CODE (type) == TYPE_CODE_COMPLEX)
> +	    {
> +	      int lgpr = 11;

I'd simply use the constant 11 further down in the code instead of
intruducing a new variable here.  Matches the existing code better and
makes it easier to verify the code.

> +	      int type_size = TYPE_LENGTH (type);
> +	      int ngpr = type_size / tdep->wordsize;
> +
> +	      /* The PowerPC Unified 32-bit ABI states that complex types should
> +		 be handled in two different ways.  Either they are passed via
> +		 general registers or they are returned as a pointer, in a
> +		 general register, to an area that contains the data.
> +
> +		 Unfortunately there is no straightforward way to decide what
> +    		 variation a program is using.  Therefore we assume the GCC
> +    		 mechanism of passing the complex data in general registers.
> +
> +		 Float complex uses 2 consecutive GPR's.
> +
> +		 Double complex uses 4 consecutive GPR's.
> +
> +		 Long Double complex uses 4 or 8 consecutive GPR's, depending on
> +    		 whether the long double is represented as a double or as a
> +    		 128-bit entity.
> +
> +		 Scalar-based complex types are passed in the same way as their
> +		 floating point counterparts.  */

    /* The PowerPC Unified 32-bit ABI specifies two mutually
       conflicting conventions for passing and returning complex
       floating-point values.  These values are either treated as if
       they were represented as a structure containing an array of
       size two of the corresponding floating-point types (as
       identified by the ATR-PASS-COMPLEX-AS-STRUCT ABI attribute) or
       passed in the GPRs (as identified by the
       ATR-PASS-COMPLEX-IN-GPRS ABI attribute).  Since the latter
       convention is the default in GCC, and mandated by the Linux
       ABI, that's what we implement.  */

> +
> +	      /* Check if we should pass this parameter in registers or
> +		 stack.  */
> +	      if (ngpr + greg > lgpr)
> +		{
> +		  /* Pass parameter in the stack.  */
> +		  argoffset = align_up (argoffset, 8);
> +		  if (write_pass)
> +		    write_memory (sp + argoffset, val, len);
> +		  argoffset += len;
> +		}
> +	      else
> +		{
> +		  /* Pass the parameter in registers.  */
> +		  if (write_pass)
> +		    {
> +		      int i;
> +
> +		      for (i = 0; i < ngpr; i++)
> +			regcache_cooked_write (regcache,
> +					       tdep->ppc_gp0_regnum + greg + i,
> +					       val + i * 4);
> +		    }
> +		  greg += ngpr;
> +		}
> +	    }
>  	  else if (TYPE_CODE (type) == TYPE_CODE_DECFLOAT && len <= 8
>  		   && !tdep->soft_float)
>  	    {
> @@ -724,6 +775,45 @@ do_ppc_sysv_return_value (struct gdbarch *gdbarch, struct type *func_type,
>  	}
>        return RETURN_VALUE_REGISTER_CONVENTION;
>      }
> +
> +  /* The PowerPC Unified 32-bit ABI handles return of complex types in two
> +     different ways.  Either they are returned via general registers or they are
> +     returned as a pointer, in a general register, to an area that contains the
> +     data.
> +
> +     Unfortunately there is no straightforward way to decide what variation a
> +     program is using.  Therefore we assume the GCC mechanism of returning the
> +     complex data in general registers.
> +
> +     Float complex uses 2 consecutive GPR's.
> +
> +     Double complex uses 4 consecutive GPR's.
> +
> +     Long Double complex uses 4 or 8 consecutive GPR's, depending on whether the
> +     long double is represented as a double or as a 128-bit entity.
> +
> +     Scalar-based complex types are returned in the same way as their floating
> +     point counterparts.  */

No real need to repeat all this.  I'd say something like:

    /* The PowerPC Unified 32-bit specifies that complex
       floating-point values are returned in the GPRs for
       ATR-PASS-COMPLEX-IN-GPRS.  */

is good enough here.

> +  if (TYPE_CODE (type) == TYPE_CODE_COMPLEX)
> +    {
> +      int i, nregs;
> +      int return_reg = tdep->ppc_gp0_regnum + 3;
> +
> +      nregs = TYPE_LENGTH (type) / tdep->wordsize;
> +
> +      for (i = 0; i < nregs; i++)
> +	{
> +	  if (readbuf)
> +	    regcache_cooked_read (regcache, return_reg + i,
> +				  readbuf + i * tdep->wordsize);
> +	  if (writebuf)
> +	    regcache_cooked_write (regcache, return_reg + i,
> +				   writebuf + i * tdep->wordsize);
> +	}
> +	    
> +      return RETURN_VALUE_REGISTER_CONVENTION;
> +    }
> +
>    if (TYPE_CODE (type) == TYPE_CODE_FLT
>        && TYPE_LENGTH (type) == 16
>        && !tdep->soft_float