From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1655 invoked by alias); 2 Feb 2003 11:36:53 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 1647 invoked from network); 2 Feb 2003 11:36:51 -0000 Received: from unknown (HELO moutvdom.kundenserver.de) (212.227.126.251) by 172.16.49.205 with SMTP; 2 Feb 2003 11:36:51 -0000 Received: from [212.227.126.224] (helo=mrvdomng.kundenserver.de) by moutvdom.kundenserver.de with esmtp (Exim 3.35 #1) id 18fIQa-0007Vv-00; Sun, 02 Feb 2003 12:36:48 +0100 Received: from [80.138.131.91] (helo=tau) by mrvdomng.kundenserver.de with smtp (Exim 3.35 #1) id 18fIQZ-00050N-00; Sun, 02 Feb 2003 12:36:47 +0100 From: Gerhard Tonn To: Jim Blandy Subject: Re: [Patch] Fix ABI incompatibilities on s390x Date: Sun, 02 Feb 2003 11:36:00 -0000 Content-Type: text/plain; charset="iso-8859-1" Cc: ton@de.ibm.com, gdb-patches@sources.redhat.com MIME-Version: 1.0 Message-Id: <03020212430000.00722@tau> Content-Transfer-Encoding: 8bit X-SW-Source: 2003-02/txt/msg00044.txt.bz2 > I'm afraid I have the same concerns about this patch as I did about > the first one. > It seems that there are two main differences between the s390 and > s390x ABI's: > - the s390 registers are larger. > - the s390x does not have a special DOUBLE_ARG class of arguments that > it handles specially. > Most of the first sort of difference you can account for using > REGISTER_SIZE, but there is one case that cannot be handled this way. > You handle this by testing GDB_TARGET_IS_ESAME. > Since there are several bits of code that know about the existence of > DOUBLE_ARGs (i.e., we have to exclude DOUBLE_ARGs from the other > classes of arguments), you handle each of those by testing > GDB_TARGET_IS_ESAME. > Please handle the first case by defining a function: > static int > is_power_of_two (unsigned int n) > { > return ((n & (n-1)) == 0); > } > and then saying ! (is_power_of_two (length) && length <= REGISTER_SIZE) > in pass_by_copy_ref. > Please handle the second case by explicitly calling is_double_arg from > is_simple_arg, and then change is_double_arg as follows: > static int > is_double_arg (struct type *type) > { > unsigned length = TYPE_LENGTH (type); > > /* The s390x ABI doesn't handle DOUBLE_ARGS specially. */ > if (GDB_TARGET_IS_ESAME) > return 0; > > return ((is_integer_like (type) > || is_struct_like (type)) > && length == 8); > } Good idea. Thanks for your time and your help. BTW., I have run the test suite after each of my changes. The results look pretty good. Gerhard 2003-02-02 Gerhard Tonn * s390-tdep.c (is_simple_arg): Substitute hardcoded register size by macro REGISTER_SIZE and check for double args. Long double checks are dropped, since they are not supported by s390 and s390x. (is_power_of_two): New. (pass_by_copy_ref): Call is_power_of_two() instead of explicit checks of length. Long double checks are dropped, since they are not supported by s390 and s390x. (is_double_arg): Return 0 for s390x architecture. (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 2003-02-01 19:13:31.000000000 +0000 +++ s390-tdep.c 2003-02-02 08:40:40.000000000 +0000 @@ -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,13 +1371,18 @@ /* 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_float_like (type) && length == 16)); + || (is_struct_like (type) && !is_double_arg (type))); } +static int +is_power_of_two (unsigned int n) +{ + return ((n & (n - 1)) == 0); +} + /* Return non-zero if TYPE should be passed as a pointer to a copy, zero otherwise. TYPE must be a SIMPLE_ARG, as recognized by `is_simple_arg'. */ @@ -1384,8 +1391,8 @@ { unsigned length = TYPE_LENGTH (type); - return ((is_struct_like (type) && length != 1 && length != 2 && length != 4) - || (is_float_like (type) && length == 16)); + return (is_struct_like (type) + && !(is_power_of_two (length) && length <= REGISTER_SIZE)); } @@ -1417,6 +1424,10 @@ { unsigned length = TYPE_LENGTH (type); + /* The s390x ABI doesn't handle DOUBLE_ARGS specially. */ + if (GDB_TARGET_IS_ESAME) + return 0; + return ((is_integer_like (type) || is_struct_like (type)) && length == 8); @@ -1542,9 +1553,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 +1576,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 +1613,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 +1629,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 +1639,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 +1662,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