From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17080 invoked by alias); 20 Dec 2003 12:52:11 -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 17068 invoked from network); 20 Dec 2003 12:52:09 -0000 Received: from unknown (HELO host1.kbnet.net) (81.26.107.2) by sources.redhat.com with SMTP; 20 Dec 2003 12:52:09 -0000 Received: from fairadsl.co.uk (mailhost.fairadsl.co.uk [81.26.107.14]) by host1.kbnet.net (8.11.6/8.11.6) with ESMTP id hBKCq8e24396; Sat, 20 Dec 2003 12:52:08 GMT Received: from meolyon.local [81.26.104.192] by fairadsl.co.uk with ESMTP (SMTPD32-7.15) id A676186E0124; Sat, 20 Dec 2003 12:54:14 +0000 Received: from meolyon.local (localhost [127.0.0.1]) by meolyon.local (8.12.6/8.12.6/SuSE Linux 0.6) with ESMTP id hBKD5MM4001589; Sat, 20 Dec 2003 13:05:22 GMT Received: (from amylaar@localhost) by meolyon.local (8.12.6/8.12.6/Submit) id hBKD5MHZ001588; Sat, 20 Dec 2003 13:05:22 GMT From: Joern Rennecke Message-Id: <200312201305.hBKD5MHZ001588@meolyon.local> Subject: Re: [RFA] sh-sim: restructure expand_opcode. To: msnyder@redhat.com (Michael Snyder) Date: Sat, 20 Dec 2003 12:52:00 -0000 Cc: amylaar@fairadsl.co.uk (Joern Rennecke), andrew.stubbs@superh.com, joern.rennecke@superh.com, gdb-patches@sources.redhat.com In-Reply-To: <3FE27B0A.7080207@redhat.com> from "Michael Snyder" at Dec 18, 2003 08:14:02 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2003-12/txt/msg00454.txt.bz2 Michael Snyder wrote: > > This is a multi-part message in MIME format. > --------------070904020002040701070808 > Content-Type: text/plain; charset=us-ascii; format=flowed > Content-Transfer-Encoding: 7bit > > Hi Joern, > > This supercedes my last patch. All I'm doing in this one is > expand_opcode, implementing some of the changes we discussed > (to pave the way for the sh4a patch to come). It's much > simpler, and fewer lines of code. > > It now accepts either 2 or 4 bits at a time (except for ones > and zeroes, which accept an arbitrary sequence). The sub- > cases such as 'N' and 'x' are moved out to the outer switch > (so its flatter and simpler). The "shift" parameter is gone > (which required that the movx/movy expansion be moved into > the post-processing pass in ppi_moves). "ddd0" becomes > "eeee" (even numbered register), and I don't check it at runtime. > > The XY/xy stuff will come in the next patch. > > Michael > > > --------------070904020002040701070808 > Content-Type: text/plain; > name="joern" > Content-Transfer-Encoding: 7bit > Content-Disposition: inline; > filename="joern" > > 2003-12-18 Michael Snyder > > * gencode.c (expand_opcode): Simplify and reorganize. > Eliminate "shift" parameter. Eliminate "4 bits at a time" > assumption. Flatten switch statement to a single level. > Add "eeee" token for even-numbered registers. > (bton): Delete. > (fsca): Use "eeee" token. > (ppi_moves): Rename to "expand_ppi_movxy". Do the ddt > [movx/movy] expansion here, as well as the ppi expansion. > (gensim_caselist): Accept 'eeee' along with 'nnnn'. Yes, I agree that this patch improves the code. Just a few more suggestions / corrections: > ! /* Take an opcode, expand all varying fields in it out and fill all the > ! right entries in 'table' with the opcode index. */ > > static void > ! expand_opcode (val, i, s) This never adhered to the GNU coding standards on comments (unless it preceded the clause on documenting arguments :-) . This seems a good opportunity to fix this. Something like: /* Take an opcode, expand all varying fields in it out and fill all the right entries in 'table' with the opcode index I. VAL is the opcode computed so far, which needs left-shifting by the number of bits that are still to be processed. S is a string describing the remaining bits. */ > + int j = 0, m = 0; > + > switch (s[0]) > { > ! default: > ! fprintf (stderr, "expand_opcode: illegal char '%c'\n", s[0]); > ! exit (1); > case '0': > case '1': > ! /* Consume an arbitrary number of ones and zeros. */ > ! do { > ! j = (j << 1) + (s[m++] - '0'); > ! } while (s[m] == '0' || s[m] == '1'); > ! expand_opcode ((val << m) | j, i, s + m); > break; I think this is simpler: case '0': case '1': /* Consume an arbitrary number of ones and zeros. */ do { val = (val << 1) + (*s++ - '0'); } while (s[m] == '0' || s[m] == '1'); expand_opcode (val, i, s); break; > ! case 'x': /* xx -- 2-way fork */ > ! /* Cross-breeding with movy moved to separate function. */ > ! for (j = 0; j < 4; j += 2) > ! expand_opcode ((val << 2) | j, i, s + 2); > ! break; > ! case 'y': /* yy -- two-way fork */ > ! for (j = 0; j < 2; j++) > ! expand_opcode ((val << 2) | j, i, s + 2); > ! break; .... > case 'X': > case 'a': > ! /* XX, aa -- two-way fork */ > ! for (j = 0; j < 4; j += 2) > ! expand_opcode ((val << 2) | j, i, s + 2); > break; > case 'Y': > case 'A': > ! /* YY, AA -- two-way fork */ > ! for (j = 0; j < 2; j++) > ! expand_opcode ((val << 2) | j, i, s + 2); > break; You can do this with x0 / 0y / X0 / a0 / 0Y / 0A patterns now, have the '0' be handled by the generic constant code, and thus only need a trivial case of a one-letter, two-way fork. gensim_caselist of course will need to have the increments for these letters changed from two to one. > --- 2108,2136 ---- > processing insns (ppi) for code 0xf800 (ppi nopx nopy). Copy the > latter tag to represent all combinations of ppi with ddt. */ > static void > ! expand_ppi_movxy () You change what the function does; this should be reflected in the comment: /* Table already contais all the switch case tags for separate movx and movy insns, and the switch case tag for processing parallel processing insns (ppi) for code 0xf800 (ppi nopx nopy). Fill in all the movx/movy combinations, thus completing the coverage of the 16-bit opcode double data transfer (ddt) insns, and copy the ppi tag to represent all combinations of ppi with ddt. */ > + /* Ignore illegal nopy */ > + if ((m & 7) == 0 && m != 0) > + continue; The legal nopy has already been covered by expand_opcode, so we may skip it too. /* Ignore non-movy.w patterns. */ if ((m & 3) == 0 && m != 0) continue; About these new mov[xy].l instructions... do they actually have four opcodes each that do the same thing?