Re: Simulator: base_insn and insn in decode.c

[Ronald Hecht <ronald dot hecht at uni-rostock dot de>]

Dave Brolley wrote:

> 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.


I completely agree with that. This actually works.

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


Yes, but im my case entire instruction starts at bit 23 with the 
instruction opcode. Bit 15 is the start of an 16 Bit operand. In the 
case of a 16 Bit instruction the opcode starts at bit 15 and then comes 
a 8 bit immediate. The decode functionality beneath decodes this correctly:

<snip>
extract_sfmt_lda:
 {
   const IDESC *idesc = &proc8bf_insn_data[itype];
   CGEN_INSN_INT insn = entire_insn;
#define FLD(f) abuf->fields.sfmt_lda.f
   UINT f_uimm16;

   f_uimm16 = EXTRACT_MSB0_UINT (insn, 24, 8, 16);

 /* Record the fields for the semantic handler.  */
 FLD (f_uimm16) = f_uimm16;
</snip>

or

<snip>
extract_sfmt_ldc:
 {
   const IDESC *idesc = &proc8bf_insn_data[itype];
   CGEN_INSN_INT insn = entire_insn;
#define FLD(f) abuf->fields.sfmt_ldc.f
   UINT f_uimm8;

   f_uimm8 = EXTRACT_MSB0_UINT (insn, 16, 8, 8);

 /* Record the fields for the semantic handler.  */
 FLD (f_uimm8) = f_uimm8;
</snip>

So this is correct, but the if clauses in the case statement should use 
the correct bits to double-check. As I mentioned, when I remove the if 
clauses, everything works fine. The simulator works as expected.

> 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

I agreee. The test should be against base_insn. But then it still seems 
to be double-checking against the same, or not?

By the way, I'm preparing a website holding the stuff about the 
processor and the tools port. It was pretty difficult to find some 
useful information about how to port the GNU tools starting from a very 
simple example. So I decided to put this information on this website.

http://www-md.e-technik.uni-rostock.de/lehre/vlsi_i/proc8/

Ronald