From: Doug Evans Date: Wed, 9 Sep 2009 20:43:05 +0000 (+0000) Subject: Add support for controlling warnings/errors. X-Git-Url: https://sourceware.org/git/?a=commitdiff_plain;h=ff8fd477d70c09d220346718acdd9b576639db32;p=cgen.git Add support for controlling warnings/errors. Add tests for iformat description errors. * dev.scm (cload): New option #:diag. * read.scm (): New member verify-iformat?. (/parse-diagnostic, parse-warning): New functions. (parse-error): Guts moved to /parse-diagnostic. (/set-diagnostic-options!): New function. (cpu-load): New arg diagnostic-options, all callers updated. Recognize -w diagnostic-option-list. * ifield.scm (ifields-base-ifields): Move here from iformat.scm. (ifld-simple-ifields, ifields-simple-ifields): New function. * insn.scm (/parse-insn-format-iflds): New function. (/parse-insn-format): Guts moved to /parse-insn-format-iflds. New arg isa, all callers updated. Do some basic validation of the ifield list if requested. * mach.scm (/sanity-check-insns): Improve error message text. * doc/running.text: Document -w option. * ifield.scm (/multi-ifield-parse): Initialize bitrange. --- diff --git a/ChangeLog b/ChangeLog index 59991f1..4bb2424 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,25 @@ 2009-09-09 Doug Evans + Add support for controlling warnings/errors. + Add tests for iformat description errors. + * dev.scm (cload): New option #:diag. + * read.scm (): New member verify-iformat?. + (/parse-diagnostic, parse-warning): New functions. + (parse-error): Guts moved to /parse-diagnostic. + (/set-diagnostic-options!): New function. + (cpu-load): New arg diagnostic-options, all callers updated. + Recognize -w diagnostic-option-list. + * ifield.scm (ifields-base-ifields): Move here from iformat.scm. + (ifld-simple-ifields, ifields-simple-ifields): New function. + * insn.scm (/parse-insn-format-iflds): New function. + (/parse-insn-format): Guts moved to /parse-insn-format-iflds. + New arg isa, all callers updated. Do some basic validation of the + ifield list if requested. + * mach.scm (/sanity-check-insns): Improve error message text. + * doc/running.text: Document -w option. + + * ifield.scm (/multi-ifield-parse): Initialize bitrange. + * dev.scm (*): Change default verbosity level to 2. * ifield.scm ( pretty-print): New method. diff --git a/cpu/play.cpu b/cpu/play.cpu index da27ec1..59a2322 100644 --- a/cpu/play.cpu +++ b/cpu/play.cpu @@ -116,6 +116,45 @@ (dnh h-pc "program counter" (PC PROFILE) (pc) () () ()) +; get the real index of a gpr on the circular register queue. + +(define-pmacro (real-gpr-index index) + (if WI (lt index 20) + ; global reg + index + ; local reg + (sequence WI ((WI pr)) + (set pr (add index (reg h-gr 0))) + (if WI (ge pr 256) + (sub pr 236) + pr + ) + ) + ) +) + +; All accesses to gprs in instructions use {get|set}-{gpr|vgpr} +; functions. These functions handle register 0 being 0. This cannot +; be handled without understanding the context of the get or set since +; regs 1-3 are 0 for vector instructions but not for scalar +; instructions. +; +; general purpose registers. + +;(define-hardware +; (name h-gpr) +; (comment "General Purpose Registers") +; (type register DI (256)) +; (indices keyword "" (gen-register-names "r" 256 0 1)) +; (get (index) +; (raw-reg DI h-gpr (real-gpr-index index)) +; ) +; (set (index newval) +; (set (raw-reg DI h-gpr (real-gpr-index index)) newval) +; ) +;) + + (define-hardware (name h-gr) (comment "general registers") @@ -126,6 +165,12 @@ (r0 0) (r1 1) (r2 2) (r3 3) (r4 4) (r5 5) (r6 6) (r7 7) (r8 8) (r9 9) (r10 10) (r11 11) (r12 12) (r13 13) (r14 14) (r15 15) )) + (get (index) + (raw-reg WI h-gr (real-gpr-index index)) + ) + (set (index newval) + (set (raw-reg WI h-gr (real-gpr-index index)) newval) + ) ) (define-hardware @@ -150,6 +195,11 @@ (dsh h-df "df test" () (register DF)) (dsh h-tf "tf test" () (register TF)) + +(define-hardware + (name accum) + (type register (INT 64)) +) ; Operand attributes. diff --git a/dev.scm b/dev.scm index 6a7d410..587a210 100644 --- a/dev.scm +++ b/dev.scm @@ -37,7 +37,8 @@ (keep-mach "all") (keep-isa "all") (options "") - (trace-options "")) + (trace-options "") + (diagnostic-options "")) ; Doesn't check if (cadr args) exists or if #:arch was specified, but ; this is a debugging tool! @@ -51,48 +52,57 @@ ((#:isas) (set! keep-isa (cadr args))) ((#:options) (set! options (cadr args))) ((#:trace) (set! trace-options (cadr args))) + ((#:diag) (set! diagnostic-options (cadr args))) (else (error "unknown option:" (car args)))) (loop (cddr args))))) (case APPLICATION ((UNKNOWN) (error "application not loaded")) ((DESC) (cpu-load cpu-file - keep-mach keep-isa options trace-options + keep-mach keep-isa options + trace-options diagnostic-options desc-init! desc-finish! desc-analyze!)) ((DOC) (cpu-load cpu-file - keep-mach keep-isa options trace-options + keep-mach keep-isa options + trace-options diagnostic-options doc-init! doc-finish! doc-analyze!)) ((OPCODES) (cpu-load cpu-file - keep-mach keep-isa options trace-options + keep-mach keep-isa options + trace-options diagnostic-options opcodes-init! opcodes-finish! opcodes-analyze!)) ((GAS-TEST) (cpu-load cpu-file - keep-mach keep-isa options trace-options + keep-mach keep-isa options + trace-options diagnostic-options gas-test-init! gas-test-finish! gas-test-analyze!)) ((SIMULATOR) (cpu-load cpu-file - keep-mach keep-isa options trace-options + keep-mach keep-isa options + trace-options diagnostic-options sim-init! sim-finish! sim-analyze!)) ((SID-SIMULATOR) (cpu-load cpu-file - keep-mach keep-isa options trace-options + keep-mach keep-isa options + trace-options diagnostic-options sim-init! sim-finish! sim-analyze!)) ((SIM-TEST) (cpu-load cpu-file - keep-mach keep-isa options trace-options + keep-mach keep-isa options + trace-options diagnostic-options sim-test-init! sim-test-finish! sim-test-analyze!)) ((TESTSUITE) (cpu-load cpu-file - keep-mach keep-isa options trace-options + keep-mach keep-isa options + trace-options diagnostic-options testsuite-init! testsuite-finish! testsuite-analyze!)) diff --git a/doc/running.texi b/doc/running.texi index 8337b71..a435405 100644 --- a/doc/running.texi +++ b/doc/running.texi @@ -36,6 +36,7 @@ and applications use uppercase letters for their arguments. * s:: -s Specify the source directory. * t:: -t Specify tracing of various things. * v:: -v Increment the verbosity level. +* w:: -w Enable various diagnostics. * version:: --version Print version info. * opcodes:: Opcodes generator arguments. @@ -293,6 +294,45 @@ make stamp-m32r CGENFLAGS="-v -b -t pmacros" Specifying multiple @code{-v} options will increase the verbosity. +@node w +@section Enable various diagnostics. @option{-w} @var{diagnostic-list} + +Use this to turn on warnings or errors of various things. +The argument is a comma-separated list. +At present the following diagnostics are supported. + +@itemize @bullet + +@item @option{iformat} + +Turn on verification of the instruction format. +If an instruction's field list has missing bits or too many bits +then a warning is issued. + +@emph{NOTE:} The checking is incomplete, but it does catch most +common forms of errors. + +@item @option{all} + +Turn on diagnostics for everything. + +@end itemize + +Each application will invoke CGEN in its own way, so the details of +enabling diagnostics may vary from application to application. +Generally though, each application has a CGENFLAGS makefile variable +for passing flags to CGEN. + +Binutils example: + +@smallexample +# Turn on verification of instruction formats while generating +# the m32r port's opcodes files in the binutils package. +cd obj/opcodes +rm stamp-m32r +make stamp-m32r CGENFLAGS="-v -b -w iformat" +@end smallexample + @node version @section Print version info. @option{--version} diff --git a/ifield.scm b/ifield.scm index cac0415..6479e09 100644 --- a/ifield.scm +++ b/ifield.scm @@ -933,6 +933,7 @@ Define an instruction multi-field, all arguments specified. (elm-xset! result 'mode (parse-mode-name context mode)) (elm-xset! result 'encode (/ifld-parse-encode context encode)) (elm-xset! result 'decode (/ifld-parse-encode context decode)) + (elm-xset! result 'bitrange "multi-ifields don't have bitranges") ;; FIXME (if insert (elm-xset! result 'insert insert) (elm-xset! result 'insert @@ -1090,13 +1091,37 @@ Define an instruction multi-field, all arguments specified. (derived-ifield-subfields self)))) ) -; Traverse the ifield to collect all base (non-derived) ifields used in it. +; Return a list of all base (non-derived) ifields in IFLD. +; NOTE: multi-ifields are *not* reduced to their sub-ifields. (define (ifld-base-ifields ifld) (cond ((derived-ifield? ifld) (ifields-base-ifields (derived-ifield-subfields ifld))) ;;((multi-ifield? ifld) (ifields-base-ifields (multi-ifld-subfields ifld))) (else (list ifld))) ) + +; Collect all base (non-derived) ifields in IFLD-LIST. +; NOTE: multi-ifields are *not* reduced to their sub-ifields. + +(define (ifields-base-ifields ifld-list) + (collect ifld-base-ifields ifld-list) +) + +; Return a list of all simple ifields in IFLD. +; NOTE: multi-ifields *are* reduced to their sub-ifields. + +(define (ifld-simple-ifields ifld) + (cond ((derived-ifield? ifld) (ifields-simple-ifields (derived-ifield-subfields ifld))) + ((multi-ifield? ifld) (ifields-simple-ifields (multi-ifld-subfields ifld))) + (else (list ifld))) +) + +; Collect all simple ifields in IFLD-LIST. +; NOTE: multi-ifields *are* reduced to their sub-ifields. + +(define (ifields-simple-ifields ifld-list) + (collect ifld-simple-ifields ifld-list) +) ; Misc. utilities. diff --git a/iformat.scm b/iformat.scm index 6306506..bb91660 100644 --- a/iformat.scm +++ b/iformat.scm @@ -67,13 +67,6 @@ (number key ifields mask-length length mask eg-insn) ) -; Traverse the ifield list to collect all base (non-derived) ifields -; used in it. - -(define (ifields-base-ifields ifld-list) - (collect ifld-base-ifields ifld-list) -) - ; Return enum cgen_fmt_type value for FMT. ; ??? Not currently used. @@ -554,7 +547,7 @@ ; intelligent processing of it later. (for-each (lambda (insn) - (logit 3 "Scanning operands of " (obj:name insn) ": " + (logit 2 "Scanning operands of " (obj:name insn) ": " (insn-syntax insn) " ...\n") (let ((sem-ops (ifmt-analyze insn compute-sformat?))) (insn-set-fmt-desc! insn (car sem-ops)) @@ -598,7 +591,7 @@ ) (for-each (lambda (insn) - (logit 3 "Processing format for " (obj:name insn) ": " + (logit 2 "Processing format for " (obj:name insn) ": " (insn-syntax insn) " ...\n") (let ((fmt-desc (insn-fmt-desc insn))) diff --git a/insn.scm b/insn.scm index dce927c..c3343d8 100644 --- a/insn.scm +++ b/insn.scm @@ -394,7 +394,8 @@ ;; Pick out name first to augment the error context. (let* ((name (parse-name context name)) (context (context-append-name context name)) - (atlist-obj (atlist-parse context attrs "cgen_insn"))) + (atlist-obj (atlist-parse context attrs "cgen_insn")) + (isas (bitset-attr->list (atlist-attr-value atlist-obj 'ISA #f)))) (if (keep-atlist? atlist-obj #f) @@ -405,6 +406,11 @@ semantics #f)) (format (/parse-insn-format (context-append context " format") + ;; Just pick the first, the base len + ;; for each should be the same. + ;; If not this is caught by + ;; compute-insn-base-mask-length. + (current-isa-lookup (car isas)) fmt)) (comment (parse-comment context comment)) ; If there are no semantics, mark this as an alias. @@ -590,37 +596,10 @@ (parse-error context "unknown ifield" spec))) ) -; Given an insn format field from a .cpu file, replace it with a list of -; ifield objects with the values assigned. -; -; An insn format field is a list of ifields that make up the instruction. -; All bits must be specified, including reserved bits -; [at present no checking is made of this, but the rule still holds]. -; -; A normal entry begins with `+' and then consist of the following: -; - operand name -; - (ifield-name [options] value) -; - (operand-name [options] [value]) -; - insn ifield enum -; -; Example: (+ OP1_ADD (f-res2 0) dr src1 (f-src2 1) (f-res1 #xea)) -; -; where OP1_ADD is an enum, dr and src1 are operands, and f-src2 and f-res1 -; are ifield's. The `+' allows for future extension. -; -; The other form of entry begins with `=' and is followed by an instruction -; name that has the same format. The specified instruction must already be -; defined. Instructions with this form typically also include an -; `ifield-assertion' spec to keep them separate. -; -; An empty field list is ok. This means it's unspecified. -; VIRTUAL insns have this. -; -; This is one of the more important routines to be efficient. -; It's called for each instruction, and is one of the more expensive routines -; in insn parsing. +; Subroutine of /parse-insn-format to simplify it. +; Parse the provided iformat spec and return the list of ifields. -(define (/parse-insn-format context fld-list) +(define (/parse-insn-iformat-iflds context fld-list) (if (null? fld-list) nil ; field list unspecified (case (car fld-list) @@ -656,6 +635,80 @@ )) ) +; Given an insn format field from a .cpu file, replace it with a list of +; ifield objects with the values assigned. +; If ISA is non-#f, it is an object, and we perform various checks +; on the format (which require an isa). +; +; An insn format field is a list of ifields that make up the instruction. +; All bits must be specified, including reserved bits +; [at present no checking is made of this, but the rule still holds]. +; +; A normal entry begins with `+' and then consist of the following: +; - operand name +; - (ifield-name [options] value) +; - (operand-name [options] [value]) +; - insn ifield enum +; +; Example: (+ OP1_ADD (f-res2 0) dr src1 (f-src2 1) (f-res1 #xea)) +; +; where OP1_ADD is an enum, dr and src1 are operands, and f-src2 and f-res1 +; are ifield's. The `+' allows for future extension. +; +; The other form of entry begins with `=' and is followed by an instruction +; name that has the same format. The specified instruction must already be +; defined. Instructions with this form typically also include an +; `ifield-assertion' spec to keep them separate. +; +; An empty field list is ok. This means it's unspecified. +; VIRTUAL insns have this. +; +; This is one of the more important routines to be efficient. +; It's called for each instruction, and is one of the more expensive routines +; in insn parsing. + +(define (/parse-insn-format context isa ifld-list) + (let* ((parsed-ifld-list (/parse-insn-iformat-iflds context ifld-list))) + + ;; NOTE: We could sort the fields here, but it introduces differences + ;; in the generated opcodes files. Later it might be a good thing to do + ;; but keeping the output consistent is important right now. + ;; (sorted-ifld-list (sort-ifield-list parsed-ifld-list + ;; (not (current-arch-insn-lsb0?)))) + ;; The rest of the code assumes the list isn't sorted. + ;; Is there a benefit to removing this assumption? Note that + ;; multi-ifields can be discontiguous, so the sorting isn't perfect. + + (if (and isa + (reader-verify-iformat? CURRENT-READER)) + (let ((base-len (isa-base-insn-bitsize isa))) + + ;; Perform some error checking. + ;; Look for overlapping ifields and missing bits. + ;; With derived ifields this is really hard, so only do the base insn + ;; for now. Do the simple test for now, it doesn't catch everything, + ;; but it should catch a lot. + + (let* ((base-iflds (find (lambda (f) + (not (ifld-beyond-base? f))) + (ifields-simple-ifields parsed-ifld-list))) + (base-iflds-length (apply + (map ifld-length base-iflds)))) + ;; FIXME: We don't use parse-error here because some existing ports + ;; have problems, and I don't have time to fix them right now. + (cond ((< base-iflds-length base-len) + (parse-warning context + "insufficient number of bits specified in base insn" + ifld-list)) + ((> base-iflds-length base-len) + (parse-warning context + "too many bits specified in base insn" + ifld-list))) + ) + )) + + parsed-ifld-list) +) + ; Return a boolean indicating if IFLD-LIST contains anyof operands. (define (anyof-operand-format? ifld-list) diff --git a/mach.scm b/mach.scm index 86e6aec..10e378b 100644 --- a/mach.scm +++ b/mach.scm @@ -1773,6 +1773,8 @@ ;; Ensure instruction base values agree with their masks. ;; Errors can come from bad .cpu files, bugs, or both. ;; It's better to catch such errors early. + ;; If it is an error in the .cpu file, we don't want to crash + ;; on a Guile error. (for-each @@ -1791,6 +1793,11 @@ "This usually means some kind of error in the instruction's ifield list.\n" "base mask: 0x" (number->hex base-mask) ", base value: 0x" (number->hex base-value) + "\nfield list:" + (string-map (lambda (f) + (string-append " " + (ifld-pretty-print f))) + (insn-iflds insn)) ))) ;; Insert more checks here. diff --git a/operand.scm b/operand.scm index 4bc7e4a..c5ce82a 100644 --- a/operand.scm +++ b/operand.scm @@ -796,7 +796,7 @@ ; ??? Calling /parse-insn-format is a quick hack. ; It's an internal routine of some other file. - (let ((iflds (/parse-insn-format context encoding))) + (let ((iflds (/parse-insn-format context #f encoding))) (make operand-name 'derived-ifield ; (string-append " for " operand-name) diff --git a/read.scm b/read.scm index 2c6daf6..5c15b88 100644 --- a/read.scm +++ b/read.scm @@ -275,6 +275,9 @@ ; Boolean indicating if pmacro tracing is on. (cons 'trace-pmacros? #f) + ; Issue diagnostics for instruction format issues. + (cons 'verify-iformat? #f) + ; Currently select cpu family, computed from `keep-mach'. ; Some applications don't care, and this is moderately ; expensive to compute so we use delay/force. @@ -297,11 +300,11 @@ (define-getters reader (keep-mach keep-isa - trace-commands? trace-pmacros? + trace-commands? trace-pmacros? verify-iformat? current-cpu commands location)) (define-setters reader (keep-mach keep-isa - trace-commands? trace-pmacros? + trace-commands? trace-pmacros? verify-iformat? current-cpu commands location)) (define (reader-add-command! name comment attrs arg-spec handler) @@ -336,14 +339,11 @@ ":"))) ) -;;; Signal a parse error while reading a .cpu file. -;;; If CONTEXT is #f, use a default context of the current reader location -;;; and an empty prefix. -;;; If MAYBE-HELP-TEXT is specified, elide the last trailing \n. -;;; Multiple lines of help text need embedded newlines, and should be no longer -;;; than 79 characters. +;;; Subroutine of parse-error, parse-warning to simplify them. +;;; Flag an error or a warning. +;;; EMITTER is a function of one argument, the message to print. -(define (parse-error context message expr . maybe-help-text) +(define (/parse-diagnostic emitter context message expr maybe-help-text) (if (not context) (set! context (make (current-reader-location) #f))) @@ -353,7 +353,7 @@ (prefix (or (context-prefix context) "Error")) (text (string-append prefix ": " message))) - (error + (emitter (simple-format #f "\n~A:\n@ ~A:\n\n~A: ~A: ~S~A" @@ -362,9 +362,39 @@ (single-location->simple-string top-sloc) text expr - (if (null? maybe-help-text) - "" - (string-append "\n\n" (car maybe-help-text)))))) + (if maybe-help-text + (string-append "\n\n" maybe-help-text) + "")))) +) + +;;; Signal a parse error while reading a .cpu file. +;;; If CONTEXT is #f, use a default context of the current reader location +;;; and an empty prefix. +;;; If MAYBE-HELP-TEXT is specified, elide the last trailing \n. +;;; Multiple lines of help text need embedded newlines, and should be no longer +;;; than 79 characters. + +(define (parse-error context errmsg expr . maybe-help-text) + (/parse-diagnostic error + context + errmsg + expr + (if (null? maybe-help-text) "" (car maybe-help-text))) +) + +;;; Signal a parse warning while reading a .cpu file. +;;; If CONTEXT is #f, use a default context of the current reader location +;;; and an empty prefix. +;;; If MAYBE-HELP-TEXT is specified, elide the last trailing \n. +;;; Multiple lines of help text need embedded newlines, and should be no longer +;;; than 79 characters. + +(define (parse-warning context errmsg expr . maybe-help-text) + (/parse-diagnostic (lambda (text) (message "Warning: " text "\n")) + context + errmsg + expr + (if (null? maybe-help-text) #f (car maybe-help-text))) ) ; Return the current source location. @@ -783,6 +813,36 @@ *UNSPECIFIED* ) +;; Diagnostic support. + +;;; Enable the specified diagnostics. +;;; DIAGNOSTIC-OPTIONS is a comma-separated list of things to trace. +;;; +;;; Currently supported diagnostics: +;;; iformat - issue diagnostics for iformat issues +;;; all - turn on all diagnostics +;;; +;;; [If we later need to support disabling some diagnostic, one way is to +;;; recognize an "-" in front of an option.] + +(define (/set-diagnostic-options! diagnostic-options) + (let ((all (list "iformat")) + (requests (string-cut diagnostic-options #\,))) + (if (member "all" requests) + (append! requests all)) + (for-each (lambda (item) + (cond ((string=? "iformat" item) + (reader-set-verify-iformat?! CURRENT-READER #t)) + ((string=? "all" item) + #t) ;; handled above + (else + (cgen-usage 'unknown (string-append "-w " item) + common-arguments)))) + requests)) + + *UNSPECIFIED* +) + ; If #f, treat reserved fields as operands and extract them with the insn. ; Code can then be emitted in the extraction routines to validate them. ; If #t, treat reserved fields as part of the opcode. @@ -1006,6 +1066,7 @@ Define a preprocessor-style macro. ; KEEP-ISA is a string of comma separated isas to keep. ; OPTIONS is the OPTIONS argument to -init-parse-cpu!. ; TRACE-OPTIONS is a random list of things to trace. +; DIAGNOSTIC-OPTIONS is a random list of things to warn/error about. ; APP-INITER! is an application specific zero argument proc (thunk) ; to call after -init-parse-cpu! ; APP-FINISHER! is an application specific zero argument proc to call after @@ -1016,11 +1077,13 @@ Define a preprocessor-style macro. ; ; This function isn't local because it's used by dev.scm. -(define (cpu-load file keep-mach keep-isa options trace-options +(define (cpu-load file keep-mach keep-isa options + trace-options diagnostic-options app-initer! app-finisher! analyzer!) (/init-reader!) (/init-parse-cpu! keep-mach keep-isa options) (/set-trace-options! trace-options) + (/set-diagnostic-options! diagnostic-options) (app-initer!) (logit 1 "Loading cpu description " file " ...\n") (set! arch-path (dirname file)) @@ -1132,6 +1195,11 @@ Define a preprocessor-style macro. "pmacros - trace pmacro expansion" "all - trace everything") ("-v" #f "increment verbosity level") + ("-w" "diagnostic-options" "specify list of things to issue diagnostics about" + "Options:" + "iformat - verify instruction formats are valid" + "all - turn on all diagnostics") + ("--version" #f "print version info") ) ) @@ -1207,6 +1275,7 @@ Define a preprocessor-style macro. (moreopts? #t) (debugging #f) ; default is off, for speed (trace-options "") + (diagnostic-options "") (cep (current-error-port)) (str=? string=?) ) @@ -1257,6 +1326,9 @@ Define a preprocessor-style macro. ((str=? "-v" (car opt)) (verbose-inc!) ) + ((str=? "-w" (car opt)) + (set! diagnostic-options arg) + ) ((str=? "--version" (car opt)) (begin (display "Cpu tools GENerator version ") @@ -1297,7 +1369,8 @@ Define a preprocessor-style macro. (debug-repl nil)) (cpu-load arch-file - keep-mach keep-isa flags trace-options + keep-mach keep-isa flags + trace-options diagnostic-options app-init! app-finish! app-analyze!) ;; Start another repl loop if -d.