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

[Philippe Mathieu-Daudé]

On 1/10/22 10:52, Philipp Tomsich wrote:
> For RISC-V the opcode decode will change between different vendor
> implementations of RISC-V (emulated by the same qemu binary).
> Any two vendors may reuse the same opcode space, e.g., we may end up with:
> 
> # *** RV64 Custom-3 Extension ***
> {
>   vt_maskc   0000000  ..... ..... 110 ..... 1111011 @r |has_xventanacondops_p
>   vt_maskcn  0000000  ..... ..... 111 ..... 1111011 @r |has_xventanacondops_p
>   someone_something  ............ ..... 000 ..... 1100111 @i
> |has_xsomeonesomething_p
> }
> 
> With extensions being enabled either from the commandline
>     -cpu any,xventanacondops=true
> or possibly even having a AMP in one emulation setup (e.g. application
> cores having one extension and power-mangement cores having a
> different one — or even a conflicting one).

I understand, I think this is what MIPS does, see commit 9d005392390:
("target/mips: Introduce decodetree structure for NEC Vr54xx extension")

> 
> Cheers,
> Philipp.
> 
> 
> On Mon, 10 Jan 2022 at 10:43, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> 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)
>>>