public inbox for cgen@sourceware.org
 help / color / mirror / Atom feed
From: Dave Brolley <brolley@redhat.com>
To: Hans-Peter Nilsson <hans-peter.nilsson@axis.com>
Cc: cgen@sourceware.org, jimb@redhat.com
Subject: Re: Unbreak CRIS sim: fix -gen-decode-insn-entry.  And why the widened  mem fetch?
Date: Fri, 02 Dec 2005 19:13:00 -0000	[thread overview]
Message-ID: <43909CD6.2090508@redhat.com> (raw)
In-Reply-To: <200512020342.jB23gZ03025776@ignucius.se.axis.com>

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

  parent reply	other threads:[~2005-12-02 19:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-12-02  3:42 Hans-Peter Nilsson
2005-12-02  7:06 ` Jim Blandy
2005-12-02 15:42   ` Hans-Peter Nilsson
2005-12-02 19:13 ` Dave Brolley [this message]
2005-12-05  1:31   ` Hans-Peter Nilsson
2005-12-05 17:11     ` Dave Brolley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=43909CD6.2090508@redhat.com \
    --to=brolley@redhat.com \
    --cc=cgen@sourceware.org \
    --cc=hans-peter.nilsson@axis.com \
    --cc=jimb@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).