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

2019-08-18 Thread 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 
> ---
>  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 00..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 00..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    %foo
> 

Reviewed-by: Philippe Mathieu-Daude 



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

2019-08-18 Thread Richard Henderson
Call this form a "parameter", returning a value extracted
from the DisasContext.

Signed-off-by: Richard Henderson 
---
 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 00..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 00..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    %foo
-- 
2.17.1