Re: Unbreak CRIS sim: fix -gen-decode-insn-entry. And why the widened mem fetch?

[Dave Brolley <brolley at redhat dot 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