Re: [PATCH v1 1/2] decodetree: Add an optional predicate-function for decoding

[Philippe Mathieu-Daudé]

Hi Philipp,

On 1/9/22 21:56, Philipp Tomsich wrote:
> This adds the possibility to specify a predicate-function that is
> called as part of decoding in multi-patterns; it is intended for
> use-cases (such as vendor-defined instructions in RISC-V) where the
> same bitpattern may decode into different functions depending on the
> overall configuration of the emulation target.

But for a particular CPU, its "vendor ISAs" won't change at runtime.

Since we know this at build time, I don't understand why you need
predicate support at all.

> 
> At this time, we only support predicates for multi-patterns.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> 
> ---
> 
>  docs/devel/decodetree.rst |  7 ++++++-
>  scripts/decodetree.py     | 24 +++++++++++++++++++++---
>  2 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/devel/decodetree.rst b/docs/devel/decodetree.rst
> index 49ea50c2a7..241aaec8bb 100644
> --- a/docs/devel/decodetree.rst
> +++ b/docs/devel/decodetree.rst
> @@ -144,9 +144,10 @@ Patterns
>  Syntax::
>  
>    pat_def      := identifier ( pat_elt )+
> -  pat_elt      := fixedbit_elt | field_elt | field_ref | args_ref | fmt_ref 
> | const_elt
> +  pat_elt      := fixedbit_elt | field_elt | field_ref | args_ref | fmt_ref 
> | const_elt | predicate
>    fmt_ref      := '@' identifier
>    const_elt    := identifier '=' number
> +  predicate    := '|' identifier
>  
>  The *fixedbit_elt* and *field_elt* specifiers are unchanged from formats.
>  A pattern that does not specify a named format will have one inferred
> @@ -156,6 +157,10 @@ A *const_elt* allows a argument to be set to a constant 
> value.  This may
>  come in handy when fields overlap between patterns and one has to
>  include the values in the *fixedbit_elt* instead.
>  
> +A *predicate* allows to specify a predicate function (returing true or
> +false) to determine the applicability of the pattern.  Currently, this
> +will change the decode-behaviour  for overlapping multi-patterns only.
> +
>  The decoder will call a translator function for each pattern matched.
>  
>  Pattern examples::
> diff --git a/scripts/decodetree.py b/scripts/decodetree.py
> index a03dc6b5e3..7da2282411 100644
> --- a/scripts/decodetree.py
> +++ b/scripts/decodetree.py
> @@ -52,6 +52,7 @@
>  re_fld_ident = '%[a-zA-Z0-9_]*'
>  re_fmt_ident = '@[a-zA-Z0-9_]*'
>  re_pat_ident = '[a-zA-Z0-9_]*'
> +re_predicate_ident = '\|[a-zA-Z_][a-zA-Z0-9_]*'
>  
>  def error_with_file(file, lineno, *args):
>      """Print an error message from file:line and args and exit."""
> @@ -119,6 +120,14 @@ def whexC(val):
>          suffix = 'u'
>      return whex(val) + suffix
>  
> +def predicate(val):
> +    """Return a string for calling a predicate function
> +       (if specified, accepting 'None' as an indication
> +       that no predicate is to be emitted) with the ctx
> +       as a parameter."""
> +    if (val == None):
> +        return ''
> +    return ' && ' + val + '(ctx)'
>  
>  def str_match_bits(bits, mask):
>      """Return a string pretty-printing BITS/MASK"""
> @@ -340,7 +349,7 @@ def output_def(self):
>  
>  class General:
>      """Common code between instruction formats and instruction patterns"""
> -    def __init__(self, name, lineno, base, fixb, fixm, udfm, fldm, flds, w):
> +    def __init__(self, name, lineno, base, fixb, fixm, udfm, fldm, flds, w, 
> p = None):
>          self.name = name
>          self.file = input_file
>          self.lineno = lineno
> @@ -351,6 +360,7 @@ def __init__(self, name, lineno, base, fixb, fixm, udfm, 
> fldm, flds, w):
>          self.fieldmask = fldm
>          self.fields = flds
>          self.width = w
> +        self.predicate = p
>  
>      def __str__(self):
>          return self.name + ' ' + str_match_bits(self.fixedbits, 
> self.fixedmask)
> @@ -499,7 +509,7 @@ def output_code(self, i, extracted, outerbits, outermask):
>              if outermask != p.fixedmask:
>                  innermask = p.fixedmask & ~outermask
>                  innerbits = p.fixedbits & ~outermask
> -                output(ind, f'if ((insn & {whexC(innermask)}) == 
> {whexC(innerbits)}) {{\n')
> +                output(ind, f'if ((insn & {whexC(innermask)}) == 
> {whexC(innerbits)}{predicate(p.predicate)}) {{\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')
> @@ -826,6 +836,7 @@ def parse_generic(lineno, parent_pat, name, toks):
>      global re_fld_ident
>      global re_fmt_ident
>      global re_C_ident
> +    global re_predicate_ident
>      global insnwidth
>      global insnmask
>      global variablewidth
> @@ -839,6 +850,7 @@ def parse_generic(lineno, parent_pat, name, toks):
>      flds = {}
>      arg = None
>      fmt = None
> +    predicate = None
>      for t in toks:
>          # '&Foo' gives a format an explicit argument set.
>          if re.fullmatch(re_arg_ident, t):
> @@ -881,6 +893,12 @@ def parse_generic(lineno, parent_pat, name, toks):
>              flds = add_field(lineno, flds, fname, ConstField(value))
>              continue
>  
> +        # '|predicate' sets a predicate function to be called.
> +        if re.fullmatch(re_predicate_ident, t):
> +            tt = t[1:]
> +            predicate = tt;
> +            continue
> +
>          # Pattern of 0s, 1s, dots and dashes indicate required zeros,
>          # required ones, or dont-cares.
>          if re.fullmatch('[01.-]+', t):
> @@ -979,7 +997,7 @@ def parse_generic(lineno, parent_pat, name, toks):
>              if f not in flds.keys() and f not in fmt.fields.keys():
>                  error(lineno, f'field {f} not initialized')
>          pat = Pattern(name, lineno, fmt, fixedbits, fixedmask,
> -                      undefmask, fieldmask, flds, width)
> +                      undefmask, fieldmask, flds, width, predicate)
>          parent_pat.pats.append(pat)
>          allpatterns.append(pat)
>