From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15202 invoked by alias); 1 Mar 2013 02:55:46 -0000 Received: (qmail 15194 invoked by uid 22791); 1 Mar 2013 02:55:45 -0000 X-SWARE-Spam-Status: No, hits=-6.5 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_EG X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 01 Mar 2013 02:55:36 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r212tSqu015264 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 28 Feb 2013 21:55:28 -0500 Received: from psique (ovpn-113-23.phx2.redhat.com [10.3.113.23]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r212tPLp009066 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 28 Feb 2013 21:55:26 -0500 From: Sergio Durigan Junior To: Tiago =?utf-8?Q?St=C3=BCrmer?= Daitx Cc: gdb-patches@sourceware.org, emachado@br.ibm.com, Ulrich.Weigand@de.ibm.com Subject: Re: [PATCH] Fix complex argument handling in ppc64 dummy function call References: <512f9ad7.MB2qjZDogDfnrfE3%tdaitx@linux.vnet.ibm.com> X-URL: http://www.redhat.com Date: Fri, 01 Mar 2013 02:55:00 -0000 In-Reply-To: <512f9ad7.MB2qjZDogDfnrfE3%tdaitx@linux.vnet.ibm.com> ("Tiago =?utf-8?Q?St=C3=BCrmer?= Daitx"'s message of "Thu, 28 Feb 2013 11:58:47 -0600") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2013-03/txt/msg00001.txt.bz2 On Thursday, February 28 2013, Tiago St=C3=BCrmer Daitx wrote: > Handle complex arguments for dummy function call on PPC64. I refactored t= he > code to extract the float logic into a function to reuse it for complex=20 > arguments as well - ABI defines that complex arguments are to be handled = as=20 > if 2 float arguments were given. Thanks for the patch, Tiago. I have only formatting comments. > gdb/ChangeLog > 2013-02-05 Tiago Sturmer Daitx Don't forget to update the date if your patch is accepted :-). > * ppc-sysv-tdep.c (ppc64_sysv_abi_push_dummy_call): Handle complex=20 > arguments. > * ppc-sysv-tdep.c (ppc64_sysv_abi_push_float): New functions to handle=20 > float arguments. You don't need multiple entries for the same file. The order of the functions is reverse (it's not a big issue, but I try to write my ChangeLog entries following the order of the modifications in the patch). You can also just write "New function." for the `ppc64_sysv_abi_push_float'. So your ChangeLog would be something like: * ppc-sysv-tdep.c (ppc64_sysv_abi_push_float): New function. (ppc64_sysv_abi_push_dummy_call): Handle complex arguments. > diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c > index 0ffeab9..690d65d 100644 > --- a/gdb/ppc-sysv-tdep.c > +++ b/gdb/ppc-sysv-tdep.c > @@ -1101,6 +1101,76 @@ convert_code_addr_to_desc_addr (CORE_ADDR code_add= r, CORE_ADDR *desc_addr) > return 1; > } >=20=20 > +static void > +ppc64_sysv_abi_push_float (struct gdbarch *gdbarch, struct regcache *reg= cache, > + struct gdbarch_tdep *tdep, struct type *type,=20 > + const bfd_byte *val, int freg, int greg, > + CORE_ADDR gparam) > +{ Since this is a new function, it needs a comment describing it. Oh, and there should be a blank line between the comment and the function declaration :-). > + /* Floats and Doubles go in f1 .. f13. They also consume a left a= ligned=20 > + GREG, and can end up in memory. */ > + if (freg <=3D 13) > + { > + struct type *regtype; > + regtype =3D register_type (gdbarch, tdep->ppc_fp0_regnum + freg); I know you've just made a copy-and-paste here, but it's a standard to put a blank line between the declaration of the variable(s) and the actual code. > @@ -1276,36 +1302,55 @@ ppc64_sysv_abi_push_dummy_call (struct gdbarch *g= dbarch, > && (gdbarch_long_double_format (gdbarch) > =3D=3D floatformats_ibm_long_double)) > { > - /* IBM long double stored in two doublewords of the > - parameter save area and corresponding registers. */ > if (write_pass) > - { > - if (!tdep->soft_float && freg <=3D 13) > - { > - regcache_cooked_write (regcache, > - tdep->ppc_fp0_regnum + freg, > - val); > - if (freg <=3D 12) > - regcache_cooked_write (regcache, > - tdep->ppc_fp0_regnum + freg + 1, > - val + 8); > - } > - if (greg <=3D 10) > - { > - regcache_cooked_write (regcache, > - tdep->ppc_gp0_regnum + greg, > - val); > - if (greg <=3D 9) > - regcache_cooked_write (regcache, > - tdep->ppc_gp0_regnum + greg + 1, > - val + 8); > - } > - write_memory (gparam, val, TYPE_LENGTH (type)); > - } > + ppc64_sysv_abi_push_float (gdbarch, regcache, tdep, type,=20 Spurious space at the end of the line. > + val, freg, greg, gparam); [...] > + else if (TYPE_CODE (type) =3D=3D TYPE_CODE_COMPLEX > + && (TYPE_LENGTH (type) =3D=3D 8 || TYPE_LENGTH (type) =3D= =3D 16)) > + { > + int i; Same comment as above: blank line between variable declaration and code. > + for (i =3D 0; i < 2; i++) > + { > + if (write_pass) > + { > + struct type *target_type; Likewise. > + target_type =3D check_typedef (TYPE_TARGET_TYPE (type)); > + ppc64_sysv_abi_push_float (gdbarch, regcache, tdep,=20 Spurious space at the end of the line. > + target_type, val + i *=20 Likewise. > + TYPE_LENGTH (target_type), > + freg, greg, gparam); > + } > + freg++; > + greg++; > + /* Always consume parameter stack space. */ > + gparam =3D align_up(gparam + 8, tdep->wordsize); Space between function name and open parenthesis. > + } > + } > + else if (TYPE_CODE (type) =3D=3D TYPE_CODE_COMPLEX > + && TYPE_LENGTH (type) =3D=3D 32 > + && (gdbarch_long_double_format (gdbarch) > + =3D=3D floatformats_ibm_long_double)) > + { > + int i; > + for (i =3D 0; i < 2; i++) > + { > + struct type *target_type; > + target_type =3D check_typedef (TYPE_TARGET_TYPE (type)); > + if (write_pass) This "if" is indented only with spaces (not using GNU style). > + ppc64_sysv_abi_push_float (gdbarch, regcache, tdep,=20 > + target_type, val + i *=20 Spurious space at the end of both lines. > + TYPE_LENGTH (target_type), > + freg, greg, gparam); > + freg +=3D 2; > + greg +=3D 2; > + gparam =3D align_up (gparam + TYPE_LENGTH (target_type= ), > + tdep->wordsize); > + } > + } > else if (TYPE_CODE (type) =3D=3D TYPE_CODE_DECFLOAT > && TYPE_LENGTH (type) <=3D 8) > { Well, I guess that's it. Sorry for the nitpicking, and thanks a lot for the code refactoring :-). --=20 Sergio