From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30505 invoked by alias); 21 Feb 2011 06:31:41 -0000 Received: (qmail 30497 invoked by uid 22791); 21 Feb 2011 06:31:40 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 21 Feb 2011 06:31:36 +0000 Received: (qmail 30946 invoked from network); 21 Feb 2011 06:31:33 -0000 Received: from unknown (HELO ?0.0.0.0?) (yao@127.0.0.2) by mail.codesourcery.com with ESMTPA; 21 Feb 2011 06:31:33 -0000 Message-ID: <4D6206C4.7030405@codesourcery.com> Date: Mon, 21 Feb 2011 07:41:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.13) Gecko/20101208 Lightning/1.0b2 Thunderbird/3.1.7 MIME-Version: 1.0 To: Ulrich Weigand CC: gdb-patches@sourceware.org Subject: Re: [patch 2/3] Displaced stepping for 16-bit Thumb instructions References: <201102181217.p1ICHKWK008179@d06av02.portsmouth.uk.ibm.com> In-Reply-To: <201102181217.p1ICHKWK008179@d06av02.portsmouth.uk.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: 2011-02/txt/msg00524.txt.bz2 On 02/18/2011 08:17 PM, Ulrich Weigand wrote: > 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 ... > Yes, I did that in my Thumb-2 patch. /* Combine two 16 bit instructions to a 32-bit format. Some 32-bit Thumb instructions have the same encodings as ARM counterparts. */ #define COMBINE_16BIT_TO_32BIT_INSN(INSN1, INSN2) \ ((INSN1 << 16) | INSN2) >> > 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 ... > The latter one, I think. >> > 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 ... > Yes. If I understand you correctly, modinsn is a 'unsigned long' array. * ARM instruction occupies one slot with flag `ARM', * Thumb 16 bit instruction occupies one slot with flag `Thumb' * Thumb 32-bit instruction occupies *two* slots with flag `Thumb', That works, I think. I just recall one extra benefit of my original approach is about sharing some copy_* routines for Thumb 32-bit instructions and ARM instructions. In ARM manual, I noticed that some encodings of 32-bit Thumb-2 instructions are the same ARM counterparts (such as preload preload_reg, and svc_copro), so that their copy routines can be shared. In order to hide the difference of ARM instructions and 32-bit Thumb-2 instructions in these copy_* routines, two parts of 32-bit Thumb instructions are combined as a `32-bit instruction' via macro COMBINE_16BIT_TO_32BIT_INSN. Inside these copy_* routines, we don't have to worry about they are ARM instructions or 32-bit Thumb instructions. In my proposed approach, each instruction only occupies one slot, * ARM instruction, with instruction size `4-byte', * Thumb 16 bit instruction, with instruction size `2-byte' * Thumb 32-bit instruction, converted and with instruction size `4-byte', Do you think this benefit is strong enough to convince us to adapt to my original approach to `complicate source code' to some extent? -- Yao (齐尧)