Hi Richard, thank you for the feedback.

> On 8/20/25 04:21, Kohei Tokunaga wrote:
> > The tcg_out_extract and tcg_out_sextract functions were used by several
> > other functions (e.g. tcg_out_ext*) and intended to emit TCI code. So
they
> > have been renamed to tcg_tci_out_extract and tcg_tci_out_sextract.
> >
> > Signed-off-by: Kohei Tokunaga <ktokunaga.m...@gmail.com>
> > ---
> >   tcg/wasm/tcg-target.c.inc | 104 +++++++++++++++++++++++++++++++++-----
> >   1 file changed, 91 insertions(+), 13 deletions(-)
> >
> > diff --git a/tcg/wasm/tcg-target.c.inc b/tcg/wasm/tcg-target.c.inc
> > index 03cb3b2f46..6220b43f98 100644
> > --- a/tcg/wasm/tcg-target.c.inc
> > +++ b/tcg/wasm/tcg-target.c.inc
> > @@ -163,7 +163,10 @@ typedef enum {
> >       OPC_I64_SHR_U = 0x88,
> >
> >       OPC_I32_WRAP_I64 = 0xa7,
> > +    OPC_I64_EXTEND_I32_S = 0xac,
> >       OPC_I64_EXTEND_I32_U = 0xad,
> > +    OPC_I64_EXTEND8_S = 0xc2,
> > +    OPC_I64_EXTEND16_S = 0xc3,
> >   } WasmInsn;
> >
> >   typedef enum {
> > @@ -380,6 +383,66 @@ static void tcg_wasm_out_movcond(TCGContext *s,
TCGType type, TCGReg ret,
> >       tcg_wasm_out_op_idx(s, OPC_GLOBAL_SET, REG_IDX(ret));
> >   }
> >
> > +static void tcg_wasm_out_deposit(TCGContext *s,
> > +                                 TCGReg dest, TCGReg arg1, TCGReg arg2,
> > +                                 int pos, int len)
> > +{
> > +    int64_t mask = (((int64_t)1 << len) - 1) << pos;
> > +    tcg_wasm_out_op_idx(s, OPC_GLOBAL_GET, REG_IDX(arg1));
> > +    tcg_wasm_out_op_const(s, OPC_I64_CONST, ~mask);
> > +    tcg_wasm_out_op(s, OPC_I64_AND);
> > +    tcg_wasm_out_op_idx(s, OPC_GLOBAL_GET, REG_IDX(arg2));
> > +    tcg_wasm_out_op_const(s, OPC_I64_CONST, pos);
> > +    tcg_wasm_out_op(s, OPC_I64_SHL);
> > +    tcg_wasm_out_op_const(s, OPC_I64_CONST, mask);
> > +    tcg_wasm_out_op(s, OPC_I64_AND);
> > +    tcg_wasm_out_op(s, OPC_I64_OR);
> > +    tcg_wasm_out_op_idx(s, OPC_GLOBAL_SET, REG_IDX(dest));
> > +}
> > +
> > +static void tcg_wasm_out_extract(TCGContext *s, TCGReg dest, TCGReg
arg1,
> > +                                 int pos, int len)
> > +{
> > +    int64_t mask = ~0ULL >> (64 - len);
> > +    tcg_wasm_out_op_idx(s, OPC_GLOBAL_GET, REG_IDX(arg1));
> > +    if (pos > 0) {
> > +        tcg_wasm_out_op_const(s, OPC_I64_CONST, pos);
> > +        tcg_wasm_out_op(s, OPC_I64_SHR_U);
> > +    }
> > +    if ((pos + len) < 64) {
> > +        tcg_wasm_out_op_const(s, OPC_I64_CONST, mask);
> > +        tcg_wasm_out_op(s, OPC_I64_AND);
> > +    }
> > +    tcg_wasm_out_op_idx(s, OPC_GLOBAL_SET, REG_IDX(dest));
> > +}
>
> This is no better than the generic tcg expansion.
> You should omit it.
>
> > +
> > +static void tcg_wasm_out_sextract(TCGContext *s, TCGReg dest, TCGReg
arg1,
> > +                                  int pos, int len)
> > +{
> > +    int discard = 64 - len;
> > +    int high = discard - pos;
> > +
> > +    tcg_wasm_out_op_idx(s, OPC_GLOBAL_GET, REG_IDX(arg1));
> > +
> > +    if ((pos == 0) && (len == 8)) {
> > +        tcg_wasm_out_op(s, OPC_I64_EXTEND8_S);
> > +    } else if ((pos == 0) && (len == 16)) {
> > +        tcg_wasm_out_op(s, OPC_I64_EXTEND16_S);
> > +    } else if ((pos == 0) && (len == 32)) {
> > +        tcg_wasm_out_op(s, OPC_I32_WRAP_I64);
> > +        tcg_wasm_out_op(s, OPC_I64_EXTEND_I32_S);
>
> This is worth keeping.
> Compare tcg/i386/tcg-target-has.h, tcg_target_sextract_valid.

I'll apply them in the next version of the patch series.

Regards,
Kohei

Reply via email to