From db647703ba9d82519363c107715f562187fbcc16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 13 Apr 2021 10:10:08 +0200 Subject: [PATCH 1/5] exec: Remove accel/tcg/ from include paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When TCG is enabled, the accel/tcg/ include path is added to the project global include search list. This accel/tcg/ directory contains a header named "internal.h" which, while intented to be internal to accel/tcg/, is accessible by all files compiled when TCG is enabled. This might lead to problem with other directories using the same "internal.h" header name: $ git ls-files | fgrep /internal.h accel/tcg/internal.h include/hw/ide/internal.h target/hexagon/internal.h target/mips/internal.h target/ppc/internal.h target/s390x/internal.h As we don't need to expose accel/tcg/ internals to the rest of the code base, simplify by removing it from the include search list, and include the accel/tcg/ public headers relative to the project root search path (which is already in the generic include search path). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Claudio Fontana Message-Id: <20210413081008.3409459-1-f4bug@amsat.org> Signed-off-by: Richard Henderson --- include/exec/helper-gen.h | 4 ++-- include/exec/helper-proto.h | 4 ++-- include/exec/helper-tcg.h | 4 ++-- meson.build | 1 - 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/include/exec/helper-gen.h b/include/exec/helper-gen.h index 29c02f85dc..1c2e7a8ed3 100644 --- a/include/exec/helper-gen.h +++ b/include/exec/helper-gen.h @@ -81,8 +81,8 @@ static inline void glue(gen_helper_, name)(dh_retvar_decl(ret) \ #include "helper.h" #include "trace/generated-helpers.h" #include "trace/generated-helpers-wrappers.h" -#include "tcg-runtime.h" -#include "plugin-helpers.h" +#include "accel/tcg/tcg-runtime.h" +#include "accel/tcg/plugin-helpers.h" #undef DEF_HELPER_FLAGS_0 #undef DEF_HELPER_FLAGS_1 diff --git a/include/exec/helper-proto.h b/include/exec/helper-proto.h index 659f9298e8..ba100793a7 100644 --- a/include/exec/helper-proto.h +++ b/include/exec/helper-proto.h @@ -39,8 +39,8 @@ dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \ #include "helper.h" #include "trace/generated-helpers.h" -#include "tcg-runtime.h" -#include "plugin-helpers.h" +#include "accel/tcg/tcg-runtime.h" +#include "accel/tcg/plugin-helpers.h" #undef IN_HELPER_PROTO diff --git a/include/exec/helper-tcg.h b/include/exec/helper-tcg.h index 27870509a2..6888514635 100644 --- a/include/exec/helper-tcg.h +++ b/include/exec/helper-tcg.h @@ -60,8 +60,8 @@ #include "helper.h" #include "trace/generated-helpers.h" -#include "tcg-runtime.h" -#include "plugin-helpers.h" +#include "accel/tcg/tcg-runtime.h" +#include "accel/tcg/plugin-helpers.h" #undef str #undef DEF_HELPER_FLAGS_0 diff --git a/meson.build b/meson.build index c6f4b0cf5e..d8bb1ec5aa 100644 --- a/meson.build +++ b/meson.build @@ -258,7 +258,6 @@ if not get_option('tcg').disabled() tcg_arch = 'riscv' endif add_project_arguments('-iquote', meson.current_source_dir() / 'tcg' / tcg_arch, - '-iquote', meson.current_source_dir() / 'accel/tcg', language: ['c', 'cpp', 'objc']) accelerators += 'CONFIG_TCG' From c7cefe6c667f706f76f4676ae2894286c5b8b9b3 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Wed, 28 Apr 2021 16:27:56 -0700 Subject: [PATCH 2/5] decodetree: Introduce whex and whexC helpers Form a hex constant of the appropriate insnwidth. Begin using f-strings on changed lines. Reviewed-by: Luis Pires Signed-off-by: Richard Henderson --- scripts/decodetree.py | 66 +++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/scripts/decodetree.py b/scripts/decodetree.py index 4637b633e7..0861e5d503 100644 --- a/scripts/decodetree.py +++ b/scripts/decodetree.py @@ -102,6 +102,21 @@ def str_fields(fields): return r[1:] +def whex(val): + """Return a hex string for val padded for insnwidth""" + global insnwidth + return f'0x{val:0{insnwidth // 4}x}' + + +def whexC(val): + """Return a hex string for val padded for insnwidth, + and with the proper suffix for a C constant.""" + suffix = '' + if val >= 0x80000000: + suffix = 'u' + return whex(val) + suffix + + def str_match_bits(bits, mask): """Return a string pretty-printing BITS/MASK""" global insnwidth @@ -477,11 +492,8 @@ class IncMultiPattern(MultiPattern): if outermask != p.fixedmask: innermask = p.fixedmask & ~outermask innerbits = p.fixedbits & ~outermask - output(ind, 'if ((insn & ', - '0x{0:08x}) == 0x{1:08x}'.format(innermask, innerbits), - ') {\n') - output(ind, ' /* ', - str_match_bits(p.fixedbits, p.fixedmask), ' */\n') + output(ind, f'if ((insn & {whexC(innermask)}) == {whexC(innerbits)}) {{\n') + output(ind, f' /* {str_match_bits(p.fixedbits, p.fixedmask)} */\n') p.output_code(i + 4, extracted, p.fixedbits, p.fixedmask) output(ind, '}\n') else: @@ -500,12 +512,12 @@ class Tree: def str1(self, i): ind = str_indent(i) - r = '{0}{1:08x}'.format(ind, self.fixedmask) + r = ind + whex(self.fixedmask) if self.format: r += ' ' + self.format.name r += ' [\n' for (b, s) in self.subs: - r += '{0} {1:08x}:\n'.format(ind, b) + r += ind + f' {whex(b)}:\n' r += s.str1(i + 4) + '\n' r += ind + ']' return r @@ -529,16 +541,16 @@ class Tree: if sh > 0: # Propagate SH down into the local functions. def str_switch(b, sh=sh): - return '(insn >> {0}) & 0x{1:x}'.format(sh, b >> sh) + return f'(insn >> {sh}) & {b >> sh:#x}' def str_case(b, sh=sh): - return '0x{0:x}'.format(b >> sh) + return hex(b >> sh) else: def str_switch(b): - return 'insn & 0x{0:08x}'.format(b) + return f'insn & {whexC(b)}' def str_case(b): - return '0x{0:08x}'.format(b) + return whexC(b) output(ind, 'switch (', str_switch(self.thismask), ') {\n') for b, s in sorted(self.subs): @@ -962,19 +974,19 @@ def parse_generic(lineno, parent_pat, name, toks): # Validate the masks that we have assembled. if fieldmask & fixedmask: - error(lineno, 'fieldmask overlaps fixedmask (0x{0:08x} & 0x{1:08x})' - .format(fieldmask, fixedmask)) + error(lineno, 'fieldmask overlaps fixedmask ', + f'({whex(fieldmask)} & {whex(fixedmask)})') if fieldmask & undefmask: - error(lineno, 'fieldmask overlaps undefmask (0x{0:08x} & 0x{1:08x})' - .format(fieldmask, undefmask)) + error(lineno, 'fieldmask overlaps undefmask ', + f'({whex(fieldmask)} & {whex(undefmask)})') if fixedmask & undefmask: - error(lineno, 'fixedmask overlaps undefmask (0x{0:08x} & 0x{1:08x})' - .format(fixedmask, undefmask)) + error(lineno, 'fixedmask overlaps undefmask ', + f'({whex(fixedmask)} & {whex(undefmask)})') if not is_format: allbits = fieldmask | fixedmask | undefmask if allbits != insnmask: - error(lineno, 'bits left unspecified (0x{0:08x})' - .format(allbits ^ insnmask)) + error(lineno, 'bits left unspecified ', + f'({whex(allbits ^ insnmask)})') # end parse_general @@ -1104,10 +1116,9 @@ class SizeTree: def str1(self, i): ind = str_indent(i) - r = '{0}{1:08x}'.format(ind, self.mask) - r += ' [\n' + r = ind + whex(self.mask) + ' [\n' for (b, s) in self.subs: - r += '{0} {1:08x}:\n'.format(ind, b) + r += ind + f' {whex(b)}:\n' r += s.str1(i + 4) + '\n' r += ind + ']' return r @@ -1131,16 +1142,16 @@ class SizeTree: if sh > 0: # Propagate SH down into the local functions. def str_switch(b, sh=sh): - return '(insn >> {0}) & 0x{1:x}'.format(sh, b >> sh) + return f'(insn >> {sh}) & {b >> sh:#x}' def str_case(b, sh=sh): - return '0x{0:x}'.format(b >> sh) + return hex(b >> sh) else: def str_switch(b): - return 'insn & 0x{0:08x}'.format(b) + return f'insn & {whexC(b)}' def str_case(b): - return '0x{0:08x}'.format(b) + return whexC(b) output(ind, 'switch (', str_switch(self.mask), ') {\n') for b, s in sorted(self.subs): @@ -1162,8 +1173,7 @@ class SizeLeaf: self.width = w def str1(self, i): - ind = str_indent(i) - return '{0}{1:08x}'.format(ind, self.mask) + return str_indent(i) + whex(self.mask) def __str__(self): return self.str1(0) From 9f6e2b4d34363c066a0cee1a1df3ec7a0c9f3255 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Wed, 28 Apr 2021 16:37:02 -0700 Subject: [PATCH 3/5] decodetree: More use of f-strings Reviewed-by: Luis Pires Signed-off-by: Richard Henderson --- scripts/decodetree.py | 50 ++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/scripts/decodetree.py b/scripts/decodetree.py index 0861e5d503..d5da101167 100644 --- a/scripts/decodetree.py +++ b/scripts/decodetree.py @@ -59,9 +59,9 @@ def error_with_file(file, lineno, *args): prefix = '' if file: - prefix += '{0}:'.format(file) + prefix += f'{file}:' if lineno: - prefix += '{0}:'.format(lineno) + prefix += f'{lineno}:' if prefix: prefix += ' ' print(prefix, end='error: ', file=sys.stderr) @@ -203,7 +203,7 @@ class Field: extr = 'sextract32' else: extr = 'extract32' - return '{0}(insn, {1}, {2})'.format(extr, self.pos, self.len) + return f'{extr}(insn, {self.pos}, {self.len})' def __eq__(self, other): return self.sign == other.sign and self.mask == other.mask @@ -227,11 +227,11 @@ class MultiField: ret = '0' pos = 0 for f in reversed(self.subs): + ext = f.str_extract() if pos == 0: - ret = f.str_extract() + ret = ext else: - ret = 'deposit32({0}, {1}, {2}, {3})' \ - .format(ret, pos, 32 - pos, f.str_extract()) + ret = f'deposit32({ret}, {pos}, {32 - pos}, {ext})' pos += f.len return ret @@ -675,11 +675,11 @@ def parse_field(lineno, name, toks): subtoks = t.split(':') sign = False else: - error(lineno, 'invalid field token "{0}"'.format(t)) + error(lineno, f'invalid field token "{t}"') po = int(subtoks[0]) le = int(subtoks[1]) if po + le > insnwidth: - error(lineno, 'field {0} too large'.format(t)) + error(lineno, f'field {t} too large') f = Field(sign, po, le) subs.append(f) width += le @@ -724,9 +724,9 @@ def parse_arguments(lineno, name, toks): anyextern = True continue if not re.fullmatch(re_C_ident, t): - error(lineno, 'invalid argument set token "{0}"'.format(t)) + error(lineno, f'invalid argument set token "{t}"') if t in flds: - error(lineno, 'duplicate argument "{0}"'.format(t)) + error(lineno, f'duplicate argument "{t}"') flds.append(t) if name in arguments: @@ -895,14 +895,14 @@ def parse_generic(lineno, parent_pat, name, toks): flen = flen[1:] shift = int(flen, 10) if shift + width > insnwidth: - error(lineno, 'field {0} exceeds insnwidth'.format(fname)) + error(lineno, f'field {fname} exceeds insnwidth') f = Field(sign, insnwidth - width - shift, shift) flds = add_field(lineno, flds, fname, f) fixedbits <<= shift fixedmask <<= shift undefmask <<= shift else: - error(lineno, 'invalid token "{0}"'.format(t)) + error(lineno, f'invalid token "{t}"') width += shift if variablewidth and width < insnwidth and width % 8 == 0: @@ -914,7 +914,7 @@ def parse_generic(lineno, parent_pat, name, toks): # We should have filled in all of the bits of the instruction. elif not (is_format and width == 0) and width != insnwidth: - error(lineno, 'definition has {0} bits'.format(width)) + error(lineno, f'definition has {width} bits') # Do not check for fields overlapping fields; one valid usage # is to be able to duplicate fields via import. @@ -932,8 +932,7 @@ def parse_generic(lineno, parent_pat, name, toks): if arg: for f in flds.keys(): if f not in arg.fields: - error(lineno, 'field {0} not in argument set {1}' - .format(f, arg.name)) + error(lineno, f'field {f} not in argument set {arg.name}') else: arg = infer_argument_set(flds) if name in formats: @@ -960,13 +959,12 @@ def parse_generic(lineno, parent_pat, name, toks): arg = fmt.base for f in flds.keys(): if f not in arg.fields: - error(lineno, 'field {0} not in argument set {1}' - .format(f, arg.name)) + error(lineno, f'field {f} not in argument set {arg.name}') if f in fmt.fields.keys(): - error(lineno, 'field {0} set by format and pattern'.format(f)) + error(lineno, f'field {f} set by format and pattern') for f in arg.fields: if f not in flds.keys() and f not in fmt.fields.keys(): - error(lineno, 'field {0} not initialized'.format(f)) + error(lineno, f'field {f} not initialized') pat = Pattern(name, lineno, fmt, fixedbits, fixedmask, undefmask, fieldmask, flds, width) parent_pat.pats.append(pat) @@ -1097,7 +1095,7 @@ def parse_file(f, parent_pat): elif re.fullmatch(re_pat_ident, name): parse_generic(start_lineno, parent_pat, name, toks) else: - error(lineno, 'invalid token "{0}"'.format(name)) + error(lineno, f'invalid token "{name}"') toks = [] if nesting != 0: @@ -1131,9 +1129,8 @@ class SizeTree: # If we need to load more bytes to test, do so now. if extracted < self.width: - output(ind, 'insn = ', decode_function, - '_load_bytes(ctx, insn, {0}, {1});\n' - .format(extracted // 8, self.width // 8)); + output(ind, f'insn = {decode_function}_load_bytes', + f'(ctx, insn, {extracted // 8}, {self.width // 8});\n') extracted = self.width # Attempt to aid the compiler in producing compact switch statements. @@ -1184,9 +1181,8 @@ class SizeLeaf: # If we need to load more bytes, do so now. if extracted < self.width: - output(ind, 'insn = ', decode_function, - '_load_bytes(ctx, insn, {0}, {1});\n' - .format(extracted // 8, self.width // 8)); + output(ind, f'insn = {decode_function}_load_bytes', + f'(ctx, insn, {extracted // 8}, {self.width // 8});\n') extracted = self.width output(ind, 'return insn;\n') # end SizeLeaf @@ -1220,7 +1216,7 @@ def build_size_tree(pats, width, outerbits, outermask): for p in pats: pnames.append(p.name + ':' + p.file + ':' + str(p.lineno)) error_with_file(pats[0].file, pats[0].lineno, - 'overlapping patterns size {0}:'.format(width), pnames) + f'overlapping patterns size {width}:', pnames) bins = {} for i in pats: From 60c425f328957caaa46da895086cc0ea46865d3d Mon Sep 17 00:00:00 2001 From: Luis Fernando Fujita Pires Date: Wed, 7 Apr 2021 22:18:49 +0000 Subject: [PATCH 4/5] decodetree: Add support for 64-bit instructions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow '64' to be specified for the instruction width command line params and use the appropriate extract and deposit functions in that case. This will be used to implement the new 64-bit Power ISA 3.1 instructions. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Luis Pires Message-Id: [rth: Drop the change to the field type; use bitop_width instead of separate variables for extract/deposit; use "ull" for 64-bit constants.] Reviewed-by: Luis Pires Signed-off-by: Richard Henderson --- scripts/decodetree.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/scripts/decodetree.py b/scripts/decodetree.py index d5da101167..f85da45ee3 100644 --- a/scripts/decodetree.py +++ b/scripts/decodetree.py @@ -27,6 +27,7 @@ import sys import getopt insnwidth = 32 +bitop_width = 32 insnmask = 0xffffffff variablewidth = False fields = {} @@ -112,7 +113,9 @@ def whexC(val): """Return a hex string for val padded for insnwidth, and with the proper suffix for a C constant.""" suffix = '' - if val >= 0x80000000: + if val >= 0x100000000: + suffix = 'ull' + elif val >= 0x80000000: suffix = 'u' return whex(val) + suffix @@ -199,11 +202,9 @@ class Field: return str(self.pos) + ':' + s + str(self.len) def str_extract(self): - if self.sign: - extr = 'sextract32' - else: - extr = 'extract32' - return f'{extr}(insn, {self.pos}, {self.len})' + global bitop_width + s = 's' if self.sign else '' + return f'{s}extract{bitop_width}(insn, {self.pos}, {self.len})' def __eq__(self, other): return self.sign == other.sign and self.mask == other.mask @@ -224,6 +225,7 @@ class MultiField: return str(self.subs) def str_extract(self): + global bitop_width ret = '0' pos = 0 for f in reversed(self.subs): @@ -231,7 +233,7 @@ class MultiField: if pos == 0: ret = ext else: - ret = f'deposit32({ret}, {pos}, {32 - pos}, {ext})' + ret = f'deposit{bitop_width}({ret}, {pos}, {bitop_width - pos}, {ext})' pos += f.len return ret @@ -1270,6 +1272,7 @@ def main(): global insntype global insnmask global decode_function + global bitop_width global variablewidth global anyextern @@ -1299,6 +1302,10 @@ def main(): if insnwidth == 16: insntype = 'uint16_t' insnmask = 0xffff + elif insnwidth == 64: + insntype = 'uint64_t' + insnmask = 0xffffffffffffffff + bitop_width = 64 elif insnwidth != 32: error(0, 'cannot handle insns of width', insnwidth) else: From af93ccacc772019298be4c3e47251cdaa60d0c21 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 29 Apr 2021 10:03:59 -0700 Subject: [PATCH 5/5] decodetree: Extend argument set syntax to allow types Rather than force all structure members to be 'int', allow the type of the member to be specified. Reviewed-by: Luis Pires Signed-off-by: Richard Henderson --- docs/devel/decodetree.rst | 11 ++++--- scripts/decodetree.py | 45 +++++++++++++++++---------- tests/decode/succ_argset_type1.decode | 1 + 3 files changed, 36 insertions(+), 21 deletions(-) create mode 100644 tests/decode/succ_argset_type1.decode diff --git a/docs/devel/decodetree.rst b/docs/devel/decodetree.rst index 74f66bf46e..49ea50c2a7 100644 --- a/docs/devel/decodetree.rst +++ b/docs/devel/decodetree.rst @@ -40,9 +40,6 @@ and returns an integral value extracted from there. A field with no ``unnamed_fields`` and no ``!function`` is in error. -FIXME: the fields of the structure into which this result will be stored -is restricted to ``int``. Which means that we cannot expand 64-bit items. - Field examples: +---------------------------+---------------------------------------------+ @@ -66,9 +63,14 @@ Argument Sets Syntax:: args_def := '&' identifier ( args_elt )+ ( !extern )? - args_elt := identifier + args_elt := identifier (':' identifier)? Each *args_elt* defines an argument within the argument set. +If the form of the *args_elt* contains a colon, the first +identifier is the argument name and the second identifier is +the argument type. If the colon is missing, the argument +type will be ``int``. + Each argument set will be rendered as a C structure "arg_$name" with each of the fields being one of the member arguments. @@ -86,6 +88,7 @@ Argument set examples:: ®3 ra rb rc &loadstore reg base offset + &longldst reg base offset:int64_t Formats diff --git a/scripts/decodetree.py b/scripts/decodetree.py index f85da45ee3..a03dc6b5e3 100644 --- a/scripts/decodetree.py +++ b/scripts/decodetree.py @@ -165,11 +165,15 @@ def is_contiguous(bits): return -1 -def eq_fields_for_args(flds_a, flds_b): - if len(flds_a) != len(flds_b): +def eq_fields_for_args(flds_a, arg): + if len(flds_a) != len(arg.fields): return False + # Only allow inference on default types + for t in arg.types: + if t != 'int': + return False for k, a in flds_a.items(): - if k not in flds_b: + if k not in arg.fields: return False return True @@ -313,10 +317,11 @@ class ParameterField: class Arguments: """Class representing the extracted fields of a format""" - def __init__(self, nm, flds, extern): + def __init__(self, nm, flds, types, extern): self.name = nm self.extern = extern - self.fields = sorted(flds) + self.fields = flds + self.types = types def __str__(self): return self.name + ' ' + str(self.fields) @@ -327,8 +332,8 @@ class Arguments: def output_def(self): if not self.extern: output('typedef struct {\n') - for n in self.fields: - output(' int ', n, ';\n') + for (n, t) in zip(self.fields, self.types): + output(f' {t} {n};\n') output('} ', self.struct_name(), ';\n\n') # end Arguments @@ -719,21 +724,27 @@ def parse_arguments(lineno, name, toks): global anyextern flds = [] + types = [] extern = False - for t in toks: - if re.fullmatch('!extern', t): + for n in toks: + if re.fullmatch('!extern', n): extern = True anyextern = True continue - if not re.fullmatch(re_C_ident, t): - error(lineno, f'invalid argument set token "{t}"') - if t in flds: - error(lineno, f'duplicate argument "{t}"') - flds.append(t) + if re.fullmatch(re_C_ident + ':' + re_C_ident, n): + (n, t) = n.split(':') + elif re.fullmatch(re_C_ident, n): + t = 'int' + else: + error(lineno, f'invalid argument set token "{n}"') + if n in flds: + error(lineno, f'duplicate argument "{n}"') + flds.append(n) + types.append(t) if name in arguments: error(lineno, 'duplicate argument set', name) - arguments[name] = Arguments(name, flds, extern) + arguments[name] = Arguments(name, flds, types, extern) # end parse_arguments @@ -760,11 +771,11 @@ def infer_argument_set(flds): global decode_function for arg in arguments.values(): - if eq_fields_for_args(flds, arg.fields): + if eq_fields_for_args(flds, arg): return arg name = decode_function + str(len(arguments)) - arg = Arguments(name, flds.keys(), False) + arg = Arguments(name, flds.keys(), ['int'] * len(flds), False) arguments[name] = arg return arg diff --git a/tests/decode/succ_argset_type1.decode b/tests/decode/succ_argset_type1.decode new file mode 100644 index 0000000000..ed946b420d --- /dev/null +++ b/tests/decode/succ_argset_type1.decode @@ -0,0 +1 @@ +&asdf b:bool c:uint64_t a