From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4114 invoked by alias); 18 Feb 2011 12:17:32 -0000 Received: (qmail 4100 invoked by uid 22791); 18 Feb 2011 12:17:31 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,SPF_SOFTFAIL,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate4.uk.ibm.com (HELO mtagate4.uk.ibm.com) (194.196.100.164) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 18 Feb 2011 12:17:26 +0000 Received: from d06nrmr1507.portsmouth.uk.ibm.com (d06nrmr1507.portsmouth.uk.ibm.com [9.149.38.233]) by mtagate4.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p1ICHND5007134 for ; Fri, 18 Feb 2011 12:17:23 GMT Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by d06nrmr1507.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p1ICHT2b1749058 for ; Fri, 18 Feb 2011 12:17:29 GMT Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p1ICHMPn008249 for ; Fri, 18 Feb 2011 05:17:22 -0700 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with SMTP id p1ICHKWK008179; Fri, 18 Feb 2011 05:17:21 -0700 Message-Id: <201102181217.p1ICHKWK008179@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Fri, 18 Feb 2011 13:17:20 +0100 Subject: Re: [patch 2/3] Displaced stepping for 16-bit Thumb instructions To: yao@codesourcery.com (Yao Qi) Date: Fri, 18 Feb 2011 12:18:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: <4D5E0C48.7050002@codesourcery.com> from "Yao Qi" at Feb 18, 2011 02:06:00 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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: 2011-02/txt/msg00456.txt.bz2 Yao Qi wrote: > On 02/18/2011 03:22 AM, Ulrich Weigand wrote: > > I don't think this is the right way to go. You cannot have a mixture of > > ARM and Thumb instructions in a single modinsn block, and if you have > > Thumb instructions, they all need to be transfered in 16-bit chunks, > > even the 32-bit Thumb2 instructions, to get the endian conversion right. > > I don't have a mixture of ARM and Thumb instructions in a single modinsn > block. When displace stepping 16-bit instructions, modinsn[].insn.t is > used to record 16-bit instructions and all instructions in copy area are > 16-bit also. In 32-bit case, modinsn[].insn.a is used, and all > instructions in copy area are 32-bit. I'm not sure if I understood you correctly, but 32-bit Thumb2 instructions must be transfered in 2 16-bit chunks to get the endian conversion right, just as is done everywhere else in arm-tdep.c. But maybe this discussion can wait until we see the Thumb2 patch ... > The reason I propose a union here is to try to avoid too-many byte > operations during recording instructions and copying to copy area. The > union will waste some space in 16-bit instructions case, but IMO, it > doesn't matter too much. Are you talking about operations accessing the target, or computations done on host? If the former, the choice of data type for modinsns will not affect that at all. If the latter, I don't think there will be any measurable difference either way ... > I agree that we should a single flag for mode, and remove field size > from struct insn. > > The changes in `struct displaced_step_closure' is like this, > > - unsigned long modinsn[DISPLACED_MODIFIED_INSNS]; > + > + unsigned short flag; /* indicates the mode of instructions in > MODINSNS. */ > + union > + { > + unsigned long a; > + unsigned short t; > + }modinsns[DISPLACED_MODIFIED_INSNS]; > > Do you agree on this proposed data structure? We need an agreement on > this basic data structure before I start to write/change the rest of > patches. Well, I just don't see the point. The arm-tdep code usually does: dsc->modinsn[...] = Code generated from that will not be significantly different whether the underlying type of dsc->modinsn is short, int, or long; on some host platforms, having the destination type short will actually require extra conversion steps ... Because I don't see any actual performance benefit, I'd argue for keeping the source code simple. > I'll remove this chunk from my patch, and create another patch specific > to this 'magic number' problem separately. [snip] > Yes, we've already known the mode. We can use either > tdep->arm_breakpoint or tdep->thumb_breakpoint directly. [snip] > `if (!dsc->wrote_to_pc)' guard that we will not follow branch in this > case. However, since we've known the mode, we can adjust pc directly, > without bothering complicated arm_get_next_pc_raw. OK, thanks! Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com