* [PATCH CGEN v1] cgen: Compute correct mask and values when offset in define-ifield is not 0.
@ 2021-08-20 2:32 Guillermo E. Martinez
2021-09-02 13:22 ` Guillermo Martinez
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Guillermo E. Martinez @ 2021-08-20 2:32 UTC (permalink / raw)
To: cgen; +Cc: Guillermo E. Martinez
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.
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH CGEN v1] cgen: Compute correct mask and values when offset in define-ifield is not 0.
2021-08-20 2:32 [PATCH CGEN v1] cgen: Compute correct mask and values when offset in define-ifield is not 0 Guillermo E. Martinez
@ 2021-09-02 13:22 ` Guillermo Martinez
2021-09-10 1:23 ` Frank Ch. Eigler
2021-09-14 21:34 ` Jose E. Marchesi
2021-09-15 16:32 ` [PATCH v2] " Guillermo E. Martinez
2 siblings, 1 reply; 12+ messages in thread
From: Guillermo Martinez @ 2021-09-02 13:22 UTC (permalink / raw)
To: cgen
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.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH CGEN v1] cgen: Compute correct mask and values when offset in define-ifield is not 0.
2021-09-02 13:22 ` Guillermo Martinez
@ 2021-09-10 1:23 ` Frank Ch. Eigler
2021-09-10 2:29 ` Jose E. Marchesi
0 siblings, 1 reply; 12+ messages in thread
From: Frank Ch. Eigler @ 2021-09-10 1:23 UTC (permalink / raw)
To: Guillermo Martinez; +Cc: cgen
Hi -
> Just to know if you have some comments to this patch :-)
> Thanks in advance.
Sorry, no fresh enough knowledge to offer informed commentary. If
your regression tests on other platform indicated no problems, and
it solves a real problem with your arch, go for it.
- FChE
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH CGEN v1] cgen: Compute correct mask and values when offset in define-ifield is not 0.
2021-09-10 1:23 ` Frank Ch. Eigler
@ 2021-09-10 2:29 ` Jose E. Marchesi
2021-09-10 2:36 ` Jose E. Marchesi
0 siblings, 1 reply; 12+ messages in thread
From: Jose E. Marchesi @ 2021-09-10 2:29 UTC (permalink / raw)
To: Frank Ch. Eigler via Cgen
>> Just to know if you have some comments to this patch :-)
>> Thanks in advance.
>
> Sorry, no fresh enough knowledge to offer informed commentary. If
> your regression tests on other platform indicated no problems, and
> it solves a real problem with your arch, go for it.
FWIW we reviewed carefully the change internally and it looks good to
me...
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH CGEN v1] cgen: Compute correct mask and values when offset in define-ifield is not 0.
2021-09-10 2:29 ` Jose E. Marchesi
@ 2021-09-10 2:36 ` Jose E. Marchesi
0 siblings, 0 replies; 12+ messages in thread
From: Jose E. Marchesi @ 2021-09-10 2:36 UTC (permalink / raw)
To: Jose E. Marchesi via Cgen
>>> Just to know if you have some comments to this patch :-)
>>> Thanks in advance.
>>
>> Sorry, no fresh enough knowledge to offer informed commentary. If
>> your regression tests on other platform indicated no problems, and
>> it solves a real problem with your arch, go for it.
>
> FWIW we reviewed carefully the change internally and it looks good to
> me...
... and of course if we break it, we fix it ;)
/me crosses fingers and toes and...
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH CGEN v1] cgen: Compute correct mask and values when offset in define-ifield is not 0.
2021-08-20 2:32 [PATCH CGEN v1] cgen: Compute correct mask and values when offset in define-ifield is not 0 Guillermo E. Martinez
2021-09-02 13:22 ` Guillermo Martinez
@ 2021-09-14 21:34 ` Jose E. Marchesi
2021-09-15 16:32 ` [PATCH v2] " Guillermo E. Martinez
2 siblings, 0 replies; 12+ messages in thread
From: Jose E. Marchesi @ 2021-09-14 21:34 UTC (permalink / raw)
To: Guillermo E. Martinez via Cgen
Hi Guillermo.
> 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.
I just noticed that you forgot to add entries for OFFSET to the comment
blocks of word-value and word-mask.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] cgen: Compute correct mask and values when offset in define-ifield is not 0.
2021-08-20 2:32 [PATCH CGEN v1] cgen: Compute correct mask and values when offset in define-ifield is not 0 Guillermo E. Martinez
2021-09-02 13:22 ` Guillermo Martinez
2021-09-14 21:34 ` Jose E. Marchesi
@ 2021-09-15 16:32 ` Guillermo E. Martinez
2021-09-15 16:49 ` Jose E. Marchesi
2 siblings, 1 reply; 12+ messages in thread
From: Guillermo E. Martinez @ 2021-09-15 16:32 UTC (permalink / raw)
To: cgen
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 | 18 ++++++++++--------
4 files changed, 24 insertions(+), 19 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..a6d2d53 100644
--- a/utils.scm
+++ b/utils.scm
@@ -797,37 +797,39 @@
(define (minmax min max) (cons min max))
-; Move VALUE of LENGTH bits to position START in a word of SIZE bits.
+; Move VALUE of LENGTH bits to position START plus an OFFSET bits
+; in a word of SIZE bits.
; LSB0? is non-#f if bit numbering goes LSB->MSB.
; Otherwise it goes MSB->LSB.
; START-LSB? is non-#f if START denotes the least significant bit.
; 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.
+; Return a bit mask of LENGTH bits in a word of SIZE bits starting
+; at START plus an OFFSET bits.
; LSB0? is non-#f if bit numbering goes LSB->MSB.
; Otherwise it goes MSB->LSB.
; 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.
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] cgen: Compute correct mask and values when offset in define-ifield is not 0.
2021-09-15 16:32 ` [PATCH v2] " Guillermo E. Martinez
@ 2021-09-15 16:49 ` Jose E. Marchesi
2021-09-15 18:04 ` Guillermo Martinez
0 siblings, 1 reply; 12+ messages in thread
From: Jose E. Marchesi @ 2021-09-15 16:49 UTC (permalink / raw)
To: Guillermo E. Martinez via Cgen
Hi Guillermo.
I just installed this version of the patch on your behalf.
Thanks!
> 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 | 18 ++++++++++--------
> 4 files changed, 24 insertions(+), 19 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..a6d2d53 100644
> --- a/utils.scm
> +++ b/utils.scm
> @@ -797,37 +797,39 @@
>
> (define (minmax min max) (cons min max))
>
> -; Move VALUE of LENGTH bits to position START in a word of SIZE bits.
> +; Move VALUE of LENGTH bits to position START plus an OFFSET bits
> +; in a word of SIZE bits.
> ; LSB0? is non-#f if bit numbering goes LSB->MSB.
> ; Otherwise it goes MSB->LSB.
> ; START-LSB? is non-#f if START denotes the least significant bit.
> ; 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.
> +; Return a bit mask of LENGTH bits in a word of SIZE bits starting
> +; at START plus an OFFSET bits.
> ; LSB0? is non-#f if bit numbering goes LSB->MSB.
> ; Otherwise it goes MSB->LSB.
> ; 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] cgen: Compute correct mask and values when offset in define-ifield is not 0.
2021-09-15 16:49 ` Jose E. Marchesi
@ 2021-09-15 18:04 ` Guillermo Martinez
2021-11-25 23:02 ` Alan Modra
0 siblings, 1 reply; 12+ messages in thread
From: Guillermo Martinez @ 2021-09-15 18:04 UTC (permalink / raw)
To: Guillermo E. Martinez via Cgen, Jose E. Marchesi
On Wednesday, September 15, 2021 11:49:12 AM CDT Jose E. Marchesi wrote:
Hi Jose,
great!
Thanks to you!
>
> Hi Guillermo.
>
> I just installed this version of the patch on your behalf.
> Thanks!
>
> > 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 | 18 ++++++++++--------
> > 4 files changed, 24 insertions(+), 19 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..a6d2d53 100644
> > --- a/utils.scm
> > +++ b/utils.scm
> > @@ -797,37 +797,39 @@
> >
> > (define (minmax min max) (cons min max))
> >
> > -; Move VALUE of LENGTH bits to position START in a word of SIZE bits.
> > +; Move VALUE of LENGTH bits to position START plus an OFFSET bits
> > +; in a word of SIZE bits.
> > ; LSB0? is non-#f if bit numbering goes LSB->MSB.
> > ; Otherwise it goes MSB->LSB.
> > ; START-LSB? is non-#f if START denotes the least significant bit.
> > ; 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.
> > +; Return a bit mask of LENGTH bits in a word of SIZE bits starting
> > +; at START plus an OFFSET bits.
> > ; LSB0? is non-#f if bit numbering goes LSB->MSB.
> > ; Otherwise it goes MSB->LSB.
> > ; 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.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] cgen: Compute correct mask and values when offset in define-ifield is not 0.
2021-09-15 18:04 ` Guillermo Martinez
@ 2021-11-25 23:02 ` Alan Modra
2021-11-25 23:28 ` Guillermo Martinez
0 siblings, 1 reply; 12+ messages in thread
From: Alan Modra @ 2021-11-25 23:02 UTC (permalink / raw)
To: Guillermo Martinez, Jose E. Marchesi; +Cc: cgen, binutils
On Wed, Sep 15, 2021 at 06:04:44PM +0000, Guillermo Martinez wrote:
> On Wednesday, September 15, 2021 11:49:12 AM CDT Jose E. Marchesi wrote:
> > I just installed this version of the patch on your behalf.
> > Thanks!
> >
> > > 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:
[snip]
It's been a while since I refreshed the binutils opcodes files that
are cgen generated. On doing so it appears this patch is responsible
for build errors when using mainline gcc. First example:
/home/alan/src/binutils-gdb/opcodes/bpf-opc.c:57:11: error: conversion from ‘long unsigned int’ to ‘unsigned int’ changes value from ‘18446744073709486335’ to ‘4294902015’ [-Werror=overflow]
57 | 64, 64, 0xffffffffffff00ff, { { F (F_IMM32) }, { F (F_OFFSET16) }, { F (F_SRCLE) }, { F (F_OP_CODE) }, { F (F_DSTLE) }, { F (F_OP_SRC) }, { F (F_OP_CLASS) }, { 0 } }
| ^~~~~~~~~~~~~~~~~~
I tested an obvious workaround,
diff --git a/include/opcode/cgen.h b/include/opcode/cgen.h
index 8b7d2a4b547..0e9571d19b0 100644
--- a/include/opcode/cgen.h
+++ b/include/opcode/cgen.h
@@ -914,7 +914,7 @@ typedef struct
Each insn's value is stored with the insn.
The first step in recognizing an insn for disassembly is
(opcode & mask) == value. */
- CGEN_INSN_INT mask;
+ uint64_t mask;
#define CGEN_IFMT_MASK(ifmt) ((ifmt)->mask)
/* Instruction fields.
diff --git a/opcodes/cgen-dis.c b/opcodes/cgen-dis.c
index 1a5d1ae8459..37ee5a23564 100644
--- a/opcodes/cgen-dis.c
+++ b/opcodes/cgen-dis.c
@@ -39,7 +39,7 @@ static void add_insn_to_hash_chain (CGEN_INSN_LIST *,
static int
count_decodable_bits (const CGEN_INSN *insn)
{
- unsigned mask = CGEN_INSN_BASE_MASK (insn);
+ uint64_t mask = CGEN_INSN_BASE_MASK (insn);
#if GCC_VERSION >= 3004
return __builtin_popcount (mask);
#else
Quite possibly the above isn't a complete fix, I just threw it
together to see what happens. Regressions:
+FAIL: eBPF CALL instruction
+FAIL: eBPF CALL instruction, big endian
+FAIL: CALL with disp32 reloc
+FAIL: CALL with disp32 reloc and addend
+FAIL: CALL check unsigned underflow
gas/testsuite/gas.log for the first one shows:
regexp_diff match failure
regexp "^ 20: 85 10 00 00 00 00 00 00 call 0$"
line " 20: 85 10 00 00 00 00 00 00 *unknown*"
regexp_diff match failure
regexp "^ 28: 85 10 00 00 ff ff ff ff call -1$"
line " 28: 85 10 00 00 ff ff ff ff *unknown*"
regexp_diff match failure
regexp "^ 30: 85 10 00 00 fe ff ff ff call -2$"
line " 30: 85 10 00 00 fe ff ff ff *unknown*"
regexp_diff match failure
regexp "^ 38: 85 10 00 00 fd ff ff ff call -3$"
line " 38: 85 10 00 00 fd ff ff ff *unknown*"
FAIL: eBPF CALL instruction
At this point I don't intend to look any further into the problem.
The opcodes/ refresh can wait.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] cgen: Compute correct mask and values when offset in define-ifield is not 0.
2021-11-25 23:02 ` Alan Modra
@ 2021-11-25 23:28 ` Guillermo Martinez
2021-11-26 0:46 ` Alan Modra
0 siblings, 1 reply; 12+ messages in thread
From: Guillermo Martinez @ 2021-11-25 23:28 UTC (permalink / raw)
To: Jose Marchesi, Alan Modra; +Cc: cgen, binutils
On Thursday, November 25, 2021 5:02:11 PM CST Alan Modra wrote:
> On Wed, Sep 15, 2021 at 06:04:44PM +0000, Guillermo Martinez wrote:
> > On Wednesday, September 15, 2021 11:49:12 AM CDT Jose E. Marchesi wrote:
> > > I just installed this version of the patch on your behalf.
> > > Thanks!
> > >
> > > > 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:
> [snip]
>
> It's been a while since I refreshed the binutils opcodes files that
> are cgen generated. On doing so it appears this patch is responsible
> for build errors when using mainline gcc. First example:
>
> /home/alan/src/binutils-gdb/opcodes/bpf-opc.c:57:11: error: conversion from ‘long unsigned int’ to ‘unsigned int’ changes value from ‘18446744073709486335’ to ‘4294902015’ [-Werror=overflow]
> 57 | 64, 64, 0xffffffffffff00ff, { { F (F_IMM32) }, { F (F_OFFSET16) }, { F (F_SRCLE) }, { F (F_OP_CODE) }, { F (F_DSTLE) }, { F (F_OP_SRC) }, { F (F_OP_CLASS) }, { 0 } }
> | ^~~~~~~~~~~~~~~~~~
>
> I tested an obvious workaround,
>
> diff --git a/include/opcode/cgen.h b/include/opcode/cgen.h
> index 8b7d2a4b547..0e9571d19b0 100644
> --- a/include/opcode/cgen.h
> +++ b/include/opcode/cgen.h
> @@ -914,7 +914,7 @@ typedef struct
> Each insn's value is stored with the insn.
> The first step in recognizing an insn for disassembly is
> (opcode & mask) == value. */
> - CGEN_INSN_INT mask;
> + uint64_t mask;
> #define CGEN_IFMT_MASK(ifmt) ((ifmt)->mask)
>
> /* Instruction fields.
> diff --git a/opcodes/cgen-dis.c b/opcodes/cgen-dis.c
> index 1a5d1ae8459..37ee5a23564 100644
> --- a/opcodes/cgen-dis.c
> +++ b/opcodes/cgen-dis.c
> @@ -39,7 +39,7 @@ static void add_insn_to_hash_chain (CGEN_INSN_LIST *,
> static int
> count_decodable_bits (const CGEN_INSN *insn)
> {
> - unsigned mask = CGEN_INSN_BASE_MASK (insn);
> + uint64_t mask = CGEN_INSN_BASE_MASK (insn);
> #if GCC_VERSION >= 3004
> return __builtin_popcount (mask);
> #else
>
> Quite possibly the above isn't a complete fix, I just threw it
> together to see what happens. Regressions:
correct, it's not a complete fix:
https://sourceware.org/pipermail/binutils/2021-October/118284.html
However applying patch v2 on upstream pitifully it won't work because there is
an error in BPF simulator (sim/bpf/decode-le.c (extract_sfmt_lddwle))
(not introduced by this patch) but now raised by adding -Werror=shift-count-overflow
in binutils-gdb builder (currently im debugging this :-))
> +FAIL: eBPF CALL instruction
> +FAIL: eBPF CALL instruction, big endian
> +FAIL: CALL with disp32 reloc
> +FAIL: CALL with disp32 reloc and addend
> +FAIL: CALL check unsigned underflow
>
> gas/testsuite/gas.log for the first one shows:
> regexp_diff match failure
> regexp "^ 20: 85 10 00 00 00 00 00 00 call 0$"
> line " 20: 85 10 00 00 00 00 00 00 *unknown*"
> regexp_diff match failure
> regexp "^ 28: 85 10 00 00 ff ff ff ff call -1$"
> line " 28: 85 10 00 00 ff ff ff ff *unknown*"
> regexp_diff match failure
> regexp "^ 30: 85 10 00 00 fe ff ff ff call -2$"
> line " 30: 85 10 00 00 fe ff ff ff *unknown*"
> regexp_diff match failure
> regexp "^ 38: 85 10 00 00 fd ff ff ff call -3$"
> line " 38: 85 10 00 00 fd ff ff ff *unknown*"
> FAIL: eBPF CALL instruction
>
> At this point I don't intend to look any further into the problem.
> The opcodes/ refresh can wait.
I'll work on this ASAP.
Thanks Alan!
Guillermo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] cgen: Compute correct mask and values when offset in define-ifield is not 0.
2021-11-25 23:28 ` Guillermo Martinez
@ 2021-11-26 0:46 ` Alan Modra
0 siblings, 0 replies; 12+ messages in thread
From: Alan Modra @ 2021-11-26 0:46 UTC (permalink / raw)
To: Guillermo Martinez; +Cc: Jose Marchesi, cgen, binutils
On Thu, Nov 25, 2021 at 11:28:31PM +0000, Guillermo Martinez wrote:
> correct, it's not a complete fix:
> https://sourceware.org/pipermail/binutils/2021-October/118284.html
OK, I saw that one go by, but when targets have an active maintainer I
tend to leave any review to the target maintainer.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-11-26 0:46 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 2:32 [PATCH CGEN v1] cgen: Compute correct mask and values when offset in define-ifield is not 0 Guillermo E. Martinez
2021-09-02 13:22 ` Guillermo Martinez
2021-09-10 1:23 ` Frank Ch. Eigler
2021-09-10 2:29 ` Jose E. Marchesi
2021-09-10 2:36 ` Jose E. Marchesi
2021-09-14 21:34 ` Jose E. Marchesi
2021-09-15 16:32 ` [PATCH v2] " Guillermo E. Martinez
2021-09-15 16:49 ` Jose E. Marchesi
2021-09-15 18:04 ` Guillermo Martinez
2021-11-25 23:02 ` Alan Modra
2021-11-25 23:28 ` Guillermo Martinez
2021-11-26 0:46 ` Alan Modra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).