This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [Patch] Fix ABI incompatibilities on s390x
- From: "Gerhard Tonn" <TON at de dot ibm dot com>
- To: gdb-patches at sources dot redhat dot com
- Date: Tue, 26 Nov 2002 10:23:32 +0100
- Subject: Re: [Patch] Fix ABI incompatibilities on s390x
- Sensitivity:
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 <jimb@redhat.com> 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 <ton@de.ibm.com> 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 <ton@de.ibm.com>
>
> * 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.