Re: Simulator: base_insn and insn in decode.c

[Dave Brolley <brolley at redhat dot com>]

Ronald Hecht wrote:

> Hello again,
>
> i found another problem in decode. c. As I have variable instruction 
> size, decode.c does not handle base_insn and insn in the right way. 
> The file looks like this:
>
> const IDESC *
> proc8bf_decode (SIM_CPU *current_cpu, IADDR pc,
>              CGEN_INSN_INT base_insn, CGEN_INSN_INT entire_insn,
>              ARGBUF *abuf)
> {
>  /* Result of decoder.  */
>  PROC8BF_INSN_TYPE itype;
>
>  {
>    CGEN_INSN_INT insn = base_insn;
>
>    {
>      unsigned int val = (((insn >> 0) & (63 << 0)));
>      switch (val)
>      {
>      case 0 :
>        if ((entire_insn & 0xff) == 0x0)
>          { itype = PROC8BF_INSN_NOP; goto extract_sfmt_nop; }
>        itype = PROC8BF_INSN_X_INVALID; goto extract_sfmt_empty;

The generated decoder expects base_insn to be the first base-insn-size 
bits of the insn shifted as far right as possible. What you have 
specified is correct and the (((insn >> 0) & (63 << 0))) test us 
correct. It is looking at the low order 6 bits of the base_insn which 
are the only significant ones (according to your cpu file.

The generated decoder expects entire_insn to the all of the insn bits, 
also shifted as far right as possible.

The test around the goto is intended to make sure that the untested 
base_insn bits are as expected. It needs to be there, otherwise, invalid 
insns get recognized as valid ones. However, it would seem that the test 
should be against base_insn and not against entire_insn. I'll have a 
look at some other cgen simulators, including those which use SID to 
make sure that this assertion holds water. It probably hasn't come up 
until now, since most sims I have seen are able to pass the same value 
for base_insn and entire_insn.

Dave