Re: [patch][rfa]: Decoding insns > 32 bits in length

[Dave Brolley <brolley at redhat dot com>]

Frank Ch. Eigler wrote:

>brolley wrote:
>
>: [...]
>: The patch to opcodes/cgen-ibld.in fixes a problem which was affecting
>: the fr30 port and an internal port I'm working on. The patch is an
>: obvious improvement, since 'x' is now at least initialized and decodes
>: correctly for fr30 and the internal port.
>: [...]
>
>Take this up with nickc; he added that "#if 0" about a month ago.
>
ok - I will. I suspect that he only meant to #if out the "int big_p = 
....." since it is unused.

>: The patch to cgen/utils-gen.scm corrects the length argument in the
>: generated calls to EXTRACT_MSB0_UINT in <target>-decode.cxx for SID
>: cpus. [...]  As far as I can tell, this does not affect any other
>: ports.
>
>I wouldn't expect this sort of systemic bug to have gone undetected
>this long.  Can you spell out a failing case?
>
This would only affect architectures which resort to the base-insn + 
(additional insn words) model for decoding insns. Do we have any of 
these? The fr30 has 48 bit insns, but is not a SID simulator. The case 
which bit me was like this:

The architecture has 16 bit insns which may be followed by 1 or two 
additional immediate words (16 bits each). In the case that the insn was 
followed by two additional immediates, the decoder was generating

  word_1 = current_cpu->GETIMEMUSI (pc, pc + 2);
    f_dest_addr_2 = (0|(EXTRACT_MSB0_UINT (word_1, 32, 16, 32) << 0));
    f_source_addr_1 = (0|(EXTRACT_MSB0_UINT (word_1, 32, 0, 16) << 0));

Note the length for the extract of 'f_dest_addr_2' which should be 16, 
but is computed as 32. This is because (in utils-gen.scm), end==32 and 
word-end==32. Therefore word-length gets substituted as the number of 
bits to extract.

The computed value is supposed to be the number of bits to extract. Now 
I see that while the patch I made is sufficient for this particular 
case, the general solution which handles fields which overlap word 
boundaries is in the attached patch which replaces my previous patch.

Dave

Index: utils-gen.scm
===================================================================
RCS file: /cvs/src/src/cgen/utils-gen.scm,v
retrieving revision 1.5
diff -c -p -r1.5 utils-gen.scm
*** utils-gen.scm	2001/01/06 12:11:09	1.5
--- utils-gen.scm	2001/11/06 18:29:26
***************
*** 118,124 ****
  			   unsigned? lsb0?)
    ; ??? lsb0?
    (let ((word-end (+ word-start word-length))
! 	(end (+ start length)))
      (string-append "("
  		   "EXTRACT_"
  		   (if (current-arch-insn-lsb0?) "LSB0" "MSB0")
--- 118,125 ----
  			   unsigned? lsb0?)
    ; ??? lsb0?
    (let ((word-end (+ word-start word-length))
! 	(end (+ start length))
! 	(base (if (< start word-start) word-start start)))
      (string-append "("
  		   "EXTRACT_"
  		   (if (current-arch-insn-lsb0?) "LSB0" "MSB0")
***************
*** 137,144 ****
  				       (- start word-start)))
  		   ", "
  		   (number->string (if (< end word-end)
! 				       (- word-end end)
! 				       word-length))
  		   ") << "
  		   (number->string (if (> end word-end)
  				       (- end word-end)
--- 138,145 ----
  				       (- start word-start)))
  		   ", "
  		   (number->string (if (< end word-end)
! 				       (- end base)
! 				       (- word-end base)))
  		   ") << "
  		   (number->string (if (> end word-end)
  				       (- end word-end)