Re: [Qemu-devel] [PATCH v2 1/3] decodetree: Allow !function with no input bits

[Philippe Mathieu-Daudé]

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 <address@hidden>
> ---
>  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 <address@hidden>