From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18025 invoked by alias); 26 Nov 2002 09:24:11 -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 18017 invoked from network); 26 Nov 2002 09:24:09 -0000 Received: from unknown (HELO d12lmsgate-5.de.ibm.com) (194.196.100.238) by sources.redhat.com with SMTP; 26 Nov 2002 09:24:09 -0000 Received: from d12relay02.de.ibm.com (d12relay02.de.ibm.com [9.165.215.23]) by d12lmsgate-5.de.ibm.com (8.12.3/8.12.3) with ESMTP id gAQ9O7sF060950 for ; Tue, 26 Nov 2002 10:24:07 +0100 Received: from d12ml015.de.ibm.com (d12ml015_cs0 [9.165.223.40]) by d12relay02.de.ibm.com (8.12.3/NCO/VER6.4) with ESMTP id gAQ9O06S157120 for ; Tue, 26 Nov 2002 10:24:00 +0100 Importance: Normal Sensitivity: Subject: Re: [Patch] Fix ABI incompatibilities on s390x To: gdb-patches@sources.redhat.com Message-ID: From: "Gerhard Tonn" Date: Tue, 26 Nov 2002 01:24:00 -0000 MIME-Version: 1.0 Content-type: text/plain; charset=us-ascii X-SW-Source: 2002-11/txt/msg00639.txt.bz2 Hi, 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. Regards / Mit freundlichen Gruessen Gerhard Gerhard Tonn, Linux for eServer Development, +(49)-7031-16-4716, Lotus Notes: ton@ibmde, Internet: ton@de.ibm.com Jim Blandy on 21.11.2002 06:16:41 To: Gerhard Tonn/Germany/IBM@IBMDE cc: gdb-patches@sources.redhat.com Subject: Re: [Patch] Fix ABI incompatibilities on s390x Gerhard Tonn writes: > attached is a patch that fixes some ABI incompatibilities for the s390x > architecture. Hi, thanks for working on this! First of all, this patch incudes a number of different fixes, all mixed together: - struct return fixes - push_arguments fixes - get_frame_info fixes These each need to be submitted as separate patches. (I understand that IBM has a special agreement regarding how copyright assignment is handled; perhaps the patches could be reviewed separately, but then assigned as a single patch.) > 2002-11-19 Gerhard Tonn > > * s390-tdep.c (s390_push_arguments, s390_get_frame_info): > Fix the s390x ELF ABI implementation bugs. If our Changelog entries all said, "Fixed bugs in this code", they wouldn't be very useful. :) We have had very bad experiences with trying to make a single function serve two different ABI's in the past. (mips_push_arguments seems to have been cleaned up since I last looked; it was a real mess.) So while using things like 'REGISTER_SIZE' and 'S390_STACK_PARAMETER_ALIGNMENT' are clearly a good idea, for the sake of the other stuff I'd like to see a separate 's390x_push_arguments' function written that does things right for the s390x's ABI. The helper functions like `is_simple_arg' should be duplicated, rather than testing GDB_TARGET_IS_ESAME. I understand this may seem pedantic --- after all, it's just a few minor differences, why duplicate all that code? --- but I think history will back me up. Some other comments: > + || (is_struct_like (type) && (GDB_TARGET_IS_ESAME ? 1 : length != 8)) That idiom shows up a lot in this patch --- wouldn't it be more legible to write: || (is_struct_like (type) && (GDB_TARGET_IS_ESAME || length != 8)) ? Of course, a lot of these will go away entirely when the s390/s390x functions are split.