From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24537 invoked by alias); 1 Feb 2003 01:27:59 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 24530 invoked from network); 1 Feb 2003 01:27:59 -0000 Received: from unknown (HELO mx1.redhat.com) (172.16.49.200) by 172.16.49.205 with SMTP; 1 Feb 2003 01:27:59 -0000 Received: from int-mx2.corp.redhat.com (nat-pool-rdu-dmz.redhat.com [172.16.52.200] (may be forged)) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id h111Rxf13812 for ; Fri, 31 Jan 2003 20:27:59 -0500 Received: from potter.sfbay.redhat.com (potter.sfbay.redhat.com [172.16.27.15]) by int-mx2.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h111Rwn13524; Fri, 31 Jan 2003 20:27:58 -0500 Received: from redhat.com (reddwarf.sfbay.redhat.com [172.16.24.50]) by potter.sfbay.redhat.com (8.11.6/8.11.6) with ESMTP id h111RvQ21310; Fri, 31 Jan 2003 17:27:58 -0800 Message-ID: <3E3B229D.67F2E805@redhat.com> Date: Sat, 01 Feb 2003 01:27:00 -0000 From: Michael Snyder Organization: Red Hat, Inc. X-Accept-Language: en MIME-Version: 1.0 To: Kazu Hirata CC: gdb-patches@sources.redhat.com Subject: Re: Unreviewed patches References: <20030130.010258.25912168.kazu@cs.umass.edu> <3E39E145.A5B01D17@redhat.com> <20030130.234517.112286807.kazu@cs.umass.edu> Content-Type: multipart/mixed; boundary="------------7E5DFAB1FA9BC8524448AA44" X-SW-Source: 2003-02/txt/msg00006.txt.bz2 This is a multi-part message in MIME format. --------------7E5DFAB1FA9BC8524448AA44 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Content-length: 2521 Kazu Hirata wrote: > > Hi Michael, > > Thanks for reviewing the patches. > > > > [RFA] sim/h8300/h8300.c: Fix the handling of bxor. > > > http://sources.redhat.com/ml/gdb-patches/2003-01/msg00328.html > > > > For this one, I find the expression "!!(ea & m)" a bit obscure. > > How about "(ea & m) != 0)"? > > Sure. Does this mean the patch is OK to apply with your sugestion? Yes. > > > [RFA] sim/h8300/compile.c: Fix the handling of extu.w. > > > http://sources.redhat.com/ml/gdb-patches/2002-12/msg00685.html > > > > When you say "an 8-bit wide register that does not exist", > > do you mean "that isn't simulated"? It seems to me that > > the more correct solution is that breg[] is not big enough. > > It ought to be at least 24 words, if not 32. > > Well, H8/300H and H8S have 8 32-bit registers, from er0 to er7. Each > of them can be treated as two 16-bit wide registers. That is, er0 is > split into e0 and r0. Furthermore, the lower half, r0, can be divided > into two parts, r0h and r0l. The organization of one register looks > like so: > > 31 24 23 16 15 8 7 0 > +--------+--------+--------+--------+ > | N/A | N/A | r0h | r0l | > +--------+--------+--------+--------+ > | e0 | r0 | > +-----------------+-----------------+ > | er0 | > +-----------------------------------+ > > Now, "extu.w r0" zero-extends the value of r0l and stores the result > to r0. The H8 simulator without my patch correctly handles this. A > problem arises when the simulator sees "extu.w e0". The correct > behavior is to zero-extend the lower half of e0 and store the result > into e0. However, GET_B_REG in the simulator has no way to refer to > the lower half of the e0. The real hardware does not even have this, > so I decided to take e0 and clear the upper half of e0, which also > works for "extu.w r0". Just take r0 and clear the upper half of r0. Yes, thanks. My problem is this: your patch uses a host-order sign-extend to simulate a target-order sign-extend. If the host and target have different byte orders, you lose. That's probably why the simulator uses breg[] to fetch bytes, instead of using wreg and masking. I suggest that it would be comparatively easy to extend the breg[] array so that it would cover at least the first three bytes in the register (and possibly all four, just because it's no extra effort). Something like the attached. Then the code that references breg[] does not need to change. --------------7E5DFAB1FA9BC8524448AA44 Content-Type: text/plain; charset=us-ascii; name="tmp.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="tmp.diff" Content-length: 884 *************** compile (SIM_DESC sd, int pc) *** 777,784 **** } ! static unsigned char *breg[18]; ! static unsigned short *wreg[18]; static unsigned int *lreg[18]; #define GET_B_REG(X) *(breg[X]) --- 777,784 ---- } ! static unsigned char *breg[32]; ! static unsigned short *wreg[16]; static unsigned int *lreg[18]; #define GET_B_REG(X) *(breg[X]) *************** init_pointers (SIM_DESC sd) *** 1065,1077 **** while (p < e) { if (*p == 0x22) - { breg[i] = p; - } if (*p == 0x33) - { breg[i + 8] = p; ! } p++; } --- 1065,1077 ---- while (p < e) { if (*p == 0x22) breg[i] = p; if (*p == 0x33) breg[i + 8] = p; ! if (*p == 0x11) ! breg[i + 16] = p; ! if (*p == 0x00) ! breg[i + 24] = p; p++; } --------------7E5DFAB1FA9BC8524448AA44--