Re: [patch][rfa] -opcode-slots: Handling of short insns

[Dave Brolley <brolley at redhat dot com>]

Attached the wrong file.....

Dave Brolley wrote:

> Hi,
>
> This patch fixes a problem I ran across while working on an internal 
> port. Incorrect opcodes and masks were sometimes being generated for 
> the short insns of variable length ISAs because of two problems in 
> -opcode-slots:
>
> 1) The test of bit positions against the insn-length was off by one. 
> Thjs could lead to incorrect opcode bits being generated.
>
> 2) While the 'compute' function correctly generated zeroes for the 
> extra bits when computing 'opcode', it was also generating zeroes for 
> these bits when computing 'opcode-mask', thus rendering these bits 
> irrelevent. This could lead to the generation of numerous unreachable 
> cases in the generated decoder switch.
>
> The patch corrects the comparison of bit position against the 
> insn-length and also allows the caller of 'compute' to specify the 
> default bit value which should be generated bits beyond the length of 
> a short insn. This allows 0 to be specified when computing 'opcode' 
> and 1 to be specified when computing opcode-mask. The patch also 
> changes some of the logit calls to print values in hex which is more 
> appropriate when examining bitmasks
>
> This patch corrects the problem encountered with my internal port. I 
> know of no other port which is affected by this bug. I have tested it 
> against frv and xstormy16 and verified no changes to the generated 
> decoders.
>
> OK to commit?
>
> Dave
>
> ------------------------------------------------------------------------
>
> 2004-01-28  Dave Brolley  <brolley@redhat.com>
>
> 	* decode.scm (-opcode-slots): For short insns, generate 'opcode' with
> 	zeroes in the extra bit positions and generate 'opcode-mask' with ones
> 	in the extra bit positions.
>
>  
>
> ------------------------------------------------------------------------
>
> cgen/ChangeLog:
> 2004-01-26  Dave Brolley  <brolley@redhat.com>
>
> 	* decode.scm (-opcode-slots): For short insns, generate 'opcode' with
> 	zeroes in the extra bit positions and generate 'opcode-mask' with ones
> 	in the extra bit positions.
>
> cgen/cpu/ChangeLog.RedHat:
> 2004-01-26  Dave Brolley  <brolley@redhat.com>
>
> 	* mep.opc (OPTION_MASK): Remove dangerous whitspace following a
> 	backslash intended as a line continuation.
>
> sid/main/dynamic/ChangeLog.RedHat:
> 2004-01-26  Dave Brolley  <brolley@redhat.com>
>
> 	* mepCfg.h (allocate_timer, configure_timer): New methods of MepBoardCfg.
> 	(set_opt_timer_channel_bitw): New method of MepBoardCfg.
> 	(timer_channel_bitw): New member of MepBoardCfg.
> 	* mepCfg.cxx (MepBoardCfg): Initialize timer_channel_bitw.
> 	(set_dmem_bank_num): Allow dmem_bank_num to be set to zero.
> 	(map_imem_dmem): Correct fmem_base_address for case of no dmem.
> 	Initialize dmem_base[0].
> 	(MepBoardCfg::write_config): Call configure_timer. Move setup of timer
> 	interrupt pins to configure_timer. Handle case where dmem_bank_num is
> 	zero.
> 	(add_timer): Move allocation, scheduling and connection of timer to
> 	allocate_timer and configure_timer.
> 	(allocate_timer, configure_timer): New methods of MepBoardCfg.
> 	(configure_dmac): Use timer_channel_bitw.
> 	(set_opt_timer_channel_bitw): New method of MepBoardCfg.
> 	* mainDynamic.cxx (usage): Document --timer-channel-bitw.
> 	(option_num): Add opt_timer_channel_bitw.
> 	(long_options): Add timer-channel-bitw.
> 	(main): Handle opt_timer_channel_bitw.
> 	* commonCfg.h (set_opt_timer_channel_bitw): New prototype.
>
> utils/mep/ChangeLog:
> 2004-01-26  Dave Brolley  <brolley@redhat.com>
>
> 	* mepcfgtool.c (generate_simulator_script): Generate
> 	--timer-channel-bitw
>
>  

Index: cgen/decode.scm
===================================================================
RCS file: /cvs/src/src/cgen/decode.scm,v
retrieving revision 1.9
diff -c -p -r1.9 decode.scm
*** cgen/decode.scm	21 Oct 2003 16:42:00 -0000	1.9
--- cgen/decode.scm	28 Jan 2004 18:15:11 -0000
***************
*** 1,5 ****
  ; Application independent decoder support.
! ; Copyright (C) 2000 Red Hat, Inc.
  ; This file is part of CGEN.
  ;
  ; This file provides utilities for building instruction set decoders.
--- 1,5 ----
  ; Application independent decoder support.
! ; Copyright (C) 2000, 2004 Red Hat, Inc.
  ; This file is part of CGEN.
  ;
  ; This file provides utilities for building instruction set decoders.
***************
*** 433,462 ****
    (letrec ((opcode (insn-value insn))
  	   (insn-len (insn-base-mask-length insn))
  	   (decode-len (length bitnums))
! 	   (compute (lambda (val insn-len decode-len bl)
  		      ;(display (list val insn-len decode-len bl)) (newline)
  		      ; Oh My God.  This isn't tail recursive.
  		      (if (null? bl)
  			  0
! 			  (+ (if (> (car bl) insn-len)
! 				 0
! 				 (if (bit-set? val
! 					       (if lsb0?
! 						   (car bl)
! 						   (- insn-len (car bl) 1)))
! 				     (integer-expt 2 (- (length bl) 1))
! 				     0))
! 			     (compute val insn-len decode-len (cdr bl)))))))
!     (let* ((opcode (compute (insn-value insn) insn-len decode-len bitnums))
! 	   (opcode-mask (compute (insn-base-mask insn) insn-len decode-len bitnums))
  	   (indices (missing-bit-indices opcode-mask (- (integer-expt 2 decode-len) 1))))
        (logit 3 "insn =" (obj:name insn)
! 	     " insn-value=" (insn-value insn)
! 	     " insn-base-mask=" (insn-base-mask insn)
  	     " insn-len=" insn-len
  	     " decode-len=" decode-len
! 	     " opcode=" opcode
! 	     " opcode-mask=" opcode-mask
  	     " indices=" indices "\n")
        (map (lambda (index) (+ opcode index)) indices)))
  )
--- 433,462 ----
    (letrec ((opcode (insn-value insn))
  	   (insn-len (insn-base-mask-length insn))
  	   (decode-len (length bitnums))
! 	   (compute (lambda (val insn-len decode-len bl default)
  		      ;(display (list val insn-len decode-len bl)) (newline)
  		      ; Oh My God.  This isn't tail recursive.
  		      (if (null? bl)
  			  0
! 			  (+ (if (or (and (>= (car bl) insn-len) (= default 1))
! 				     (and (< (car bl) insn-len)
! 					  (bit-set? val
! 						    (if lsb0?
! 							(car bl)
! 							(- insn-len (car bl) 1)))))
! 				 (integer-expt 2 (- (length bl) 1))
! 				 0)
! 			     (compute val insn-len decode-len (cdr bl) default))))))
!     (let* ((opcode (compute (insn-value insn) insn-len decode-len bitnums 0))
! 	   (opcode-mask (compute (insn-base-mask insn) insn-len decode-len bitnums 1))
  	   (indices (missing-bit-indices opcode-mask (- (integer-expt 2 decode-len) 1))))
        (logit 3 "insn =" (obj:name insn)
! 	     " insn-value=" (number->hex (insn-value insn))
! 	     " insn-base-mask=" (number->hex (insn-base-mask insn))
  	     " insn-len=" insn-len
  	     " decode-len=" decode-len
! 	     " opcode=" (number->hex opcode)
! 	     " opcode-mask=" (number->hex opcode-mask)
  	     " indices=" indices "\n")
        (map (lambda (index) (+ opcode index)) indices)))
  )