Re: [PATCH v2 2/2] target/riscv: Add XVentanaCondOps custom extension

[Philipp Tomsich]

On Wed, 19 Jan 2022 at 12:17, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 13/1/22 21:20, Philipp Tomsich wrote:
> > This adds the decoder and translation for the XVentanaCondOps custom
> > extension (vendor-defined by Ventana Micro Systems), which is
> > documented at 
> > https://github.com/ventanamicro/ventana-custom-extensions/releases/download/v1.0.0/ventana-custom-extensions-v1.0.0.pdf
> >
> > This commit then also adds a guard-function (has_XVentanaCondOps_p)
> > and the decoder function to the table of decoders, enabling the
> > support for the XVentanaCondOps extension.
> >
> > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> >
> > ---
> >
> > Changes in v2:
> > - Split off decode table into XVentanaCondOps.decode
> > - Wire up XVentanaCondOps in the decoder-table
>
> >   static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t 
> > opcode)
> >   {
> > @@ -862,6 +874,7 @@ static void decode_opc(CPURISCVState *env, DisasContext 
> > *ctx, uint16_t opcode)
> >           bool (*decode_func)(DisasContext *, uint32_t);
> >       } decoders[] = {
> >           { always_true_p,  decode_insn32 },
>
> "always_true" is the first entry,
>
> > +        { has_XVentanaCondOps_p,  decode_XVentanaCodeOps },
>
> so is that ever called?

Please refer to patch 1/2:
1. The guard-function only gates whether a decoder function is enabled/called.
2. Enabled decoders are iterated over until a decoder handles the
instruction-word—or we run out of decoders.
3. If no enabled decoder handled an instruction word, we raise an
illegal instruction.

This really is just a table-based form of the what would be equivalent
to the following pseudocode:
   if (guard_func_1() && decoder1(…))
     /* pass */ ;
   else if (guard_func_2() && decoder2(...))
     /* pass */ ;
   [...]
   else
     raise_illegal();

And just as an aside (before we start discussing performance), let's
make sure we all agree that this is perfectly optimizable (I may be
missing a 'const') by a compiler:
1. The number of entries in the array are known at compile-time and
small integer — a compiler can thus peel the loop.
2. The function pointers are in the same compilation unit, so this can
be converted from indirect to direct calls (a special case of
constant-propagation).
3. Predicate functions (given that they will be very small) can be inlined.

Best,
Philipp.

>
>
> >       };
> >
> >       /* Check for compressed insn */
>