[PATCH 0/4] S390: Vector ABI support

[PATCH 0/4] S390: Vector ABI support

[Andreas Arnez]

This small patch series introduces vector ABI support for S/390
targets to GDB.  When an inferior uses the vector ABI, vector
arguments are now passed correctly to inferior functions, and a
function's return value of vector type can now be inspected and set
correctly.

Patch #1 fixes a minor issue in is_power_of_two(), which is used by
s390_push_dummy_call().  Patches #2 and #3 restructure the
implementations of s390_push_dummy_call() and s390_return_value(),
respectively, such that they become better readable and extensible,
and such that they can more easily be extended towards the vector ABI.
Patch #4 then adds the vector ABI support itself.

Andreas Arnez (4):
  S390: For zero, let is_power_of_two() return false
  S390: Restructure s390_push_dummy_call
  S390: Re-arrange implementation of s390_return_value
  S390: Vector ABI support

 gdb/NEWS              |   2 +
 gdb/s390-linux-tdep.c | 725 +++++++++++++++++++++++++-------------------------
 2 files changed, 361 insertions(+), 366 deletions(-)

-- 
1.9.3

[PATCH 1/4] S390: For zero, let is_power_of_two() return false

[Andreas Arnez]

This fixes a minor issue with the helper function is_power_of_two(),
which returned non-zero ("true") if the argument was zero.  This led
to a wrong decision when passing a zero-sized struct in an inferior
function call.

gdb/ChangeLog:

	* s390-linux-tdep.c (is_power_of_two): Add comment.  Return
          false if the argument is zero.
---
 gdb/s390-linux-tdep.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index b83a264..44feee7 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -2484,11 +2484,12 @@ is_float_like (struct type *type)
 	  || is_float_singleton (type));
 }
 
+/* Determine whether N is a power of two.  */
 
 static int
 is_power_of_two (unsigned int n)
 {
-  return ((n & (n - 1)) == 0);
+  return n && ((n & (n - 1)) == 0);
 }
 
 /* Return non-zero if TYPE should be passed as a pointer to a copy,
-- 
1.9.3

[PATCH 2/4] S390: Restructure s390_push_dummy_call

[Andreas Arnez]

Simplify the structure of s390_push_dummy_call and its various helper
functions.  This reduces the code and makes it easier to extend.  The
new code should be functionally equivalent to the old one, except that
copies created by the caller are now always aligned on an 8-byte
boundary.

gdb/ChangeLog:

	* s390-linux-tdep.c
	(is_float_singleton): Remove function.  Move the "singleton" part
	of the logic...
	(s390_effective_inner_type): ...here.  New function.
	(is_float_like): Remove function.  Inline its logic...
	(s390_function_arg_float): ...here.
	(is_pointer_like, is_integer_like, is_struct_like): Remove
	functions.  Inline their logic...
	(s390_function_arg_integer): ...here.
	(s390_function_arg_pass_by_reference): Remove function.
	(extend_simple_arg): Remove function.
	(alignment_of): Remove function.
	(struct s390_arg_state): New structure.
	(s390_handle_arg): New function.
	(s390_push_dummy_call): Move parameter placement logic to the new
	function s390_handle_arg.  Call it for calculating the stack area
	sizes first, and again for actually writing the parameters.
---
 gdb/s390-linux-tdep.c | 490 +++++++++++++++++++++-----------------------------
 1 file changed, 200 insertions(+), 290 deletions(-)

diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index 44feee7..3b7e4b6 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -2389,99 +2389,40 @@ s390_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
 
 /* Dummy function calls.  */
 
-/* Return non-zero if TYPE is an integer-like type, zero otherwise.
-   "Integer-like" types are those that should be passed the way
-   integers are: integers, enums, ranges, characters, and booleans.  */
-static int
-is_integer_like (struct type *type)
-{
-  enum type_code code = TYPE_CODE (type);
-
-  return (code == TYPE_CODE_INT
-	  || code == TYPE_CODE_ENUM
-	  || code == TYPE_CODE_RANGE
-	  || code == TYPE_CODE_CHAR
-	  || code == TYPE_CODE_BOOL);
-}
-
-/* Return non-zero if TYPE is a pointer-like type, zero otherwise.
-   "Pointer-like" types are those that should be passed the way
-   pointers are: pointers and references.  */
-static int
-is_pointer_like (struct type *type)
-{
-  enum type_code code = TYPE_CODE (type);
-
-  return (code == TYPE_CODE_PTR
-	  || code == TYPE_CODE_REF);
-}
-
-
-/* Return non-zero if TYPE is a `float singleton' or `double
-   singleton', zero otherwise.
-
-   A `T singleton' is a struct type with one member, whose type is
-   either T or a `T singleton'.  So, the following are all float
-   singletons:
+/* Unwrap any single-field structs in TYPE and return the effective
+   "inner" type.  E.g., yield "float" for all these cases:
 
-   struct { float x };
-   struct { struct { float x; } x; };
-   struct { struct { struct { float x; } x; } x; };
+     float x;
+     struct { float x };
+     struct { struct { float x; } x; };
+     struct { struct { struct { float x; } x; } x; };  */
 
-   ... and so on.
-
-   All such structures are passed as if they were floats or doubles,
-   as the (revised) ABI says.  */
-static int
-is_float_singleton (struct type *type)
+static struct type *
+s390_effective_inner_type (struct type *type)
 {
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT && TYPE_NFIELDS (type) == 1)
-    {
-      struct type *singleton_type = TYPE_FIELD_TYPE (type, 0);
-      CHECK_TYPEDEF (singleton_type);
-
-      return (TYPE_CODE (singleton_type) == TYPE_CODE_FLT
-	      || TYPE_CODE (singleton_type) == TYPE_CODE_DECFLOAT
-	      || is_float_singleton (singleton_type));
-    }
-
-  return 0;
+  while (TYPE_CODE (type) == TYPE_CODE_STRUCT
+	 && TYPE_NFIELDS (type) == 1)
+    type = check_typedef (TYPE_FIELD_TYPE (type, 0));
+  return type;
 }
 
+/* Return non-zero if TYPE should be passed like "float" or
+   "double".  */
 
-/* Return non-zero if TYPE is a struct-like type, zero otherwise.
-   "Struct-like" types are those that should be passed as structs are:
-   structs and unions.
-
-   As an odd quirk, not mentioned in the ABI, GCC passes float and
-   double singletons as if they were a plain float, double, etc.  (The
-   corresponding union types are handled normally.)  So we exclude
-   those types here.  *shrug* */
 static int
-is_struct_like (struct type *type)
+s390_function_arg_float (struct type *type)
 {
-  enum type_code code = TYPE_CODE (type);
-
-  return (code == TYPE_CODE_UNION
-	  || (code == TYPE_CODE_STRUCT && ! is_float_singleton (type)));
-}
-
+  /* Note that long double as well as complex types are intentionally
+     excluded. */
+  if (TYPE_LENGTH (type) > 8)
+    return 0;
 
-/* Return non-zero if TYPE is a float-like type, zero otherwise.
-   "Float-like" types are those that should be passed as
-   floating-point values are.
+  /* A struct containing just a float or double is passed like a float
+     or double.  */
+  type = s390_effective_inner_type (type);
 
-   You'd think this would just be floats, doubles, long doubles, etc.
-   But as an odd quirk, not mentioned in the ABI, GCC passes float and
-   double singletons as if they were a plain float, double, etc.  (The
-   corresponding union types are handled normally.)  So we include
-   those types here.  *shrug* */
-static int
-is_float_like (struct type *type)
-{
   return (TYPE_CODE (type) == TYPE_CODE_FLT
-	  || TYPE_CODE (type) == TYPE_CODE_DECFLOAT
-	  || is_float_singleton (type));
+	  || TYPE_CODE (type) == TYPE_CODE_DECFLOAT);
 }
 
 /* Determine whether N is a power of two.  */
@@ -2492,101 +2433,172 @@ is_power_of_two (unsigned int n)
   return n && ((n & (n - 1)) == 0);
 }
 
-/* Return non-zero if TYPE should be passed as a pointer to a copy,
-   zero otherwise.  */
-static int
-s390_function_arg_pass_by_reference (struct type *type)
-{
-  if (TYPE_LENGTH (type) > 8)
-    return 1;
-
-  return (is_struct_like (type) && !is_power_of_two (TYPE_LENGTH (type)))
-	  || TYPE_CODE (type) == TYPE_CODE_COMPLEX
-	  || (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type));
-}
+/* For an argument whose type is TYPE and which is not passed like a
+   float, return non-zero if it should be passed like "int" or "long
+   long".  */
 
-/* Return non-zero if TYPE should be passed in a float register
-   if possible.  */
-static int
-s390_function_arg_float (struct type *type)
-{
-  if (TYPE_LENGTH (type) > 8)
-    return 0;
-
-  return is_float_like (type);
-}
-
-/* Return non-zero if TYPE should be passed in an integer register
-   (or a pair of integer registers) if possible.  */
 static int
 s390_function_arg_integer (struct type *type)
 {
+  enum type_code code = TYPE_CODE (type);
+
   if (TYPE_LENGTH (type) > 8)
     return 0;
 
-   return is_integer_like (type)
-	  || is_pointer_like (type)
-	  || (is_struct_like (type) && is_power_of_two (TYPE_LENGTH (type)));
+  if (code == TYPE_CODE_INT
+      || code == TYPE_CODE_ENUM
+      || code == TYPE_CODE_RANGE
+      || code == TYPE_CODE_CHAR
+      || code == TYPE_CODE_BOOL
+      || code == TYPE_CODE_PTR
+      || code == TYPE_CODE_REF)
+    return 1;
+
+  return ((code == TYPE_CODE_UNION || code == TYPE_CODE_STRUCT)
+	  && is_power_of_two (TYPE_LENGTH (type)));
 }
 
-/* Return ARG, a `SIMPLE_ARG', sign-extended or zero-extended to a full
-   word as required for the ABI.  */
-static LONGEST
-extend_simple_arg (struct gdbarch *gdbarch, struct value *arg)
-{
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  struct type *type = check_typedef (value_type (arg));
+/* Argument passing state: Internal data structure passed to helper
+   routines of s390_push_dummy_call.  */
 
-  /* Even structs get passed in the least significant bits of the
-     register / memory word.  It's not really right to extract them as
-     an integer, but it does take care of the extension.  */
-  if (TYPE_UNSIGNED (type))
-    return extract_unsigned_integer (value_contents (arg),
-				     TYPE_LENGTH (type), byte_order);
-  else
-    return extract_signed_integer (value_contents (arg),
-				   TYPE_LENGTH (type), byte_order);
-}
+struct s390_arg_state
+  {
+    /* Register cache, or NULL, if we are in "preparation mode".  */
+    struct regcache *regcache;
+    /* Next available general/floating-point register for argument
+       passing.  */
+    int gr, fr;
+    /* Current pointer to copy area (grows downwards).  */
+    CORE_ADDR copy;
+    /* Current pointer to parameter area (grows upwards).  */
+    CORE_ADDR argp;
+  };
 
+/* Prepare one argument ARG for a dummy call and update the argument
+   passing state AS accordingly.  If the regcache field in AS is set,
+   operate in "write mode" and write ARG into the inferior.  Otherwise
+   run "preparation mode" and skip all updates to the inferior.  */
 
-/* Return the alignment required by TYPE.  */
-static int
-alignment_of (struct type *type)
+static void
+s390_handle_arg (struct s390_arg_state *as, struct value *arg,
+		 struct gdbarch_tdep *tdep, int word_size,
+		 enum bfd_endian byte_order)
 {
-  int alignment;
+  struct type *type = check_typedef (value_type (arg));
+  unsigned int length = TYPE_LENGTH (type);
+  int write_mode = as->regcache != NULL;
 
-  if (is_integer_like (type)
-      || is_pointer_like (type)
-      || TYPE_CODE (type) == TYPE_CODE_FLT
-      || TYPE_CODE (type) == TYPE_CODE_DECFLOAT)
-    alignment = TYPE_LENGTH (type);
-  else if (TYPE_CODE (type) == TYPE_CODE_STRUCT
-	   || TYPE_CODE (type) == TYPE_CODE_UNION)
+  if (s390_function_arg_float (type))
+    {
+      /* The GNU/Linux for S/390 ABI uses FPRs 0 and 2 to pass
+	 arguments.  The GNU/Linux for zSeries ABI uses 0, 2, 4, and
+	 6.  */
+      if (as->fr <= (tdep->abi == ABI_LINUX_S390 ? 2 : 6))
+	{
+	  /* When we store a single-precision value in an FP register,
+	     it occupies the leftmost bits.  */
+	  if (write_mode)
+	    regcache_cooked_write_part (as->regcache,
+					S390_F0_REGNUM + as->fr,
+					0, length,
+					value_contents (arg));
+	  as->fr += 2;
+	}
+      else
+	{
+	  /* When we store a single-precision value in a stack slot,
+	     it occupies the rightmost bits.  */
+	  as->argp = align_up (as->argp + length, word_size);
+	  if (write_mode)
+	    write_memory (as->argp - length, value_contents (arg),
+			  length);
+	}
+    }
+  else if (s390_function_arg_integer (type) && length <= word_size)
     {
-      int i;
+      ULONGEST val;
 
-      alignment = 1;
-      for (i = 0; i < TYPE_NFIELDS (type); i++)
+      if (write_mode)
 	{
-	  int field_alignment
-	    = alignment_of (check_typedef (TYPE_FIELD_TYPE (type, i)));
+	  /* Place value in least significant bits of the register or
+	     memory word and sign- or zero-extend to full word size.
+	     This also applies to a struct or union.  */
+	  val = TYPE_UNSIGNED (type)
+	    ? extract_unsigned_integer (value_contents (arg),
+					length, byte_order)
+	    : extract_signed_integer (value_contents (arg),
+				      length, byte_order);
+	}
 
-	  if (field_alignment > alignment)
-	    alignment = field_alignment;
+      if (as->gr <= 6)
+	{
+	  if (write_mode)
+	    regcache_cooked_write_unsigned (as->regcache,
+					    S390_R0_REGNUM + as->gr,
+					    val);
+	  as->gr++;
+	}
+      else
+	{
+	  if (write_mode)
+	    write_memory_unsigned_integer (as->argp, word_size,
+					   byte_order, val);
+	  as->argp += word_size;
 	}
     }
-  else
-    alignment = 1;
+  else if (s390_function_arg_integer (type) && length == 8)
+    {
+      if (as->gr <= 5)
+	{
+	  if (write_mode)
+	    {
+	      regcache_cooked_write (as->regcache,
+				     S390_R0_REGNUM + as->gr,
+				     value_contents (arg));
+	      regcache_cooked_write (as->regcache,
+				     S390_R0_REGNUM + as->gr + 1,
+				     value_contents (arg) + word_size);
+	    }
+	  as->gr += 2;
+	}
+      else
+	{
+	  /* If we skipped r6 because we couldn't fit a DOUBLE_ARG
+	     in it, then don't go back and use it again later.  */
+	  as->gr = 7;
 
-  /* Check that everything we ever return is a power of two.  Lots of
-     code doesn't want to deal with aligning things to arbitrary
-     boundaries.  */
-  gdb_assert ((alignment & (alignment - 1)) == 0);
+	  if (write_mode)
+	    write_memory (as->argp, value_contents (arg), length);
+	  as->argp += length;
+	}
+    }
+  else
+    {
+      /* This argument type is never passed in registers.  Place the
+	 value in the copy area and pass a pointer to it.  Use 8-byte
+	 alignment as a conservative assumption.  */
+      as->copy = align_down (as->copy - length, 8);
+      if (write_mode)
+	write_memory (as->copy, value_contents (arg), length);
 
-  return alignment;
+      if (as->gr <= 6)
+	{
+	  if (write_mode)
+	    regcache_cooked_write_unsigned (as->regcache,
+					    S390_R0_REGNUM + as->gr,
+					    as->copy);
+	  as->gr++;
+	}
+      else
+	{
+	  if (write_mode)
+	    write_memory_unsigned_integer (as->argp, word_size,
+					   byte_order, as->copy);
+	  as->argp += word_size;
+	}
+    }
 }
 
-
 /* Put the actual parameter values pointed to by ARGS[0..NARGS-1] in
    place to be passed to a function, as specified by the "GNU/Linux
    for S/390 ELF Application Binary Interface Supplement".
@@ -2601,6 +2613,7 @@ alignment_of (struct type *type)
 
    Our caller has taken care of any type promotions needed to satisfy
    prototypes or the old K&R argument-passing rules.  */
+
 static CORE_ADDR
 s390_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		      struct regcache *regcache, CORE_ADDR bp_addr,
@@ -2611,151 +2624,48 @@ s390_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   int word_size = gdbarch_ptr_bit (gdbarch) / 8;
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int i;
+  struct s390_arg_state arg_state, arg_prep;
+  CORE_ADDR param_area_start, new_sp;
 
-  /* If the i'th argument is passed as a reference to a copy, then
-     copy_addr[i] is the address of the copy we made.  */
-  CORE_ADDR *copy_addr = alloca (nargs * sizeof (CORE_ADDR));
+  arg_prep.copy = sp;
+  arg_prep.gr = struct_return ? 3 : 2;
+  arg_prep.fr = 0;
+  arg_prep.argp = 0;
+  arg_prep.regcache = NULL;
 
-  /* Reserve space for the reference-to-copy area.  */
-  for (i = 0; i < nargs; i++)
-    {
-      struct value *arg = args[i];
-      struct type *type = check_typedef (value_type (arg));
+  /* Initialize arg_state for "preparation mode".  */
+  arg_state = arg_prep;
 
-      if (s390_function_arg_pass_by_reference (type))
-	{
-	  sp -= TYPE_LENGTH (type);
-	  sp = align_down (sp, alignment_of (type));
-	  copy_addr[i] = sp;
-	}
-    }
-
-  /* Reserve space for the parameter area.  As a conservative
-     simplification, we assume that everything will be passed on the
-     stack.  Since every argument larger than 8 bytes will be
-     passed by reference, we use this simple upper bound.  */
-  sp -= nargs * 8;
+  /* Update arg_state.copy with the start of the reference-to-copy area
+     and arg_state.argp with the size of the parameter area.  */
+  for (i = 0; i < nargs; i++)
+    s390_handle_arg (&arg_state, args[i], tdep, word_size, byte_order);
 
-  /* After all that, make sure it's still aligned on an eight-byte
-     boundary.  */
-  sp = align_down (sp, 8);
+  param_area_start = align_down (arg_state.copy - arg_state.argp, 8);
 
   /* Allocate the standard frame areas: the register save area, the
-     word reserved for the compiler (which seems kind of meaningless),
-     and the back chain pointer.  */
-  sp -= 16*word_size + 32;
-
-  /* Now we have the final SP value.  Make sure we didn't underflow;
-     on 31-bit, this would result in addresses with the high bit set,
-     which causes confusion elsewhere.  Note that if we error out
-     here, stack and registers remain untouched.  */
-  if (gdbarch_addr_bits_remove (gdbarch, sp) != sp)
+     word reserved for the compiler, and the back chain pointer.  */
+  new_sp = param_area_start - (16 * word_size + 32);
+
+  /* Now we have the final stack pointer.  Make sure we didn't
+     underflow; on 31-bit, this would result in addresses with the
+     high bit set, which causes confusion elsewhere.  Note that if we
+     error out here, stack and registers remain untouched.  */
+  if (gdbarch_addr_bits_remove (gdbarch, new_sp) != new_sp)
     error (_("Stack overflow"));
 
+  /* Pass the structure return address in general register 2.  */
+  if (struct_return)
+    regcache_cooked_write_unsigned (regcache, S390_R2_REGNUM, struct_addr);
 
-  /* Finally, place the actual parameters, working from SP towards
-     higher addresses.  The code above is supposed to reserve enough
-     space for this.  */
-  {
-    int fr = 0;
-    int gr = 2;
-    CORE_ADDR starg = sp + 16*word_size + 32;
+  /* Initialize arg_state for "write mode".  */
+  arg_state = arg_prep;
+  arg_state.argp = param_area_start;
+  arg_state.regcache = regcache;
 
-    /* A struct is returned using general register 2.  */
-    if (struct_return)
-      {
-	regcache_cooked_write_unsigned (regcache, S390_R0_REGNUM + gr,
-					struct_addr);
-	gr++;
-      }
-
-    for (i = 0; i < nargs; i++)
-      {
-	struct value *arg = args[i];
-	struct type *type = check_typedef (value_type (arg));
-	unsigned length = TYPE_LENGTH (type);
-
-	if (s390_function_arg_pass_by_reference (type))
-	  {
-	    /* Actually copy the argument contents to the stack slot
-	       that was reserved above.  */
-	    write_memory (copy_addr[i], value_contents (arg), length);
-
-	    if (gr <= 6)
-	      {
-		regcache_cooked_write_unsigned (regcache, S390_R0_REGNUM + gr,
-						copy_addr[i]);
-		gr++;
-	      }
-	    else
-	      {
-		write_memory_unsigned_integer (starg, word_size, byte_order,
-					       copy_addr[i]);
-		starg += word_size;
-	      }
-	  }
-	else if (s390_function_arg_float (type))
-	  {
-	    /* The GNU/Linux for S/390 ABI uses FPRs 0 and 2 to pass arguments,
-	       the GNU/Linux for zSeries ABI uses 0, 2, 4, and 6.  */
-	    if (fr <= (tdep->abi == ABI_LINUX_S390 ? 2 : 6))
-	      {
-		/* When we store a single-precision value in an FP register,
-		   it occupies the leftmost bits.  */
-		regcache_cooked_write_part (regcache, S390_F0_REGNUM + fr,
-					    0, length, value_contents (arg));
-		fr += 2;
-	      }
-	    else
-	      {
-		/* When we store a single-precision value in a stack slot,
-		   it occupies the rightmost bits.  */
-		starg = align_up (starg + length, word_size);
-		write_memory (starg - length, value_contents (arg), length);
-	      }
-	  }
-	else if (s390_function_arg_integer (type) && length <= word_size)
-	  {
-	    if (gr <= 6)
-	      {
-		/* Integer arguments are always extended to word size.  */
-		regcache_cooked_write_signed (regcache, S390_R0_REGNUM + gr,
-					      extend_simple_arg (gdbarch,
-								 arg));
-		gr++;
-	      }
-	    else
-	      {
-		/* Integer arguments are always extended to word size.  */
-		write_memory_signed_integer (starg, word_size, byte_order,
-					     extend_simple_arg (gdbarch, arg));
-		starg += word_size;
-	      }
-	  }
-	else if (s390_function_arg_integer (type) && length == 2*word_size)
-	  {
-	    if (gr <= 5)
-	      {
-		regcache_cooked_write (regcache, S390_R0_REGNUM + gr,
-				       value_contents (arg));
-		regcache_cooked_write (regcache, S390_R0_REGNUM + gr + 1,
-				       value_contents (arg) + word_size);
-		gr += 2;
-	      }
-	    else
-	      {
-		/* If we skipped r6 because we couldn't fit a DOUBLE_ARG
-		   in it, then don't go back and use it again later.  */
-		gr = 7;
-
-		write_memory (starg, value_contents (arg), length);
-		starg += length;
-	      }
-	  }
-	else
-	  internal_error (__FILE__, __LINE__, _("unknown argument type"));
-      }
-  }
+  /* Write all parameters.  */
+  for (i = 0; i < nargs; i++)
+    s390_handle_arg (&arg_state, args[i], tdep, word_size, byte_order);
 
   /* Store return PSWA.  In 31-bit mode, keep addressing mode bit.  */
   if (word_size == 4)
@@ -2767,11 +2677,11 @@ s390_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   regcache_cooked_write_unsigned (regcache, S390_RETADDR_REGNUM, bp_addr);
 
   /* Store updated stack pointer.  */
-  regcache_cooked_write_unsigned (regcache, S390_SP_REGNUM, sp);
+  regcache_cooked_write_unsigned (regcache, S390_SP_REGNUM, new_sp);
 
   /* We need to return the 'stack part' of the frame ID,
      which is actually the top of the register save area.  */
-  return sp + 16*word_size + 32;
+  return param_area_start;
 }
 
 /* Assuming THIS_FRAME is a dummy, return the frame ID of that
-- 
1.9.3

[PATCH 3/4] S390: Re-arrange implementation of s390_return_value

[Andreas Arnez]

Move related logic in the implementation of s390_return_value closer
together.  This makes it easier to read and extend.

gdb/ChangeLog:

	* s390-linux-tdep.c (s390_return_value_convention): Remove
	function.  Inline its logic...
	(s390_return_value): ...here.  Instead, move the handling of the
	"register" return value convention...
	(s390_register_return_value): ...here.  New function.
---
 gdb/s390-linux-tdep.c | 158 +++++++++++++++++++++++---------------------------
 1 file changed, 73 insertions(+), 85 deletions(-)

diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index 3b7e4b6..bb9b28e 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -2708,110 +2708,98 @@ s390_frame_align (struct gdbarch *gdbarch, CORE_ADDR addr)
 }
 
 
-/* Function return value access.  */
+/* Helper for s390_return_value: Set or retrieve a function return
+   value if it resides in a register.  */
 
-static enum return_value_convention
-s390_return_value_convention (struct gdbarch *gdbarch, struct type *type)
+static void
+s390_register_return_value (struct gdbarch *gdbarch, struct type *type,
+			    struct regcache *regcache,
+			    gdb_byte *out, const gdb_byte *in)
 {
-  if (TYPE_LENGTH (type) > 8)
-    return RETURN_VALUE_STRUCT_CONVENTION;
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  int word_size = gdbarch_ptr_bit (gdbarch) / 8;
+  int length = TYPE_LENGTH (type);
+  int code = TYPE_CODE (type);
 
-  switch (TYPE_CODE (type))
+  if (code == TYPE_CODE_FLT || code == TYPE_CODE_DECFLOAT)
     {
-    case TYPE_CODE_STRUCT:
-    case TYPE_CODE_UNION:
-    case TYPE_CODE_ARRAY:
-    case TYPE_CODE_COMPLEX:
-      return RETURN_VALUE_STRUCT_CONVENTION;
-
-    default:
-      return RETURN_VALUE_REGISTER_CONVENTION;
+      /* Float-like value: left-aligned in f0.  */
+      if (in != NULL)
+	regcache_cooked_write_part (regcache, S390_F0_REGNUM,
+				    0, length, in);
+      else
+	regcache_cooked_read_part (regcache, S390_F0_REGNUM,
+				   0, length, out);
+    }
+  else if (length <= word_size)
+    {
+      /* Integer: zero- or sign-extended in r2.  */
+      if (out != NULL)
+	regcache_cooked_read_part (regcache, S390_R2_REGNUM,
+				   word_size - length, length, out);
+      else if (TYPE_UNSIGNED (type))
+	regcache_cooked_write_unsigned
+	  (regcache, S390_R2_REGNUM,
+	   extract_unsigned_integer (in, length, byte_order));
+      else
+	regcache_cooked_write_signed
+	  (regcache, S390_R2_REGNUM,
+	   extract_signed_integer (in, length, byte_order));
+    }
+  else if (length == 2 * word_size)
+    {
+      /* Double word: in r2 and r3.  */
+      if (in != NULL)
+	{
+	  regcache_cooked_write (regcache, S390_R2_REGNUM, in);
+	  regcache_cooked_write (regcache, S390_R3_REGNUM,
+				 in + word_size);
+	}
+      else
+	{
+	  regcache_cooked_read (regcache, S390_R2_REGNUM, out);
+	  regcache_cooked_read (regcache, S390_R3_REGNUM,
+				out + word_size);
+	}
     }
+  else
+    internal_error (__FILE__, __LINE__, _("invalid return type"));
 }
 
+
+/* Implement the 'return_value' gdbarch method.  */
+
 static enum return_value_convention
 s390_return_value (struct gdbarch *gdbarch, struct value *function,
 		   struct type *type, struct regcache *regcache,
 		   gdb_byte *out, const gdb_byte *in)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  int word_size = gdbarch_ptr_bit (gdbarch) / 8;
   enum return_value_convention rvc;
-  int length;
 
   type = check_typedef (type);
-  rvc = s390_return_value_convention (gdbarch, type);
-  length = TYPE_LENGTH (type);
 
-  if (in)
+  switch (TYPE_CODE (type))
     {
-      switch (rvc)
-	{
-	case RETURN_VALUE_REGISTER_CONVENTION:
-	  if (TYPE_CODE (type) == TYPE_CODE_FLT
-	      || TYPE_CODE (type) == TYPE_CODE_DECFLOAT)
-	    {
-	      /* When we store a single-precision value in an FP register,
-		 it occupies the leftmost bits.  */
-	      regcache_cooked_write_part (regcache, S390_F0_REGNUM,
-					  0, length, in);
-	    }
-	  else if (length <= word_size)
-	    {
-	      /* Integer arguments are always extended to word size.  */
-	      if (TYPE_UNSIGNED (type))
-		regcache_cooked_write_unsigned (regcache, S390_R2_REGNUM,
-			extract_unsigned_integer (in, length, byte_order));
-	      else
-		regcache_cooked_write_signed (regcache, S390_R2_REGNUM,
-			extract_signed_integer (in, length, byte_order));
-	    }
-	  else if (length == 2*word_size)
-	    {
-	      regcache_cooked_write (regcache, S390_R2_REGNUM, in);
-	      regcache_cooked_write (regcache, S390_R3_REGNUM, in + word_size);
-	    }
-	  else
-	    internal_error (__FILE__, __LINE__, _("invalid return type"));
-	  break;
-
-	case RETURN_VALUE_STRUCT_CONVENTION:
-	  error (_("Cannot set function return value."));
-	  break;
-	}
+    case TYPE_CODE_STRUCT:
+    case TYPE_CODE_UNION:
+    case TYPE_CODE_ARRAY:
+    case TYPE_CODE_COMPLEX:
+      rvc = RETURN_VALUE_STRUCT_CONVENTION;
+      break;
+    default:
+      rvc = TYPE_LENGTH (type) <= 8
+	? RETURN_VALUE_REGISTER_CONVENTION
+	: RETURN_VALUE_STRUCT_CONVENTION;
     }
-  else if (out)
-    {
-      switch (rvc)
-	{
-	case RETURN_VALUE_REGISTER_CONVENTION:
-	  if (TYPE_CODE (type) == TYPE_CODE_FLT
-	      || TYPE_CODE (type) == TYPE_CODE_DECFLOAT)
-	    {
-	      /* When we store a single-precision value in an FP register,
-		 it occupies the leftmost bits.  */
-	      regcache_cooked_read_part (regcache, S390_F0_REGNUM,
-					 0, length, out);
-	    }
-	  else if (length <= word_size)
-	    {
-	      /* Integer arguments occupy the rightmost bits.  */
-	      regcache_cooked_read_part (regcache, S390_R2_REGNUM,
-					 word_size - length, length, out);
-	    }
-	  else if (length == 2*word_size)
-	    {
-	      regcache_cooked_read (regcache, S390_R2_REGNUM, out);
-	      regcache_cooked_read (regcache, S390_R3_REGNUM, out + word_size);
-	    }
-	  else
-	    internal_error (__FILE__, __LINE__, _("invalid return type"));
-	  break;
 
-	case RETURN_VALUE_STRUCT_CONVENTION:
-	  error (_("Function return value unknown."));
-	  break;
-	}
+  if (in != NULL || out != NULL)
+    {
+      if (rvc == RETURN_VALUE_REGISTER_CONVENTION)
+	s390_register_return_value (gdbarch, type, regcache, out, in);
+      else if (in != NULL)
+	error (_("Cannot set function return value."));
+      else
+	error (_("Function return value unknown."));
     }
 
   return rvc;
-- 
1.9.3

[PATCH 4/4] S390: Vector ABI support

[Andreas Arnez]

With the S390 vector ABI, vector registers are used for passing vector
arguments and for returning a vector.  Support this ABI in inferior
function calls and when setting or retrieving a function's return
value.

gdb/ChangeLog:

	* s390-linux-tdep.c: Include "elf/s390.h" and "elf-bfd.h".
	(enum s390_vector_abi_kind): New enum.
	(struct gdbarch_tdep)<vector_abi>: New field.
	(s390_effective_inner_type): Add parameter min_size.  Stop
	unwrapping if the inner type is smaller than min_size.
	(s390_function_arg_float): Adjust call to
	s390_effective_inner_type.
	(s390_function_arg_vector): New function.
	(s390_function_arg_integer): Adjust comment.
	(struct s390_arg_state)<vr>: New field.
	(s390_handle_arg): Add parameter 'is_vararg'.  Pass vector
	arguments according to vector ABI when appropriate.
	(s390_push_dummy_call): Initialize the argument state's field
	'vr'.  Adjust calls to s390_handle_arg.
	(s390_register_return_value): Handle vector return values.
	(s390_return_value): Apply the "register" return value convention
	to a vector when appropriate.
	(s390_gdbarch_init): Initialize tdep->vector_abi.
	* NEWS: Announce S390 vector ABI support.
---
 gdb/NEWS              |   2 +
 gdb/s390-linux-tdep.c | 120 ++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 109 insertions(+), 13 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 62cbdcb..b2c92b1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -42,6 +42,8 @@
   (no "set sysroot" or "file" commands are required).  See "New remote
   packets" below.
 
+* GDB now supports the vector ABI on S/390 GNU/Linux targets.
+
 * Python Scripting
 
   ** gdb.Objfile objects have a new attribute "username",
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index bb9b28e..8eb8941 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -54,6 +54,8 @@
 #include "cli/cli-utils.h"
 #include <ctype.h>
 #include "elf/common.h"
+#include "elf/s390.h"
+#include "elf-bfd.h"
 
 #include "features/s390-linux32.c"
 #include "features/s390-linux32v1.c"
@@ -80,6 +82,12 @@ enum s390_abi_kind
   ABI_LINUX_ZSERIES
 };
 
+enum s390_vector_abi_kind
+{
+  S390_VECTOR_ABI_NONE,
+  S390_VECTOR_ABI_128
+};
+
 /* The tdep structure.  */
 
 struct gdbarch_tdep
@@ -87,6 +95,9 @@ struct gdbarch_tdep
   /* ABI version.  */
   enum s390_abi_kind abi;
 
+  /* Vector ABI.  */
+  enum s390_vector_abi_kind vector_abi;
+
   /* Pseudo register numbers.  */
   int gpr_full_regnum;
   int pc_regnum;
@@ -2395,14 +2406,24 @@ s390_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
      float x;
      struct { float x };
      struct { struct { float x; } x; };
-     struct { struct { struct { float x; } x; } x; };  */
+     struct { struct { struct { float x; } x; } x; };
+
+   However, if an inner type is smaller than MIN_SIZE, abort the
+   unwrapping.  */
 
 static struct type *
-s390_effective_inner_type (struct type *type)
+s390_effective_inner_type (struct type *type, unsigned int min_size)
 {
   while (TYPE_CODE (type) == TYPE_CODE_STRUCT
 	 && TYPE_NFIELDS (type) == 1)
-    type = check_typedef (TYPE_FIELD_TYPE (type, 0));
+    {
+      struct type *inner = check_typedef (TYPE_FIELD_TYPE (type, 0));
+
+      if (TYPE_LENGTH (inner) < min_size)
+	break;
+      type = inner;
+    }
+
   return type;
 }
 
@@ -2419,12 +2440,26 @@ s390_function_arg_float (struct type *type)
 
   /* A struct containing just a float or double is passed like a float
      or double.  */
-  type = s390_effective_inner_type (type);
+  type = s390_effective_inner_type (type, 0);
 
   return (TYPE_CODE (type) == TYPE_CODE_FLT
 	  || TYPE_CODE (type) == TYPE_CODE_DECFLOAT);
 }
 
+/* Return non-zero if TYPE should be passed like a vector.  */
+
+static int
+s390_function_arg_vector (struct type *type)
+{
+  if (TYPE_LENGTH (type) > 16)
+    return 0;
+
+  /* Structs containing just a vector are passed like a vector.  */
+  type = s390_effective_inner_type (type, TYPE_LENGTH (type));
+
+  return TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type);
+}
+
 /* Determine whether N is a power of two.  */
 
 static int
@@ -2434,8 +2469,8 @@ is_power_of_two (unsigned int n)
 }
 
 /* For an argument whose type is TYPE and which is not passed like a
-   float, return non-zero if it should be passed like "int" or "long
-   long".  */
+   float or vector, return non-zero if it should be passed like "int"
+   or "long long".  */
 
 static int
 s390_function_arg_integer (struct type *type)
@@ -2465,9 +2500,9 @@ struct s390_arg_state
   {
     /* Register cache, or NULL, if we are in "preparation mode".  */
     struct regcache *regcache;
-    /* Next available general/floating-point register for argument
-       passing.  */
-    int gr, fr;
+    /* Next available general/floating-point/vector register for
+       argument passing.  */
+    int gr, fr, vr;
     /* Current pointer to copy area (grows downwards).  */
     CORE_ADDR copy;
     /* Current pointer to parameter area (grows upwards).  */
@@ -2482,7 +2517,7 @@ struct s390_arg_state
 static void
 s390_handle_arg (struct s390_arg_state *as, struct value *arg,
 		 struct gdbarch_tdep *tdep, int word_size,
-		 enum bfd_endian byte_order)
+		 enum bfd_endian byte_order, int is_vararg)
 {
   struct type *type = check_typedef (value_type (arg));
   unsigned int length = TYPE_LENGTH (type);
@@ -2514,6 +2549,28 @@ s390_handle_arg (struct s390_arg_state *as, struct value *arg,
 			  length);
 	}
     }
+  else if (tdep->vector_abi == S390_VECTOR_ABI_128
+	   && s390_function_arg_vector (type))
+    {
+      static const char use_vr[] = {24, 26, 28, 30, 25, 27, 29, 31};
+
+      if (!is_vararg && as->vr < ARRAY_SIZE (use_vr))
+	{
+	  int regnum = S390_V24_REGNUM + use_vr[as->vr] - 24;
+
+	  if (write_mode)
+	    regcache_cooked_write_part (as->regcache, regnum,
+					0, length,
+					value_contents (arg));
+	  as->vr++;
+	}
+      else
+	{
+	  if (write_mode)
+	    write_memory (as->argp, value_contents (arg), length);
+	  as->argp = align_up (as->argp + length, word_size);
+	}
+    }
   else if (s390_function_arg_integer (type) && length <= word_size)
     {
       ULONGEST val;
@@ -2626,10 +2683,15 @@ s390_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   int i;
   struct s390_arg_state arg_state, arg_prep;
   CORE_ADDR param_area_start, new_sp;
+  struct type *ftype = check_typedef (value_type (function));
+
+  if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
+    ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
 
   arg_prep.copy = sp;
   arg_prep.gr = struct_return ? 3 : 2;
   arg_prep.fr = 0;
+  arg_prep.vr = 0;
   arg_prep.argp = 0;
   arg_prep.regcache = NULL;
 
@@ -2639,7 +2701,8 @@ s390_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* Update arg_state.copy with the start of the reference-to-copy area
      and arg_state.argp with the size of the parameter area.  */
   for (i = 0; i < nargs; i++)
-    s390_handle_arg (&arg_state, args[i], tdep, word_size, byte_order);
+    s390_handle_arg (&arg_state, args[i], tdep, word_size, byte_order,
+		     TYPE_VARARGS (ftype) && i >= TYPE_NFIELDS (ftype));
 
   param_area_start = align_down (arg_state.copy - arg_state.argp, 8);
 
@@ -2665,7 +2728,8 @@ s390_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   /* Write all parameters.  */
   for (i = 0; i < nargs; i++)
-    s390_handle_arg (&arg_state, args[i], tdep, word_size, byte_order);
+    s390_handle_arg (&arg_state, args[i], tdep, word_size, byte_order,
+		     TYPE_VARARGS (ftype) && i >= TYPE_NFIELDS (ftype));
 
   /* Store return PSWA.  In 31-bit mode, keep addressing mode bit.  */
   if (word_size == 4)
@@ -2731,6 +2795,16 @@ s390_register_return_value (struct gdbarch *gdbarch, struct type *type,
 	regcache_cooked_read_part (regcache, S390_F0_REGNUM,
 				   0, length, out);
     }
+  else if (code == TYPE_CODE_ARRAY)
+    {
+      /* Vector: left-aligned in v24.  */
+      if (in != NULL)
+	regcache_cooked_write_part (regcache, S390_V24_REGNUM,
+				    0, length, in);
+      else
+	regcache_cooked_read_part (regcache, S390_V24_REGNUM,
+				   0, length, out);
+    }
   else if (length <= word_size)
     {
       /* Integer: zero- or sign-extended in r2.  */
@@ -2782,10 +2856,15 @@ s390_return_value (struct gdbarch *gdbarch, struct value *function,
     {
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
-    case TYPE_CODE_ARRAY:
     case TYPE_CODE_COMPLEX:
       rvc = RETURN_VALUE_STRUCT_CONVENTION;
       break;
+    case TYPE_CODE_ARRAY:
+      rvc = (gdbarch_tdep (gdbarch)->vector_abi == S390_VECTOR_ABI_128
+	     && TYPE_LENGTH (type) <= 16 && TYPE_VECTOR (type))
+	? RETURN_VALUE_REGISTER_CONVENTION
+	: RETURN_VALUE_STRUCT_CONVENTION;
+      break;
     default:
       rvc = TYPE_LENGTH (type) <= 8
 	? RETURN_VALUE_REGISTER_CONVENTION
@@ -2901,6 +2980,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   struct gdbarch *gdbarch;
   struct gdbarch_tdep *tdep;
   int tdep_abi;
+  enum s390_vector_abi_kind vector_abi;
   int have_upper = 0;
   int have_linux_v1 = 0;
   int have_linux_v2 = 0;
@@ -3083,6 +3163,17 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 	}
     }
 
+  /* Determine vector ABI.  */
+  if (have_vx
+      && info.abfd != NULL
+      && info.abfd->format == bfd_object
+      && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour
+      && bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_GNU,
+				   Tag_GNU_S390_ABI_Vector) == 2)
+    vector_abi = S390_VECTOR_ABI_128;
+  else
+    vector_abi = S390_VECTOR_ABI_NONE;
+
   /* Find a candidate among extant architectures.  */
   for (arches = gdbarch_list_lookup_by_info (arches, &info);
        arches != NULL;
@@ -3093,6 +3184,8 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 	continue;
       if (tdep->abi != tdep_abi)
 	continue;
+      if (tdep->vector_abi != vector_abi)
+	continue;
       if ((tdep->gpr_full_regnum != -1) != have_upper)
 	continue;
       if (tdesc_data != NULL)
@@ -3103,6 +3196,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Otherwise create a new gdbarch for the specified machine type.  */
   tdep = XCNEW (struct gdbarch_tdep);
   tdep->abi = tdep_abi;
+  tdep->vector_abi = vector_abi;
   tdep->have_linux_v1 = have_linux_v1;
   tdep->have_linux_v2 = have_linux_v2;
   tdep->have_tdb = have_tdb;
-- 
1.9.3

Re: [PATCH 4/4] S390: Vector ABI support

[Andreas Arnez]

On Wed, Apr 22 2015, Andreas Arnez wrote:

> gdb/ChangeLog:
>
> 	* s390-linux-tdep.c: Include "elf/s390.h" and "elf-bfd.h".
> 	(enum s390_vector_abi_kind): New enum.
> 	(struct gdbarch_tdep)<vector_abi>: New field.
> 	(s390_effective_inner_type): Add parameter min_size.  Stop
> 	unwrapping if the inner type is smaller than min_size.
> 	(s390_function_arg_float): Adjust call to
> 	s390_effective_inner_type.
> 	(s390_function_arg_vector): New function.
> 	(s390_function_arg_integer): Adjust comment.
> 	(struct s390_arg_state)<vr>: New field.
> 	(s390_handle_arg): Add parameter 'is_vararg'.  Pass vector
> 	arguments according to vector ABI when appropriate.
> 	(s390_push_dummy_call): Initialize the argument state's field
> 	'vr'.  Adjust calls to s390_handle_arg.
> 	(s390_register_return_value): Handle vector return values.
> 	(s390_return_value): Apply the "register" return value convention
> 	to a vector when appropriate.
> 	(s390_gdbarch_init): Initialize tdep->vector_abi.
> 	* NEWS: Announce S390 vector ABI support.

Note that this patch depends on the definition of
Tag_GNU_S390_ABI_Vector.  This is added to include/elf/s390.h by the
following Binutils patch:

  https://sourceware.org/ml/binutils/2015-04/msg00316.html

--
Andreas

Re: [PATCH 1/4] S390: For zero, let is_power_of_two() return false

[Ulrich Weigand]

Andreas Arnez wrote:

> 	* s390-linux-tdep.c (is_power_of_two): Add comment.  Return
>           false if the argument is zero.

This is OK.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 2/4] S390: Restructure s390_push_dummy_call

[Ulrich Weigand]

Andreas Arnez wrote:

> 	* s390-linux-tdep.c
> 	(is_float_singleton): Remove function.  Move the "singleton" part
> 	of the logic...
> 	(s390_effective_inner_type): ...here.  New function.
> 	(is_float_like): Remove function.  Inline its logic...
> 	(s390_function_arg_float): ...here.
> 	(is_pointer_like, is_integer_like, is_struct_like): Remove
> 	functions.  Inline their logic...
> 	(s390_function_arg_integer): ...here.
> 	(s390_function_arg_pass_by_reference): Remove function.
> 	(extend_simple_arg): Remove function.
> 	(alignment_of): Remove function.
> 	(struct s390_arg_state): New structure.
> 	(s390_handle_arg): New function.
> 	(s390_push_dummy_call): Move parameter placement logic to the new
> 	function s390_handle_arg.  Call it for calculating the stack area
> 	sizes first, and again for actually writing the parameters.

This is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 3/4] S390: Re-arrange implementation of s390_return_value

[Ulrich Weigand]

Andreas Arnez wrote:

> 	* s390-linux-tdep.c (s390_return_value_convention): Remove
> 	function.  Inline its logic...
> 	(s390_return_value): ...here.  Instead, move the handling of the
> 	"register" return value convention...
> 	(s390_register_return_value): ...here.  New function.

This is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 4/4] S390: Vector ABI support

[Ulrich Weigand]

Andreas Arnez wrote:

> 	* s390-linux-tdep.c: Include "elf/s390.h" and "elf-bfd.h".
> 	(enum s390_vector_abi_kind): New enum.
> 	(struct gdbarch_tdep)<vector_abi>: New field.
> 	(s390_effective_inner_type): Add parameter min_size.  Stop
> 	unwrapping if the inner type is smaller than min_size.
> 	(s390_function_arg_float): Adjust call to
> 	s390_effective_inner_type.
> 	(s390_function_arg_vector): New function.
> 	(s390_function_arg_integer): Adjust comment.
> 	(struct s390_arg_state)<vr>: New field.
> 	(s390_handle_arg): Add parameter 'is_vararg'.  Pass vector
> 	arguments according to vector ABI when appropriate.
> 	(s390_push_dummy_call): Initialize the argument state's field
> 	'vr'.  Adjust calls to s390_handle_arg.
> 	(s390_register_return_value): Handle vector return values.
> 	(s390_return_value): Apply the "register" return value convention
> 	to a vector when appropriate.
> 	(s390_gdbarch_init): Initialize tdep->vector_abi.

> 	* NEWS: Announce S390 vector ABI support.

The NEWS change needs to be approved by Eli.


>  static void
>  s390_handle_arg (struct s390_arg_state *as, struct value *arg,
>  		 struct gdbarch_tdep *tdep, int word_size,
> -		 enum bfd_endian byte_order)
> +		 enum bfd_endian byte_order, int is_vararg)

Maybe "is_unnamed" would be better than "is_vararg".

> +  /* Determine vector ABI.  */
> +  if (have_vx
> +      && info.abfd != NULL
> +      && info.abfd->format == bfd_object
> +      && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour
> +      && bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_GNU,
> +				   Tag_GNU_S390_ABI_Vector) == 2)

I think this needs to be under an #ifdef HAVE_ELF, otherwise we'd
get compile errors when building GDB on a non-ELF host system.


Otherwise, this is OK once the binutils patch is committed.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 4/4] S390: Vector ABI support

[Eli Zaretskii]

> From: Andreas Arnez <arnez@linux.vnet.ibm.com>
> Cc: Sergio Durigan Junior <sergiodj@redhat.com>,        Ulrich Weigand <uweigand@de.ibm.com>
> Date: Wed, 22 Apr 2015 19:52:39 +0200
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 62cbdcb..b2c92b1 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -42,6 +42,8 @@
>    (no "set sysroot" or "file" commands are required).  See "New remote
>    packets" below.
>  
> +* GDB now supports the vector ABI on S/390 GNU/Linux targets.
> +

This is OK, thanks.

Re: [PATCH 4/4] S390: Vector ABI support

[Andreas Arnez]

On Thu, Apr 23 2015, Ulrich Weigand wrote:

> Andreas Arnez wrote:
>
> [...]
>
>>  static void
>>  s390_handle_arg (struct s390_arg_state *as, struct value *arg,
>>  		 struct gdbarch_tdep *tdep, int word_size,
>> -		 enum bfd_endian byte_order)
>> +		 enum bfd_endian byte_order, int is_vararg)
>
> Maybe "is_unnamed" would be better than "is_vararg".

OK, changed.

>> +  /* Determine vector ABI.  */
>> +  if (have_vx
>> +      && info.abfd != NULL
>> +      && info.abfd->format == bfd_object
>> +      && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour
>> +      && bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_GNU,
>> +				   Tag_GNU_S390_ABI_Vector) == 2)
>
> I think this needs to be under an #ifdef HAVE_ELF, otherwise we'd
> get compile errors when building GDB on a non-ELF host system.

OK, added the #ifdef.

> Otherwise, this is OK once the binutils patch is committed.

Thanks for reviewing!  Updated patch below.

-- >8 --
Subject: [PATCH v2] S390: Vector ABI support

With the S390 vector ABI, vector registers are used for passing vector
arguments and for returning a vector.  Support this ABI in inferior
function calls and when setting or retrieving a function's return
value.

gdb/ChangeLog:

	* s390-linux-tdep.c: Include "elf/s390.h" and "elf-bfd.h".
	(enum s390_vector_abi_kind): New enum.
	(struct gdbarch_tdep)<vector_abi>: New field.
	(s390_effective_inner_type): Add parameter min_size.  Stop
	unwrapping if the inner type is smaller than min_size.
	(s390_function_arg_float): Adjust call to
	s390_effective_inner_type.
	(s390_function_arg_vector): New function.
	(s390_function_arg_integer): Adjust comment.
	(struct s390_arg_state)<vr>: New field.
	(s390_handle_arg): Add parameter 'is_unnamed'.  Pass vector
	arguments according to vector ABI when appropriate.
	(s390_push_dummy_call): Initialize the argument state's field
	'vr'.  Adjust calls to s390_handle_arg.
	(s390_register_return_value): Handle vector return values.
	(s390_return_value): Apply the "register" return value convention
	to a vector when appropriate.
	(s390_gdbarch_init): Initialize tdep->vector_abi.
	* NEWS: Announce S390 vector ABI support.
---
 gdb/NEWS              |   2 +
 gdb/s390-linux-tdep.c | 121 ++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 110 insertions(+), 13 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 62cbdcb..b2c92b1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -42,6 +42,8 @@
   (no "set sysroot" or "file" commands are required).  See "New remote
   packets" below.
 
+* GDB now supports the vector ABI on S/390 GNU/Linux targets.
+
 * Python Scripting
 
   ** gdb.Objfile objects have a new attribute "username",
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index bb9b28e..edc0da1 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -54,6 +54,8 @@
 #include "cli/cli-utils.h"
 #include <ctype.h>
 #include "elf/common.h"
+#include "elf/s390.h"
+#include "elf-bfd.h"
 
 #include "features/s390-linux32.c"
 #include "features/s390-linux32v1.c"
@@ -80,6 +82,12 @@ enum s390_abi_kind
   ABI_LINUX_ZSERIES
 };
 
+enum s390_vector_abi_kind
+{
+  S390_VECTOR_ABI_NONE,
+  S390_VECTOR_ABI_128
+};
+
 /* The tdep structure.  */
 
 struct gdbarch_tdep
@@ -87,6 +95,9 @@ struct gdbarch_tdep
   /* ABI version.  */
   enum s390_abi_kind abi;
 
+  /* Vector ABI.  */
+  enum s390_vector_abi_kind vector_abi;
+
   /* Pseudo register numbers.  */
   int gpr_full_regnum;
   int pc_regnum;
@@ -2395,14 +2406,24 @@ s390_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
      float x;
      struct { float x };
      struct { struct { float x; } x; };
-     struct { struct { struct { float x; } x; } x; };  */
+     struct { struct { struct { float x; } x; } x; };
+
+   However, if an inner type is smaller than MIN_SIZE, abort the
+   unwrapping.  */
 
 static struct type *
-s390_effective_inner_type (struct type *type)
+s390_effective_inner_type (struct type *type, unsigned int min_size)
 {
   while (TYPE_CODE (type) == TYPE_CODE_STRUCT
 	 && TYPE_NFIELDS (type) == 1)
-    type = check_typedef (TYPE_FIELD_TYPE (type, 0));
+    {
+      struct type *inner = check_typedef (TYPE_FIELD_TYPE (type, 0));
+
+      if (TYPE_LENGTH (inner) < min_size)
+	break;
+      type = inner;
+    }
+
   return type;
 }
 
@@ -2419,12 +2440,26 @@ s390_function_arg_float (struct type *type)
 
   /* A struct containing just a float or double is passed like a float
      or double.  */
-  type = s390_effective_inner_type (type);
+  type = s390_effective_inner_type (type, 0);
 
   return (TYPE_CODE (type) == TYPE_CODE_FLT
 	  || TYPE_CODE (type) == TYPE_CODE_DECFLOAT);
 }
 
+/* Return non-zero if TYPE should be passed like a vector.  */
+
+static int
+s390_function_arg_vector (struct type *type)
+{
+  if (TYPE_LENGTH (type) > 16)
+    return 0;
+
+  /* Structs containing just a vector are passed like a vector.  */
+  type = s390_effective_inner_type (type, TYPE_LENGTH (type));
+
+  return TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type);
+}
+
 /* Determine whether N is a power of two.  */
 
 static int
@@ -2434,8 +2469,8 @@ is_power_of_two (unsigned int n)
 }
 
 /* For an argument whose type is TYPE and which is not passed like a
-   float, return non-zero if it should be passed like "int" or "long
-   long".  */
+   float or vector, return non-zero if it should be passed like "int"
+   or "long long".  */
 
 static int
 s390_function_arg_integer (struct type *type)
@@ -2465,9 +2500,9 @@ struct s390_arg_state
   {
     /* Register cache, or NULL, if we are in "preparation mode".  */
     struct regcache *regcache;
-    /* Next available general/floating-point register for argument
-       passing.  */
-    int gr, fr;
+    /* Next available general/floating-point/vector register for
+       argument passing.  */
+    int gr, fr, vr;
     /* Current pointer to copy area (grows downwards).  */
     CORE_ADDR copy;
     /* Current pointer to parameter area (grows upwards).  */
@@ -2482,7 +2517,7 @@ struct s390_arg_state
 static void
 s390_handle_arg (struct s390_arg_state *as, struct value *arg,
 		 struct gdbarch_tdep *tdep, int word_size,
-		 enum bfd_endian byte_order)
+		 enum bfd_endian byte_order, int is_unnamed)
 {
   struct type *type = check_typedef (value_type (arg));
   unsigned int length = TYPE_LENGTH (type);
@@ -2514,6 +2549,28 @@ s390_handle_arg (struct s390_arg_state *as, struct value *arg,
 			  length);
 	}
     }
+  else if (tdep->vector_abi == S390_VECTOR_ABI_128
+	   && s390_function_arg_vector (type))
+    {
+      static const char use_vr[] = {24, 26, 28, 30, 25, 27, 29, 31};
+
+      if (!is_unnamed && as->vr < ARRAY_SIZE (use_vr))
+	{
+	  int regnum = S390_V24_REGNUM + use_vr[as->vr] - 24;
+
+	  if (write_mode)
+	    regcache_cooked_write_part (as->regcache, regnum,
+					0, length,
+					value_contents (arg));
+	  as->vr++;
+	}
+      else
+	{
+	  if (write_mode)
+	    write_memory (as->argp, value_contents (arg), length);
+	  as->argp = align_up (as->argp + length, word_size);
+	}
+    }
   else if (s390_function_arg_integer (type) && length <= word_size)
     {
       ULONGEST val;
@@ -2626,10 +2683,15 @@ s390_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   int i;
   struct s390_arg_state arg_state, arg_prep;
   CORE_ADDR param_area_start, new_sp;
+  struct type *ftype = check_typedef (value_type (function));
+
+  if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
+    ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
 
   arg_prep.copy = sp;
   arg_prep.gr = struct_return ? 3 : 2;
   arg_prep.fr = 0;
+  arg_prep.vr = 0;
   arg_prep.argp = 0;
   arg_prep.regcache = NULL;
 
@@ -2639,7 +2701,8 @@ s390_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* Update arg_state.copy with the start of the reference-to-copy area
      and arg_state.argp with the size of the parameter area.  */
   for (i = 0; i < nargs; i++)
-    s390_handle_arg (&arg_state, args[i], tdep, word_size, byte_order);
+    s390_handle_arg (&arg_state, args[i], tdep, word_size, byte_order,
+		     TYPE_VARARGS (ftype) && i >= TYPE_NFIELDS (ftype));
 
   param_area_start = align_down (arg_state.copy - arg_state.argp, 8);
 
@@ -2665,7 +2728,8 @@ s390_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   /* Write all parameters.  */
   for (i = 0; i < nargs; i++)
-    s390_handle_arg (&arg_state, args[i], tdep, word_size, byte_order);
+    s390_handle_arg (&arg_state, args[i], tdep, word_size, byte_order,
+		     TYPE_VARARGS (ftype) && i >= TYPE_NFIELDS (ftype));
 
   /* Store return PSWA.  In 31-bit mode, keep addressing mode bit.  */
   if (word_size == 4)
@@ -2731,6 +2795,16 @@ s390_register_return_value (struct gdbarch *gdbarch, struct type *type,
 	regcache_cooked_read_part (regcache, S390_F0_REGNUM,
 				   0, length, out);
     }
+  else if (code == TYPE_CODE_ARRAY)
+    {
+      /* Vector: left-aligned in v24.  */
+      if (in != NULL)
+	regcache_cooked_write_part (regcache, S390_V24_REGNUM,
+				    0, length, in);
+      else
+	regcache_cooked_read_part (regcache, S390_V24_REGNUM,
+				   0, length, out);
+    }
   else if (length <= word_size)
     {
       /* Integer: zero- or sign-extended in r2.  */
@@ -2782,10 +2856,15 @@ s390_return_value (struct gdbarch *gdbarch, struct value *function,
     {
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
-    case TYPE_CODE_ARRAY:
     case TYPE_CODE_COMPLEX:
       rvc = RETURN_VALUE_STRUCT_CONVENTION;
       break;
+    case TYPE_CODE_ARRAY:
+      rvc = (gdbarch_tdep (gdbarch)->vector_abi == S390_VECTOR_ABI_128
+	     && TYPE_LENGTH (type) <= 16 && TYPE_VECTOR (type))
+	? RETURN_VALUE_REGISTER_CONVENTION
+	: RETURN_VALUE_STRUCT_CONVENTION;
+      break;
     default:
       rvc = TYPE_LENGTH (type) <= 8
 	? RETURN_VALUE_REGISTER_CONVENTION
@@ -2901,6 +2980,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   struct gdbarch *gdbarch;
   struct gdbarch_tdep *tdep;
   int tdep_abi;
+  enum s390_vector_abi_kind vector_abi;
   int have_upper = 0;
   int have_linux_v1 = 0;
   int have_linux_v2 = 0;
@@ -3083,6 +3163,18 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 	}
     }
 
+  /* Determine vector ABI.  */
+  vector_abi = S390_VECTOR_ABI_NONE;
+#ifdef HAVE_ELF
+  if (have_vx
+      && info.abfd != NULL
+      && info.abfd->format == bfd_object
+      && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour
+      && bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_GNU,
+				   Tag_GNU_S390_ABI_Vector) == 2)
+    vector_abi = S390_VECTOR_ABI_128;
+#endif
+
   /* Find a candidate among extant architectures.  */
   for (arches = gdbarch_list_lookup_by_info (arches, &info);
        arches != NULL;
@@ -3093,6 +3185,8 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 	continue;
       if (tdep->abi != tdep_abi)
 	continue;
+      if (tdep->vector_abi != vector_abi)
+	continue;
       if ((tdep->gpr_full_regnum != -1) != have_upper)
 	continue;
       if (tdesc_data != NULL)
@@ -3103,6 +3197,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Otherwise create a new gdbarch for the specified machine type.  */
   tdep = XCNEW (struct gdbarch_tdep);
   tdep->abi = tdep_abi;
+  tdep->vector_abi = vector_abi;
   tdep->have_linux_v1 = have_linux_v1;
   tdep->have_linux_v2 = have_linux_v2;
   tdep->have_tdb = have_tdb;
-- 
1.9.3

Re: [PATCH 0/4] S390: Vector ABI support

[Andreas Krebbel]

On 04/22/2015 07:44 PM, Andreas Arnez wrote:
> This small patch series introduces vector ABI support for S/390
> targets to GDB.  When an inferior uses the vector ABI, vector
> arguments are now passed correctly to inferior functions, and a
> function's return value of vector type can now be inspected and set
> correctly.
> 
> Patch #1 fixes a minor issue in is_power_of_two(), which is used by
> s390_push_dummy_call().  Patches #2 and #3 restructure the
> implementations of s390_push_dummy_call() and s390_return_value(),
> respectively, such that they become better readable and extensible,
> and such that they can more easily be extended towards the vector ABI.
> Patch #4 then adds the vector ABI support itself.
> 
> Andreas Arnez (4):
>   S390: For zero, let is_power_of_two() return false
>   S390: Restructure s390_push_dummy_call
>   S390: Re-arrange implementation of s390_return_value
>   S390: Vector ABI support
> 
>  gdb/NEWS              |   2 +
>  gdb/s390-linux-tdep.c | 725 +++++++++++++++++++++++++-------------------------
>  2 files changed, 361 insertions(+), 366 deletions(-)
> 
Applied. Thanks!

-Andreas-