* [Qemu-devel] [PATCH v2 0/3] decodetree improvements @ 2019-08-18 6:39 Richard Henderson 2019-08-18 6:39 ` [Qemu-devel] [PATCH v2 1/3] decodetree: Allow !function with no input bits Richard Henderson ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Richard Henderson @ 2019-08-18 6:39 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell Changes from v1: * Drop unintended hunk from MultiField constructor. * Typo fixed in ParameterField documentation. Blurb from v1: These are split out from my decodetree coversion of the AArch32 base instruction sets. The first patch has been tidied per review from Peter. I now diagnose nonsense fields containing no bits. I eliminated the unused integer argument passed to the named function. I improved the documentation. The second patch is new, a suggestion from Phillipe. This then enables the third patch, tidying up the existing usage in riscv. r~ Richard Henderson (3): decodetree: Allow !function with no input bits decodetree: Suppress redundant declaration warnings target/riscv: Remove redundant declaration pragmas target/riscv/translate.c | 19 +-------- docs/devel/decodetree.rst | 8 +++- scripts/decodetree.py | 71 ++++++++++++++++++++++++++----- tests/decode/err_field6.decode | 5 +++ tests/decode/succ_function.decode | 6 +++ 5 files changed, 79 insertions(+), 30 deletions(-) create mode 100644 tests/decode/err_field6.decode create mode 100644 tests/decode/succ_function.decode -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 1/3] decodetree: Allow !function with no input bits 2019-08-18 6:39 [Qemu-devel] [PATCH v2 0/3] decodetree improvements Richard Henderson @ 2019-08-18 6:39 ` Richard Henderson 2019-08-18 21:49 ` Philippe Mathieu-Daudé 2019-08-18 6:39 ` [Qemu-devel] [PATCH v2 2/3] decodetree: Suppress redundant declaration warnings Richard Henderson 2019-08-18 6:39 ` [Qemu-devel] [PATCH v2 3/3] target/riscv: Remove redundant declaration pragmas Richard Henderson 2 siblings, 1 reply; 5+ messages in thread From: Richard Henderson @ 2019-08-18 6:39 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell Call this form a "parameter", returning a value extracted from the DisasContext. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- docs/devel/decodetree.rst | 8 ++++- scripts/decodetree.py | 49 ++++++++++++++++++++++++------- tests/decode/err_field6.decode | 5 ++++ tests/decode/succ_function.decode | 6 ++++ 4 files changed, 56 insertions(+), 12 deletions(-) create mode 100644 tests/decode/err_field6.decode create mode 100644 tests/decode/succ_function.decode diff --git a/docs/devel/decodetree.rst b/docs/devel/decodetree.rst index 44ac621ea8..ce7f52308f 100644 --- a/docs/devel/decodetree.rst +++ b/docs/devel/decodetree.rst @@ -23,7 +23,7 @@ Fields Syntax:: - field_def := '%' identifier ( unnamed_field )+ ( !function=identifier )? + field_def := '%' identifier ( unnamed_field )* ( !function=identifier )? unnamed_field := number ':' ( 's' ) number For *unnamed_field*, the first number is the least-significant bit position @@ -34,6 +34,12 @@ present, they are concatenated. In this way one can define disjoint fields. If ``!function`` is specified, the concatenated result is passed through the named function, taking and returning an integral value. +One may use ``!function`` with zero ``unnamed_fields``. This case is called +a *parameter*, and the named function is only passed the ``DisasContext`` +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. diff --git a/scripts/decodetree.py b/scripts/decodetree.py index d7a59d63ac..31e2f04ecb 100755 --- a/scripts/decodetree.py +++ b/scripts/decodetree.py @@ -245,7 +245,7 @@ class ConstField: class FunctionField: - """Class representing a field passed through an expander""" + """Class representing a field passed through a function""" def __init__(self, func, base): self.mask = base.mask self.sign = base.sign @@ -266,6 +266,27 @@ class FunctionField: # end FunctionField +class ParameterField: + """Class representing a pseudo-field read from a function""" + def __init__(self, func): + self.mask = 0 + self.sign = 0 + self.func = func + + def __str__(self): + return self.func + + def str_extract(self): + return self.func + '(ctx)' + + def __eq__(self, other): + return self.func == other.func + + def __ne__(self, other): + return not self.__eq__(other) +# end FunctionField + + class Arguments: """Class representing the extracted fields of a format""" def __init__(self, nm, flds, extern): @@ -433,17 +454,23 @@ def parse_field(lineno, name, toks): if width > insnwidth: error(lineno, 'field too large') - if len(subs) == 1: - f = subs[0] + if len(subs) == 0: + if func: + f = ParameterField(func) + else: + error(lineno, 'field with no value') else: - mask = 0 - for s in subs: - if mask & s.mask: - error(lineno, 'field components overlap') - mask |= s.mask - f = MultiField(subs, mask) - if func: - f = FunctionField(func, f) + if len(subs) == 1: + f = subs[0] + else: + mask = 0 + for s in subs: + if mask & s.mask: + error(lineno, 'field components overlap') + mask |= s.mask + f = MultiField(subs, mask) + if func: + f = FunctionField(func, f) if name in fields: error(lineno, 'duplicate field', name) diff --git a/tests/decode/err_field6.decode b/tests/decode/err_field6.decode new file mode 100644 index 0000000000..a719884572 --- /dev/null +++ b/tests/decode/err_field6.decode @@ -0,0 +1,5 @@ +# This work is licensed under the terms of the GNU LGPL, version 2 or later. +# See the COPYING.LIB file in the top-level directory. + +# Diagnose no bits in field +%field diff --git a/tests/decode/succ_function.decode b/tests/decode/succ_function.decode new file mode 100644 index 0000000000..7751b1784e --- /dev/null +++ b/tests/decode/succ_function.decode @@ -0,0 +1,6 @@ +# This work is licensed under the terms of the GNU LGPL, version 2 or later. +# See the COPYING.LIB file in the top-level directory. + +# "Field" as parameter pulled from DisasContext. +%foo !function=foo +foo 00000000000000000000000000000000 %foo -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] decodetree: Allow !function with no input bits 2019-08-18 6:39 ` [Qemu-devel] [PATCH v2 1/3] decodetree: Allow !function with no input bits Richard Henderson @ 2019-08-18 21:49 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2019-08-18 21:49 UTC (permalink / raw) To: Richard Henderson, qemu-devel; +Cc: peter.maydell On 8/18/19 8:39 AM, Richard Henderson wrote: > Call this form a "parameter", returning a value extracted > from the DisasContext. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > docs/devel/decodetree.rst | 8 ++++- > scripts/decodetree.py | 49 ++++++++++++++++++++++++------- > tests/decode/err_field6.decode | 5 ++++ > tests/decode/succ_function.decode | 6 ++++ > 4 files changed, 56 insertions(+), 12 deletions(-) > create mode 100644 tests/decode/err_field6.decode > create mode 100644 tests/decode/succ_function.decode > > diff --git a/docs/devel/decodetree.rst b/docs/devel/decodetree.rst > index 44ac621ea8..ce7f52308f 100644 > --- a/docs/devel/decodetree.rst > +++ b/docs/devel/decodetree.rst > @@ -23,7 +23,7 @@ Fields > > Syntax:: > > - field_def := '%' identifier ( unnamed_field )+ ( !function=identifier )? > + field_def := '%' identifier ( unnamed_field )* ( !function=identifier )? > unnamed_field := number ':' ( 's' ) number > > For *unnamed_field*, the first number is the least-significant bit position > @@ -34,6 +34,12 @@ present, they are concatenated. In this way one can define disjoint fields. > If ``!function`` is specified, the concatenated result is passed through the > named function, taking and returning an integral value. > > +One may use ``!function`` with zero ``unnamed_fields``. This case is called > +a *parameter*, and the named function is only passed the ``DisasContext`` > +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. > > diff --git a/scripts/decodetree.py b/scripts/decodetree.py > index d7a59d63ac..31e2f04ecb 100755 > --- a/scripts/decodetree.py > +++ b/scripts/decodetree.py > @@ -245,7 +245,7 @@ class ConstField: > > > class FunctionField: > - """Class representing a field passed through an expander""" > + """Class representing a field passed through a function""" > def __init__(self, func, base): > self.mask = base.mask > self.sign = base.sign > @@ -266,6 +266,27 @@ class FunctionField: > # end FunctionField > > > +class ParameterField: > + """Class representing a pseudo-field read from a function""" > + def __init__(self, func): > + self.mask = 0 > + self.sign = 0 > + self.func = func > + > + def __str__(self): > + return self.func > + > + def str_extract(self): > + return self.func + '(ctx)' > + > + def __eq__(self, other): > + return self.func == other.func > + > + def __ne__(self, other): > + return not self.__eq__(other) > +# end FunctionField Nit: end ParameterField > + > + > class Arguments: > """Class representing the extracted fields of a format""" > def __init__(self, nm, flds, extern): > @@ -433,17 +454,23 @@ def parse_field(lineno, name, toks): > > if width > insnwidth: > error(lineno, 'field too large') > - if len(subs) == 1: > - f = subs[0] > + if len(subs) == 0: > + if func: > + f = ParameterField(func) > + else: > + error(lineno, 'field with no value') > else: > - mask = 0 > - for s in subs: > - if mask & s.mask: > - error(lineno, 'field components overlap') > - mask |= s.mask > - f = MultiField(subs, mask) > - if func: > - f = FunctionField(func, f) > + if len(subs) == 1: > + f = subs[0] > + else: > + mask = 0 > + for s in subs: > + if mask & s.mask: > + error(lineno, 'field components overlap') > + mask |= s.mask > + f = MultiField(subs, mask) > + if func: > + f = FunctionField(func, f) > > if name in fields: > error(lineno, 'duplicate field', name) > diff --git a/tests/decode/err_field6.decode b/tests/decode/err_field6.decode > new file mode 100644 > index 0000000000..a719884572 > --- /dev/null > +++ b/tests/decode/err_field6.decode > @@ -0,0 +1,5 @@ > +# This work is licensed under the terms of the GNU LGPL, version 2 or later. > +# See the COPYING.LIB file in the top-level directory. > + > +# Diagnose no bits in field > +%field > diff --git a/tests/decode/succ_function.decode b/tests/decode/succ_function.decode > new file mode 100644 > index 0000000000..7751b1784e > --- /dev/null > +++ b/tests/decode/succ_function.decode > @@ -0,0 +1,6 @@ > +# This work is licensed under the terms of the GNU LGPL, version 2 or later. > +# See the COPYING.LIB file in the top-level directory. > + > +# "Field" as parameter pulled from DisasContext. > +%foo !function=foo > +foo 00000000000000000000000000000000 %foo > Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] decodetree: Suppress redundant declaration warnings 2019-08-18 6:39 [Qemu-devel] [PATCH v2 0/3] decodetree improvements Richard Henderson 2019-08-18 6:39 ` [Qemu-devel] [PATCH v2 1/3] decodetree: Allow !function with no input bits Richard Henderson @ 2019-08-18 6:39 ` Richard Henderson 2019-08-18 6:39 ` [Qemu-devel] [PATCH v2 3/3] target/riscv: Remove redundant declaration pragmas Richard Henderson 2 siblings, 0 replies; 5+ messages in thread From: Richard Henderson @ 2019-08-18 6:39 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell We can tell that a decodetree input file is "secondary" when it uses an argument set marked "!extern". This indicates that at least one of the insn translation functions will have already been declared by the "primary" input file, but given only the secondary we cannot tell which. Avoid redundant declaration warnings by suppressing them with pragmas. Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- scripts/decodetree.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/scripts/decodetree.py b/scripts/decodetree.py index 31e2f04ecb..12c1cd0ddb 100755 --- a/scripts/decodetree.py +++ b/scripts/decodetree.py @@ -33,6 +33,7 @@ arguments = {} formats = {} patterns = [] allpatterns = [] +anyextern = False translate_prefix = 'trans' translate_scope = 'static ' @@ -482,12 +483,14 @@ def parse_arguments(lineno, name, toks): """Parse one argument set from TOKS at LINENO""" global arguments global re_ident + global anyextern flds = [] extern = False for t in toks: if re_fullmatch('!extern', t): extern = True + anyextern = True continue if not re_fullmatch(re_ident, t): error(lineno, 'invalid argument set token "{0}"'.format(t)) @@ -1188,6 +1191,7 @@ def main(): global insnmask global decode_function global variablewidth + global anyextern decode_scope = 'static ' @@ -1248,6 +1252,19 @@ def main(): # A single translate function can be invoked for different patterns. # Make sure that the argument sets are the same, and declare the # function only once. + # + # If we're sharing formats, we're likely also sharing trans_* functions, + # but we can't tell which ones. Prevent issues from the compiler by + # suppressing redundant declaration warnings. + if anyextern: + output("#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE\n", + "# pragma GCC diagnostic push\n", + "# pragma GCC diagnostic ignored \"-Wredundant-decls\"\n", + "# ifdef __clang__\n" + "# pragma GCC diagnostic ignored \"-Wtypedef-redefinition\"\n", + "# endif\n", + "#endif\n\n") + out_pats = {} for i in allpatterns: if i.name in out_pats: @@ -1259,6 +1276,11 @@ def main(): out_pats[i.name] = i output('\n') + if anyextern: + output("#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE\n", + "# pragma GCC diagnostic pop\n", + "#endif\n\n") + for n in sorted(formats.keys()): f = formats[n] f.output_extract() -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] target/riscv: Remove redundant declaration pragmas 2019-08-18 6:39 [Qemu-devel] [PATCH v2 0/3] decodetree improvements Richard Henderson 2019-08-18 6:39 ` [Qemu-devel] [PATCH v2 1/3] decodetree: Allow !function with no input bits Richard Henderson 2019-08-18 6:39 ` [Qemu-devel] [PATCH v2 2/3] decodetree: Suppress redundant declaration warnings Richard Henderson @ 2019-08-18 6:39 ` Richard Henderson 2 siblings, 0 replies; 5+ messages in thread From: Richard Henderson @ 2019-08-18 6:39 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell These are now generated by decodetree itself. Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Acked-by: Palmer Dabbelt <palmer@sifive.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/riscv/translate.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 8d6ab73258..adeddb85f6 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -708,26 +708,9 @@ static bool gen_shift(DisasContext *ctx, arg_r *a, #include "insn_trans/trans_rvd.inc.c" #include "insn_trans/trans_privileged.inc.c" -/* - * Auto-generated decoder. - * Note that the 16-bit decoder reuses some of the trans_* functions - * initially declared by the 32-bit decoder, which results in duplicate - * declaration warnings. Suppress them. - */ -#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wredundant-decls" -# ifdef __clang__ -# pragma GCC diagnostic ignored "-Wtypedef-redefinition" -# endif -#endif - +/* Include the auto-generated decoder for 16 bit insn */ #include "decode_insn16.inc.c" -#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE -# pragma GCC diagnostic pop -#endif - static void decode_opc(DisasContext *ctx) { /* check for compressed insn */ -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-08-18 21:50 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-18 6:39 [Qemu-devel] [PATCH v2 0/3] decodetree improvements Richard Henderson 2019-08-18 6:39 ` [Qemu-devel] [PATCH v2 1/3] decodetree: Allow !function with no input bits Richard Henderson 2019-08-18 21:49 ` Philippe Mathieu-Daudé 2019-08-18 6:39 ` [Qemu-devel] [PATCH v2 2/3] decodetree: Suppress redundant declaration warnings Richard Henderson 2019-08-18 6:39 ` [Qemu-devel] [PATCH v2 3/3] target/riscv: Remove redundant declaration pragmas Richard Henderson
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 NNTP newsgroup(s).