From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5633 invoked by alias); 23 Dec 2003 01:23:20 -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 5626 invoked from network); 23 Dec 2003 01:23:19 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 23 Dec 2003 01:23:19 -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 hBN1NIA24242 for ; Mon, 22 Dec 2003 20:23:18 -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 hBN1NGM26226; Mon, 22 Dec 2003 20:23:16 -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 hBN1NFO26721; Mon, 22 Dec 2003 17:23:16 -0800 Message-ID: <3FE79903.5090704@redhat.com> Date: Tue, 23 Dec 2003 01:23:00 -0000 From: Michael Snyder Organization: Red Hat, Inc. User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030624 MIME-Version: 1.0 To: Joern Rennecke CC: andrew.stubbs@superh.com, joern.rennecke@superh.com, gdb-patches@sources.redhat.com Subject: Re: [RFA] sh-sim: restructure expand_opcode. References: <200312201305.hBKD5MHZ001588@meolyon.local> In-Reply-To: <200312201305.hBKD5MHZ001588@meolyon.local> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-12/txt/msg00469.txt.bz2 Joern Rennecke wrote: > 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. */ OK, added. > 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; No problem. >>! 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. Since I don't see any such patterns currently, let's save this for a follow-on patch. No use holding up perfectly good work on the basis of a hypothetical future need. > > >>--- 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. */ OK. >>+ /* 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? Let's come back to this after the holidays. Speaking of which, by the time you reply, I will be gone for the holidays, and not return until the new year. If we have acheived convergence on this piece of code, would you mind checking it in? Cheers, Michael