From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16711 invoked by alias); 7 Dec 2005 20:58:20 -0000 Received: (qmail 16681 invoked by uid 22791); 7 Dec 2005 20:58:16 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 07 Dec 2005 20:58:14 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id jB7KwCeJ017182 for ; Wed, 7 Dec 2005 15:58:12 -0500 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id jB7KwCV12651 for ; Wed, 7 Dec 2005 15:58:12 -0500 Received: from localhost.localdomain (vpn50-83.rdu.redhat.com [172.16.50.83]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id jB7KwCOp018343 for ; Wed, 7 Dec 2005 15:58:12 -0500 Received: from ironwood.lan (ironwood.lan [192.168.64.8]) by localhost.localdomain (8.12.11/8.12.10) with ESMTP id jB7Kw6KN005614 for ; Wed, 7 Dec 2005 13:58:07 -0700 Date: Thu, 08 Dec 2005 04:42:00 -0000 From: Kevin Buettner To: gdb-patches@sources.redhat.com Subject: Re: [PATCH] add 'rs6000_in_function_epilogue_p()' (Revised) Message-ID: <20051207135806.100c60bb@ironwood.lan> In-Reply-To: <200512051100.51617.pgilliam@us.ibm.com> References: <200511301225.56802.pgilliam@us.ibm.com> <200512021120.22263.pgilliam@us.ibm.com> <20051202133250.10d687bd@ironwood.lan> <200512051100.51617.pgilliam@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2005-12/txt/msg00139.txt.bz2 On Mon, 5 Dec 2005 11:00:51 -0800 Paul Gilliam wrote: > I made the changes to address Mark's concerns. You missed adding spaces around at least one of the operators. The one that I noticed was the `-' in: + for (scan pc =3D pc-PPC INSN SIZE; Also, regarding the following bit: + =A0int opcode =A0=3D (insn>>26) & 0x03f; + =A0int sd =A0 =A0 =A0=3D (insn>>21) & 0x01f; + =A0int a =A0 =A0 =A0 =3D (insn>>16) & 0x01f; + =A0/* =A0b =A0 =A0 =A0 =3D (insn>>11) & 0x01f =A0*/ + =A0int subcode =3D (insn>> 1) & 0x3ff; + =A0/* =A0rc =A0 =A0 =A0=3D =A0insn =A0 =A0 =A0& 0x001 =A0*/ I like the use of indentation to line things up, but our coding standard is to remove extraneous spaces. (Because that's what GNU indent would do. Yeah, I agree it's lame...) Ah, I just noticed that you missed the spaces around the >> operators too. > But because of the interest this has drawn, I thought I better ask > again before I commited the patch. Thanks for asking again. It was indeed useful to reread the other comments. Could I ask you to make the following modifications to your patch? 1) Abort a forward scan (and return 0) if any control transfer instruction other than blr is found. 2) Abort a backward scan (and return 0) if any control transfer instruction is found. 3) Place a hard limit on the number of instructions scanned both forwards and backwards. Epilogues are typically not very big and scanning to the ends of a large function starting at the middle seems rather wasteful. Kevin