Philippe,
Assuming that we move into this direction for the vendor-extensions, I
would like to also see some of the other conditionally available extensions
in RISC-V to move to predicated decode (which would move this mechanism
past overlapping multi-patterns).
E.g., we have the following clumsiness for the Zb[abcs] extensions:
#define REQUIRE_ZBB(ctx) do { \
> if (!RISCV_CPU(ctx->cs)->cfg.ext_zbb) { \
> return false; \
> } \
> } while (0)
> static bool trans_clz(DisasContext *ctx, arg_clz *a)
> {
> REQUIRE_ZBB(ctx);
> return gen_unary_per_ol(ctx, a, EXT_NONE, gen_clz, gen_clzw);
> }
Besides being easier to express in the table (as "|has_zbb_p"), this will
have no performance impact (as the predicate is otherwise evaluated every
time the trans_* function is invoked).
I intentionally did not include this at this stage, as the relative benefit
here is small, and changing decodetree.py for it is unjustified.
If (however) we have the mechanism in decodetree.py to support the
custom/vendor-defined extensions, this is a natural and obvious next step,
though.
Philipp.
On Mon, 10 Jan 2022 at 10:52, Philipp Tomsich <philipp.toms...@vrull.eu>
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).
>
> Cheers,
> Philipp.
>
>
> On Mon, 10 Jan 2022 at 10:43, Philippe Mathieu-Daudé <f4...@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.toms...@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)
> > >
>
