Re: [PATCH CGEN v1] cgen: Compute correct mask and values when offset in define-ifield is not 0.

[Guillermo Martinez <guillermo.e.martinez@oracle.com>]

Hi guys,

Just to know if you have some comments to this patch :-)
Thanks in advance.

Kinds regards,
Guillermo

On Thursday, August 19, 2021 9:32:54 PM CDT Guillermo E. Martinez wrote:
> If an instruction field is defined in a long form, assigning
> an offset different to 0 the mask and constant values are not
> computed appropriately:
> 
> (dwf f-op-fld-offset "field with an offset" (all-isas) 32 64 31 32 UINT)
> ;;
> ;; 63              39  32 31         7            0
> ;; +---------------------+-------....-------------+
> ;; |/////////////////////|           |            |
> ;; +---------------------+-------....-------------+
> ;;           |
> ;;           |
> ;;           |
> ;;           |
> ;;           +---> OP_FLD_OFFSET_...
> 
> bitrange: #(object <bitrange> 29 32 31 32 64 #t)
> mask: 0xffffffff
> value: simplify.inc:173:3: Error: Instruction has opcode
>        bits outside of its mask.
> 
> Fixed to:
> 
> bitrange: #(object <bitrange> 29 32 31 32 64 #t)
> mask: 0xffffffff00000000
> value:      0xaa00000000
> 
> word-length relies in base-len (sum of length of all fields
> in bitrange objects that conform the instruction), word-value
> and word-mask are not using the offset entry in the bitrange
> object to calculate the accurate values when constant fields
> are provided (ifld-constant? #t), so one more argument
> is passed to those procedures to be used in the compute.
> 
> Regression tests to the following targets were done:
> 
> bpf arm-linuxeabi arm-nacl arm-netbsdelf arm-nto arm-pe
> arm-symbianelf arm-vxworks arm-wince-pe aarch64-linux alpha-dec-vms
> alpha-linux alpha-linuxecoff alpha-netbsd alpha-unknown-freebsd4.7
> am33_2.0-linux arc-linux-uclibc avr-elf bfin-elf cr16-elf cris-elf
> crisv32-linux crx-elf d10v-elf d30v-elf dlx-elf epiphany-elf fr30-elf
> frv-elf frv-linux ft32-elf h8300-elf hppa-linux hppa-hp-hpux10
> hppa64-hp-hpux11.23 hppa64-linux mips-linux mips-vxworks mips64-linux
> mipsel-linux-gnu mipsisa32el-linux mips64-openbsd mipstx39-elf
> ia64-elf ia64-freebsd5 ia64-hpux ia64-linux ia64-netbsd ia64-vms
> ip2k-elf iq2000-elf lm32-elf m32c-elf m32r-elf m68hc11-elf
> m68hc12-elf m68k-elf m68k-linux m68k-netbsd mcore-elf mcore-pe
> mep-elf metag-linux microblaze-elf mmix mn10200-elf mn10300-elf
> moxie-elf ms1-elf msp430-elf mt-elf nds32le-elf nios2-linux or1k-elf
> pdp11-dec-aout pj-elf powerpc-eabisim powerpc-eabivle powerpc-linux
> powerpc-nto powerpc-wrs-vxworks powerpc64-linux powerpcle-cygwin
> powerpcle-elf powerpc64le-linux ppc-lynxos pru-elf riscv32-elf
> riscv64-elf rl78-elf rs6000-aix4.3.3 rs6000-aix5.1 rx-elf s390-linux
> s390x-linux score-elf sh-linux sh-nto sh-pe sh-rtems sh-vxworks
> shl-unknown-netbsdelf sparc-aout sparc-linux sparc-vxworks
> sparc64-linux sparc-sun-solaris2.12 spu-elf tic30-unknown-aout
> tic30-unknown-coff tic4x-coff tic54x-coff tic6x-elf tilegx-linux
> tilepro-linux v850-elf vax-netbsdelf visium-elf i386-darwin
> i386-lynxos i586-linux i686-nacl i686-pc-beos i686-pc-elf i686-pe
> i686-vxworks x86_64-linux x86_64-w64-mingw32 x86_64-nacl xgate-elf
> xstormy16-elf xtensa-elf z8k-coff z80-coff.
> ---
>  ChangeLog  |  9 +++++++++
>  ifield.scm | 12 ++++--------
>  insn.scm   |  4 +---
>  utils.scm  | 12 ++++++------
>  4 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 93eafe4..7b2b885 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,12 @@
> +2021-08-19  Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
> +
> +	* ifield.scm: word-len has a relative value depending of word-offset
> +	value, method field-value use word-offset parameter.
> +	* insn.scm: remove condition ifld-beyond-base? in insn-base-value
> +	procedure to allow field access when its offset is different to zero.
> +	* utils.scm: word-value/work-mask accept an offset as argument to
> +	compute the mask and get the value when offset is different that zero.
> +
>  2020-05-29  Jose E. Marchesi  <jemarch@gnu.org>
>  
>  	* desc-cpu.scm (/gen-cpu-open): Support passing the instruction
> diff --git a/ifield.scm b/ifield.scm
> index 13e0c4f..c1c8cbd 100644
> --- a/ifield.scm
> +++ b/ifield.scm
> @@ -216,16 +216,13 @@
>       (let ((lsb0? (bitrange-lsb0? bitrange))
>  	   (start (bitrange-start bitrange))
>  	   (length (bitrange-length bitrange))
> -	   (word-length (or (and (= word-offset 0) base-len)
> -			    recorded-word-length))
> +		 (word-length base-len)
>  	   (container-word-offset (bitrange-word-offset container))
>  	   (container-word-length (bitrange-word-length container)))
>         (cond
>  	; must be same lsb0
>  	((not (eq? lsb0? (bitrange-lsb0? container)))
>  	 (error "field-mask: different lsb0? values"))
> -	((not (= word-length container-word-length))
> -	 0)
>  	; container occurs after?
>  	((<= (+ word-offset word-length) container-word-offset)
>  	 0)
> @@ -233,7 +230,7 @@
>  	((>= word-offset (+ container-word-offset container-word-length))
>  	 0)
>  	(else
> -	 (word-mask start length word-length lsb0? #f))))))
> +	 (word-mask start length word-length word-offset lsb0? #f))))))
>  )
>  
>  (define (ifld-mask ifld base-len container)
> @@ -251,11 +248,11 @@
>     (let* ((bitrange (/ifld-bitrange self))
>  	  (recorded-word-length (bitrange-word-length bitrange))
>  	  (word-offset (bitrange-word-offset bitrange))
> -	  (word-length (or (and (= word-offset 0) base-len)
> -			   recorded-word-length)))
> +		 (word-length base-len))
>       (word-value (ifld-start self)
>  		 (bitrange-length bitrange)
>  		 word-length
> +		 word-offset
>  		 (bitrange-lsb0? bitrange) #f
>  		 value)))
>  )
> @@ -441,7 +438,6 @@
>  ; collision with the proc named `length'.
>  ;
>  ; FIXME: More error checking.
> -
>  (define (/ifield-parse context name comment attrs
>  		       word-offset word-length start flength follows
>  		       mode encode decode)
> diff --git a/insn.scm b/insn.scm
> index e62bb87..7a230df 100644
> --- a/insn.scm
> +++ b/insn.scm
> @@ -982,9 +982,7 @@
>        (elm-get insn '/insn-base-value)
>        (let* ((base-len (insn-base-mask-length insn))
>  	     (constant-base-iflds
> -	      (find (lambda (f)
> -		      (and (ifld-constant? f)
> -			   (not (ifld-beyond-base? f))))
> +	      (find ifld-constant?
>  		    (ifields-base-ifields (insn-iflds insn))))
>  	     (base-value (apply +
>  				(map (lambda (f)
> diff --git a/utils.scm b/utils.scm
> index 29b72ff..3573eff 100644
> --- a/utils.scm
> +++ b/utils.scm
> @@ -804,14 +804,14 @@
>  ; Otherwise START denotes the most significant bit.
>  ; N is assumed to fit in the field.
>  
> -(define (word-value start length size lsb0? start-lsb? value)
> +(define (word-value start length size offset lsb0? start-lsb? value)
>    (if lsb0?
>        (if start-lsb?
>  	  (logsll value start)
> -	  (logsll value (+ (- start length) 1)))
> +	  (logsll value (+ (- start length) offset 1)))
>        (if start-lsb?
>  	  (logsll value (- size start 1))
> -	  (logsll value (- size (+ start length)))))
> +	  (logsll value (- size (+ start length offset)))))
>  )
>  
>  ; Return a bit mask of LENGTH bits in a word of SIZE bits starting at START.
> @@ -820,14 +820,14 @@
>  ; START-LSB? is non-#f if START denotes the least significant bit.
>  ; Otherwise START denotes the most significant bit.
>  
> -(define (word-mask start length size lsb0? start-lsb?)
> +(define (word-mask start length size offset lsb0? start-lsb?)
>    (if lsb0?
>        (if start-lsb?
>  	  (logsll (mask length) start)
> -	  (logsll (mask length) (+ (- start length) 1)))
> +	  (logsll (mask length) (+ (- start length) offset 1)))
>        (if start-lsb?
>  	  (logsll (mask length) (- size start 1))
> -	  (logsll (mask length) (- size (+ start length)))))
> +	  (logsll (mask length) (- size (+ start length offset)))))
>  )
>  
>  ; Extract LENGTH bits at bit number START in a word of SIZE bits from VALUE.
>