* [Qemu-devel] [PULL 1/3] decodetree: Allow !function with no input bits
2019-08-19 15:17 [Qemu-devel] [PULL 0/3] decodetree improvements Richard Henderson
@ 2019-08-19 15:17 ` Richard Henderson
2019-08-19 15:17 ` [Qemu-devel] [PULL 2/3] decodetree: Suppress redundant declaration warnings Richard Henderson
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2019-08-19 15:17 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Call this form a "parameter", returning a value extracted
from the DisasContext.
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
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..f6f7368774 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 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
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PULL 2/3] decodetree: Suppress redundant declaration warnings
2019-08-19 15:17 [Qemu-devel] [PULL 0/3] decodetree improvements Richard Henderson
2019-08-19 15:17 ` [Qemu-devel] [PULL 1/3] decodetree: Allow !function with no input bits Richard Henderson
@ 2019-08-19 15:17 ` Richard Henderson
2019-08-19 15:17 ` [Qemu-devel] [PULL 3/3] target/riscv: Remove redundant declaration pragmas Richard Henderson
2019-08-20 9:19 ` [Qemu-devel] [PULL 0/3] decodetree improvements Peter Maydell
3 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2019-08-19 15:17 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 f6f7368774..d8c59cab60 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] 8+ messages in thread* [Qemu-devel] [PULL 3/3] target/riscv: Remove redundant declaration pragmas
2019-08-19 15:17 [Qemu-devel] [PULL 0/3] decodetree improvements Richard Henderson
2019-08-19 15:17 ` [Qemu-devel] [PULL 1/3] decodetree: Allow !function with no input bits Richard Henderson
2019-08-19 15:17 ` [Qemu-devel] [PULL 2/3] decodetree: Suppress redundant declaration warnings Richard Henderson
@ 2019-08-19 15:17 ` Richard Henderson
2019-08-20 9:19 ` [Qemu-devel] [PULL 0/3] decodetree improvements Peter Maydell
3 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2019-08-19 15:17 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] 8+ messages in thread* Re: [Qemu-devel] [PULL 0/3] decodetree improvements
2019-08-19 15:17 [Qemu-devel] [PULL 0/3] decodetree improvements Richard Henderson
` (2 preceding siblings ...)
2019-08-19 15:17 ` [Qemu-devel] [PULL 3/3] target/riscv: Remove redundant declaration pragmas Richard Henderson
@ 2019-08-20 9:19 ` Peter Maydell
3 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2019-08-20 9:19 UTC (permalink / raw)
To: Richard Henderson; +Cc: QEMU Developers
On Mon, 19 Aug 2019 at 16:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The following changes since commit 3fbd3405d2b0604ea530fc7a1828f19da1e95ff9:
>
> Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-08-17' into staging (2019-08-19 14:14:09 +0100)
>
> are available in the Git repository at:
>
> https://github.com/rth7680/qemu.git tags/pull-dt-20190819
>
> for you to fetch changes up to 59a3a1c0c211640e18b058a1b0444154c4eb6f99:
>
> target/riscv: Remove redundant declaration pragmas (2019-08-19 08:13:14 -0700)
>
> ----------------------------------------------------------------
> Implement parameter fields.
> Push warning pragmas into the generated code.
>
> ----------------------------------------------------------------
> 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
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PULL 0/3] decodetree improvements
2018-10-31 16:53 Richard Henderson
@ 2018-11-02 6:21 ` no-reply
2018-11-02 9:49 ` Peter Maydell
1 sibling, 0 replies; 8+ messages in thread
From: no-reply @ 2018-11-02 6:21 UTC (permalink / raw)
To: richard.henderson; +Cc: famz, qemu-devel, peter.maydell
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20181031165321.4422-1-richard.henderson@linaro.org
Subject: [Qemu-devel] [PULL 0/3] decodetree improvements
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
cb5d408f30 decodetree: Allow multiple input files
68998e1b57 decodetree: Remove "insn" argument from trans_* expanders
572d4f99b8 decodetree: Add !extern flag to argument sets
=== OUTPUT BEGIN ===
Checking PATCH 1/3: decodetree: Add !extern flag to argument sets...
Checking PATCH 2/3: decodetree: Remove "insn" argument from trans_* expanders...
ERROR: spaces required around that '*' (ctx:WxV)
#122: FILE: target/arm/translate-sve.c:372:
+static bool trans_##NAME##_zpzz(DisasContext *s, arg_rprr_esz *a) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#131: FILE: target/arm/translate-sve.c:404:
+static bool trans_SDIV_zpzz(DisasContext *s, arg_rprr_esz *a)
^
ERROR: spaces required around that '*' (ctx:WxV)
#158: FILE: target/arm/translate-sve.c:450:
+static bool trans_##NAME(DisasContext *s, arg_rpr_esz *a) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#167: FILE: target/arm/translate-sve.c:467:
+static bool trans_FABS(DisasContext *s, arg_rpr_esz *a)
^
ERROR: spaces required around that '*' (ctx:WxV)
#331: FILE: target/arm/translate-sve.c:851:
+static bool trans_##NAME(DisasContext *s, arg_rprrr_esz *a) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#1442: FILE: target/arm/translate-sve.c:3981:
+static bool trans_##NAME(DisasContext *s, arg_rprrr_esz *a) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#1950: FILE: target/openrisc/disas.c:55:
+static bool trans_l_##opcode(disassemble_info *info, arg_l_##opcode *a) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#1950: FILE: target/openrisc/disas.c:55:
+static bool trans_l_##opcode(disassemble_info *info, arg_l_##opcode *a) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#1968: FILE: target/openrisc/disas.c:149:
+static bool trans_lf_##opcode##_##suffix(disassemble_info *info, \
^
ERROR: spaces required around that '*' (ctx:WxV)
#1969: FILE: target/openrisc/disas.c:150:
+ arg_lf_##opcode##_##suffix *a) \
^
total: 10 errors, 0 warnings, 2539 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/3: decodetree: Allow multiple input files...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PULL 0/3] decodetree improvements
2018-10-31 16:53 Richard Henderson
2018-11-02 6:21 ` no-reply
@ 2018-11-02 9:49 ` Peter Maydell
1 sibling, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2018-11-02 9:49 UTC (permalink / raw)
To: Richard Henderson; +Cc: QEMU Developers
On 31 October 2018 at 16:53, Richard Henderson
<richard.henderson@linaro.org> wrote:
> The following changes since commit a2e002ff7913ce93aa0f7dbedd2123dce5f1a9cd:
>
> Merge remote-tracking branch 'remotes/vivier2/tags/qemu-trivial-for-3.1-pull-request' into staging (2018-10-30 15:49:55 +0000)
>
> are available in the Git repository at:
>
> https://github.com/rth7680/qemu.git tags/pull-dt-20181031
>
> for you to fetch changes up to 6699ae6a8e74381583622502db8bd47fac381c9e:
>
> decodetree: Allow multiple input files (2018-10-31 16:48:58 +0000)
>
> ----------------------------------------------------------------
> Updates to decodetree.py for risc-v.
>
> ----------------------------------------------------------------
> Richard Henderson (3):
> decodetree: Add !extern flag to argument sets
> decodetree: Remove "insn" argument from trans_* expanders
> decodetree: Allow multiple input files
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread