From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25553 invoked by alias); 2 Dec 2005 19:13:43 -0000 Received: (qmail 25530 invoked by uid 22791); 2 Dec 2005 19:13:41 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 02 Dec 2005 19:13:39 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id jB2JDaKk001652; Fri, 2 Dec 2005 14:13:36 -0500 Received: from pobox.toronto.redhat.com (pobox.toronto.redhat.com [172.16.14.4]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id jB2JDVV29455; Fri, 2 Dec 2005 14:13:31 -0500 Received: from [172.16.14.227] (IDENT:5KW//GG8QJiiBzr9AQasX7h7S9Q4LBC8@topaz.toronto.redhat.com [172.16.14.227]) by pobox.toronto.redhat.com (8.12.8/8.12.8) with ESMTP id jB2JDQbs026304; Fri, 2 Dec 2005 14:13:31 -0500 Message-ID: <43909CD6.2090508@redhat.com> Date: Fri, 02 Dec 2005 19:13:00 -0000 From: Dave Brolley User-Agent: Mozilla Thunderbird 1.0.2 (X11/20050317) MIME-Version: 1.0 To: Hans-Peter Nilsson CC: cgen@sourceware.org, jimb@redhat.com Subject: Re: Unbreak CRIS sim: fix -gen-decode-insn-entry. And why the widened mem fetch? References: <200512020342.jB23gZ03025776@ignucius.se.axis.com> In-Reply-To: <200512020342.jB23gZ03025776@ignucius.se.axis.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact cgen-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: cgen-owner@sourceware.org X-SW-Source: 2005-q4/txt/msg00046.txt.bz2 Hans-Peter Nilsson wrote: >Dave Brolley's changes broke the CRIS sim in at least two >places: in CGEN (see further below) and by assuming that all >sims also have CGEN disassemblers (film at 11). Tsk tsk. >*Please* regen, cgen-maint-rebuild and test all CGEN simulators >when hacking general changes into the CGEN sim machinery. It's >not like there are too many... > > First of all, my apologies for breaking this. I did test a variety of SID and sim targets, but obviously not this one. >Anyway, I think I unbroke it (clean test-results), but I'm not >sure this is the best change. I don't like these diffs in the >generated sim (this one seemingly unrelated to the breakage, >maybe it's due to Jim Blandy's change on 2005-05-16): > > Looks like Jim has responded to this. >And this one: > >@@ -364,8 +364,14 @@ crisv10f_decode (SIM_CPU *current_cpu, I > case 11 : /* fall through */ > case 12 : /* fall through */ > case 13 : /* fall through */ >- case 15 : itype = CRISV10F_INSN_BCC_B; goto extract_sfmt_bcc_b; >- case 14 : itype = CRISV10F_INSN_BA_B; goto extract_sfmt_ba_b; >+ case 15 : >+ if ((base_insn & 0xf00) == 0x0) >+ { itype = CRISV10F_INSN_BCC_B; goto extract_sfmt_bcc_b; } >+ itype = CRISV10F_INSN_X_INVALID; goto extract_sfmt_empty; >+ case 14 : >+ if ((base_insn & 0xff00) == 0xe000) >+ { itype = CRISV10F_INSN_BA_B; goto extract_sfmt_ba_b; } >+ itype = CRISV10F_INSN_X_INVALID; goto extract_sfmt_empty; > default : itype = CRISV10F_INSN_X_INVALID; goto extract_sfmt_empty; > } > } > >Aren't those tests redundant when base_insn == entire_insn (so >to speak; there is no entire_insn as per the patch below)? >Should I have conditionalized the whole generated if-test? > > The switch generator stops when it has tested enough bits to resolve ambiguity among the defined insns, but since it wasn't going on to test the remaining bits, the decoder was recognizing invalid insns as valid. This additional test goes on to test all the fixed bits in each insn to ensure that they are all correct. It does appear that we could be smarter about generating the test, however. In this case the tests appear to be redundant. We could probably add some logic to test only the untested bits and to not generate the additional test at all if all of the bits have already been tested. >The change below mimics the test in >sim-decode.scm:-gen-decode-fn where entire_insn is conditionally >declared as a parameter to @prefix@_decode. Without it, I get >compilation errors for the undeclared entire_insn. >Ok to commit? > > This change looks ok to me. Please commit it. Thanks, Dave