Re: [Patch] Fix ABI incompatibilities on s390x

[Andrew Cagney <ac131313@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 71 bytes --]

Kevin,
Since you're playing with an s390, can you look at this?
Andrew

[-- Attachment #2: mailbox-message://ac131313@movemail/fsf/gdb/patches#25697139 --]
[-- Type: message/rfc822, Size: 9545 bytes --]

From: Gerhard Tonn <ton@de.ibm.com>
To: gdb-patches@sources.redhat.com
Subject: Re: [Patch] Fix ABI incompatibilities on s390x
Date: Thu, 23 Jan 2003 11:59:40 +0100
Message-ID: <03012312571000.14253@fmtc804>


"Gerhard Tonn" <TON@de.ibm.com> writes:
>> I don't think that it is worthwhile to duplicate the functions. The only
>> difference between s390 and s390x is really the REGISTER_SIZE. I will
>> rework the patch so that this becomes more clear. I will also address the
>> other issues.

>Certainly, if two functions can be made identical by the appropriate
>use of REGISTER_SIZE, then there's no reason to duplicate them.
>However, I thought I had seen another sort of conditionality which is
>definitely of the sort we are trying to avoid.

>I'll watch for your new patches.  Thanks again for your work.

Please find below the revised patch. The number of floating point registers is
another difference between s390 and s390x besides the register size.

As soon as you have approved the patch  we will start the copyright assignment . I
will let you know any progress.

2003-01-23  Gerhard Tonn  <ton@de.ibm.com>

	* s390-tdep.c (is_simple_arg): Substitute hardcoded register size by
	  macro REGISTER_SIZE and consider s390x register size for structs.
	(pass_by_copy_ref): Likewise.
	(is_double_arg): Likewise.
	(s390_push_arguments): Substitute hardcoded register size by macro
	  REGISTER_SIZE. Consider ABI when returning values of type struct.
	  Substitute hardcoded number of floating point registers by macro
	  S390_NUM_FP_PARAMETER_REGISTERS. Substitute hardcoded
	  stack parameter alignment value by macro
	  S390_STACK_PARAMETER_ALIGNMENT. Substitute hardcoded stack frame
	  overhead value by macro S390_STACK_FRAME_OVERHEAD.

--- s390-tdep.c.bak	Tue Nov 26 11:49:54 2002
+++ s390-tdep.c	Tue Nov 26 12:17:05 2002
@@ -94,7 +94,9 @@
 #define S390X_SIGREGS_FP0_OFFSET      (216)
 #define S390_UC_MCONTEXT_OFFSET    (256)
 #define S390X_UC_MCONTEXT_OFFSET   (344)
-#define S390_STACK_FRAME_OVERHEAD  (GDB_TARGET_IS_ESAME ? 160:96)
+#define S390_STACK_FRAME_OVERHEAD  16*REGISTER_SIZE+32
+#define S390_STACK_PARAMETER_ALIGNMENT  REGISTER_SIZE
+#define S390_NUM_FP_PARAMETER_REGISTERS (GDB_TARGET_IS_ESAME ? 4:2)
 #define S390_SIGNAL_FRAMESIZE  (GDB_TARGET_IS_ESAME ? 160:96)
 #define s390_NR_sigreturn          119
 #define s390_NR_rt_sigreturn       173
@@ -1369,9 +1371,9 @@
 
   /* This is almost a direct translation of the ABI's language, except
      that we have to exclude 8-byte structs; those are DOUBLE_ARGs.  */
-  return ((is_integer_like (type) && length <= 4)
+  return ((is_integer_like (type) && length <= REGISTER_SIZE)
           || is_pointer_like (type)
-          || (is_struct_like (type) && length != 8)
+          || (is_struct_like (type) && (GDB_TARGET_IS_ESAME || length != 8))
           || (is_float_like (type) && length == 16));
 }
 
@@ -1384,7 +1386,8 @@
 {
   unsigned length = TYPE_LENGTH (type);
 
-  return ((is_struct_like (type) && length != 1 && length != 2 && length != 4)
+  return ((is_struct_like (type) && length != 1 && length != 2 && length != 4
+           && (!GDB_TARGET_IS_ESAME || length != 8))
           || (is_float_like (type) && length == 16));
 }
 
@@ -1419,7 +1422,7 @@
 
   return ((is_integer_like (type)
            || is_struct_like (type))
-          && length == 8);
+          && !GDB_TARGET_IS_ESAME && length == 8);
 }
 
 
@@ -1542,9 +1545,9 @@
         
         sp = round_down (sp, alignment_of (type));
 
-        /* SIMPLE_ARG values get extended to 32 bits.  Assume every
-           argument is.  */
-        if (length < 4) length = 4;
+        /* SIMPLE_ARG values get extended to REGISTER_SIZE bytes. 
+           Assume every argument is.  */
+        if (length < REGISTER_SIZE) length = REGISTER_SIZE;
         sp -= length;
       }
   }
@@ -1565,13 +1568,17 @@
     int gr = 2;
     CORE_ADDR starg = sp;
 
+    /* A struct is returned using general register 2 */
+    if (struct_return)
+      gr++;
+
     for (i = 0; i < nargs; i++)
       {
         struct value *arg = args[i];
         struct type *type = VALUE_TYPE (arg);
         
         if (is_double_or_float (type)
-            && fr <= 2)
+            && fr <= S390_NUM_FP_PARAMETER_REGISTERS * 2 - 2)
           {
             /* When we store a single-precision value in an FP register,
                it occupies the leftmost bits.  */
@@ -1598,7 +1605,7 @@
             write_register_gen (S390_GP0_REGNUM + gr,
                                 VALUE_CONTENTS (arg));
             write_register_gen (S390_GP0_REGNUM + gr + 1,
-                                VALUE_CONTENTS (arg) + 4);
+                                VALUE_CONTENTS (arg) + REGISTER_SIZE);
             gr += 2;
           }
         else
@@ -1614,9 +1621,9 @@
 
             if (is_simple_arg (type))
               {
-                /* Simple args are always either extended to 32 bits,
-                   or pointers.  */
-                starg = round_up (starg, 4);
+                /* Simple args are always extended to 
+                   REGISTER_SIZE bytes.  */
+                starg = round_up (starg, REGISTER_SIZE);
 
                 /* Do we need to pass a pointer to our copy of this
                    argument?  */
@@ -1624,18 +1631,19 @@
                   write_memory_signed_integer (starg, pointer_size,
                                                copy_addr[i]);
                 else
-                  /* Simple args are always extended to 32 bits.  */
-                  write_memory_signed_integer (starg, 4,
+                  /* Simple args are always extended to 
+                     REGISTER_SIZE bytes. */
+                  write_memory_signed_integer (starg, REGISTER_SIZE,
                                                extend_simple_arg (arg));
-                starg += 4;
+                starg += REGISTER_SIZE;
               }
             else
               {
                 /* You'd think we should say:
                    starg = round_up (starg, alignment_of (type));
                    Unfortunately, GCC seems to simply align the stack on
-                   a four-byte boundary, even when passing doubles.  */
-                starg = round_up (starg, 4);
+                   a four/eight-byte boundary, even when passing doubles. */
+                starg = round_up (starg, S390_STACK_PARAMETER_ALIGNMENT);
                 write_memory (starg, VALUE_CONTENTS (arg), length);
                 starg += length;
               }
@@ -1646,7 +1654,7 @@
   /* 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 -= 96;
+  sp -= S390_STACK_FRAME_OVERHEAD;
 
   /* Write the back chain pointer into the first word of the stack
      frame.  This will help us get backtraces from within functions



--
Regards,
Gerhard