[commit] Reject invalid insns in cgen-generated sid/sim decoders
Dave Brolley <[email protected]>
| Newsgroups | gmane.comp.gdb.patches,gmane.comp.tools.cgen.devel,gmane.comp.emulators.sid.devel |
|---|---|
| Message-ID | <[email protected]> |
Hi, I've committed the attached patch which corrects a long standing problem in the sid/sim decoders generated by cgen. These decoders select the most commonly fixed bits in the ISA a few at a time until enough bits have been selected to uniquely represent one and only one insn from the ISA. The problem is that there may still be encodings which match these bits but do not represent valid insns. As a result, these invalid encodings are simulated as if they were a valid insn. My first idea was to generate one extra nested switch using the remaining opcode bits of the insn which hadn't been used yet. This worked, however for some ISAs with only a few insns but lots of opcode bits, this can lead to the selection of many more bits for the final switch which caused an exponential explosion in the time CGEN takes to generate the decoder. This patch adds one additional test before accepting the insn. Namely, it checks that all of the fixed bits in the insn match the ones which are begin decoded. This is done by taking the logical 'and' of the insn's opcode mask and the bytes being decoded and checking that the result is equal to the insn's opcode value. I've attached a diff of the generated decoder for xstormy16 so that you can see the additional tests which are generated. Let me know if you have any problems or concerns Dave
decode.ChangeLog
(text/plain, 279 B)
2005-05-18 Dave Brolley <[email protected]> * utils-sim.scm (-gen-decode-default-entry): New function. (-gen-decode-insn-entry): Now takes 'invalid-insn' argument. Generate code to check that all opcodes bits match. (-gen-decoder-switch): Use -gen-decode-default-entry.
decode.patch.txt
(text/plain, 5.2 KB)
Index: cgen/utils-sim.scm
===================================================================
RCS file: /cvs/src/src/cgen/utils-sim.scm,v
retrieving revision 1.12
diff -c -p -r1.12 utils-sim.scm
*** cgen/utils-sim.scm 16 Jul 2003 05:35:48 -0000 1.12
--- cgen/utils-sim.scm 18 May 2005 21:51:13 -0000
***************
*** 1,5 ****
; Generic simulator application utilities.
! ; Copyright (C) 2000 Red Hat, Inc.
; This file is part of CGEN.
; See file COPYING.CGEN for details.
--- 1,5 ----
; Generic simulator application utilities.
! ; Copyright (C) 2000, 2005 Red Hat, Inc.
; This file is part of CGEN.
; See file COPYING.CGEN for details.
***************
*** 579,588 ****
; Convert decoder table into C code.
; Return code for one insn entry.
; REST is the remaining entries.
! (define (-gen-decode-insn-entry entry rest indent fn?)
(assert (eq? 'insn (dtable-entry-type entry)))
(logit 3 "Generating decode insn entry for " (obj:name (dtable-entry-value entry)) " ...\n")
--- 579,603 ----
; Convert decoder table into C code.
+ ; Return code for the default entry of each switch table
+ ;
+ (define (-gen-decode-default-entry indent invalid-insn fn?)
+ (string-append
+ "itype = "
+ (gen-cpu-insn-enum (current-cpu) invalid-insn)
+ ";"
+ (if (with-scache?)
+ (if fn?
+ " @prefix@_extract_sfmt_empty (this, current_cpu, pc, base_insn, entire_insn); goto done;\n"
+ " goto extract_sfmt_empty;\n")
+ " goto done;\n")
+ )
+ )
+
; Return code for one insn entry.
; REST is the remaining entries.
! (define (-gen-decode-insn-entry entry rest indent invalid-insn fn?)
(assert (eq? 'insn (dtable-entry-type entry)))
(logit 3 "Generating decode insn entry for " (obj:name (dtable-entry-value entry)) " ...\n")
***************
*** 609,631 ****
(else
(string-append indent " case "
! (number->string (dtable-entry-index entry)) " : "
! "itype = " (gen-cpu-insn-enum (current-cpu) insn) ";"
; Compensate for base-insn-size > current-insn-size by adjusting entire_insn.
; Activate this logic only for sid simulators; they are consistent in
; interpreting base-insn-bitsize this way.
(if (and (equal? APPLICATION 'SID-SIMULATOR)
(> (state-base-insn-bitsize) (insn-length insn)))
(string-append
! " entire_insn = base_insn >> "
(number->string (- (state-base-insn-bitsize) (insn-length insn)))
! ";")
"")
(if (with-scache?)
(if fn?
! (string-append " @prefix@_extract_" fmt-name " (this, current_cpu, pc, base_insn, entire_insn); goto done;\n")
! (string-append " goto extract_" fmt-name ";\n"))
! " goto done;\n")))))
)
; Subroutine of -decode-expr-ifield-tracking.
--- 624,650 ----
(else
(string-append indent " case "
! (number->string (dtable-entry-index entry)) " :\n"
; Compensate for base-insn-size > current-insn-size by adjusting entire_insn.
; Activate this logic only for sid simulators; they are consistent in
; interpreting base-insn-bitsize this way.
(if (and (equal? APPLICATION 'SID-SIMULATOR)
(> (state-base-insn-bitsize) (insn-length insn)))
(string-append
! indent " entire_insn = base_insn >> "
(number->string (- (state-base-insn-bitsize) (insn-length insn)))
! ";\n")
"")
+ ; Generate code to check that all of the opcode bits for this insn match
+ indent " if ((entire_insn & 0x" (number->hex (insn-base-mask insn)) ") == 0x" (number->hex (insn-value insn)) ")\n"
+ indent " { itype = " (gen-cpu-insn-enum (current-cpu) insn) ";"
(if (with-scache?)
(if fn?
! (string-append " @prefix@_extract_" fmt-name " (this, current_cpu, pc, base_insn, entire_insn); goto done;")
! (string-append " goto extract_" fmt-name ";"))
! " goto done;")
! " }\n"
! indent " " (-gen-decode-default-entry indent invalid-insn fn?)))))
)
; Subroutine of -decode-expr-ifield-tracking.
***************
*** 963,969 ****
(cdr entries)
(cons (case (dtable-entry-type (car entries))
((insn)
! (-gen-decode-insn-entry (car entries) (cdr entries) indent fn?))
((expr)
(-gen-decode-expr-entry (car entries) indent invalid-insn fn?))
((table)
--- 982,988 ----
(cdr entries)
(cons (case (dtable-entry-type (car entries))
((insn)
! (-gen-decode-insn-entry (car entries) (cdr entries) indent invalid-insn fn?))
((expr)
(-gen-decode-expr-entry (car entries) indent invalid-insn fn?))
((table)
***************
*** 974,987 ****
result))))
; ??? Can delete if all cases are present.
! indent " default : itype = "
! (gen-cpu-insn-enum (current-cpu) invalid-insn)
! ";"
! (if (with-scache?)
! (if fn?
! " @prefix@_extract_sfmt_empty (this, current_cpu, pc, base_insn, entire_insn); goto done;\n"
! " goto extract_sfmt_empty;\n")
! " goto done;\n")
indent " }\n"
indent "}\n"
)
--- 993,1000 ----
result))))
; ??? Can delete if all cases are present.
! indent " default : "
! (-gen-decode-default-entry indent invalid-insn fn?)
indent " }\n"
indent "}\n"
)
xstormy16-decode.diff.txt
(text/plain, 127.9 KB) - not displayed