| Message ID | 20230816145547.477974-3-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg/i386: Improvements to deposit | expand |
On Wed, 16 Aug 2023 at 15:58, Richard Henderson <richard.henderson@linaro.org> wrote: > > Inserting a zero into a value, or inserting a value > into zero at offset 0 my be implemented with AND. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > tcg/optimize.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/tcg/optimize.c b/tcg/optimize.c > index d2156367a3..956114b631 100644 > --- a/tcg/optimize.c > +++ b/tcg/optimize.c > @@ -1279,6 +1279,8 @@ static bool fold_ctpop(OptContext *ctx, TCGOp *op) > > static bool fold_deposit(OptContext *ctx, TCGOp *op) > { > + TCGOpcode and_opc; > + > if (arg_is_const(op->args[1]) && arg_is_const(op->args[2])) { > uint64_t t1 = arg_info(op->args[1])->val; > uint64_t t2 = arg_info(op->args[2])->val; > @@ -1287,6 +1289,39 @@ static bool fold_deposit(OptContext *ctx, TCGOp *op) > return tcg_opt_gen_movi(ctx, op, op->args[0], t1); > } > > + switch (ctx->type) { > + case TCG_TYPE_I32: > + and_opc = INDEX_op_and_i32; > + break; > + case TCG_TYPE_I64: > + and_opc = INDEX_op_and_i64; > + break; > + default: > + g_assert_not_reached(); > + } > + > + if (arg_is_const(op->args[1]) > + && arg_info(op->args[1])->val == 0 > + && op->args[3] == 0) { > + uint64_t mask = MAKE_64BIT_MASK(0, op->args[4]); The docs for the TCG deposit op don't say what the restrictions on the immediate args are, but this will be UB for QEMU if args[4] is 0. Have we already sanitized those somewhere? > + > + op->opc = and_opc; > + op->args[1] = op->args[2]; > + op->args[2] = temp_arg(tcg_constant_internal(ctx->type, mask)); > + ctx->z_mask = mask & arg_info(op->args[1])->z_mask; > + return false; > + } > + > + if (arg_is_const(op->args[2]) > + && arg_info(op->args[2])->val == 0) { > + uint64_t mask = deposit64(-1, op->args[3], op->args[4], 0); > + > + op->opc = and_opc; > + op->args[2] = temp_arg(tcg_constant_internal(ctx->type, mask)); > + ctx->z_mask = mask & arg_info(op->args[1])->z_mask; > + return false; > + } > + > ctx->z_mask = deposit64(arg_info(op->args[1])->z_mask, > op->args[3], op->args[4], > arg_info(op->args[2])->z_mask); > -- thanks -- PMM
On 8/17/23 08:50, Peter Maydell wrote: >> + if (arg_is_const(op->args[1]) >> + && arg_info(op->args[1])->val == 0 >> + && op->args[3] == 0) { >> + uint64_t mask = MAKE_64BIT_MASK(0, op->args[4]); > > The docs for the TCG deposit op don't say what the restrictions on the > immediate args are, but this will be UB for QEMU if args[4] is 0. > Have we already sanitized those somewhere? tcg_gen_deposit_{i32,i64} do so. r~
On Thu, 17 Aug 2023 at 23:07, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 8/17/23 08:50, Peter Maydell wrote: > >> + if (arg_is_const(op->args[1]) > >> + && arg_info(op->args[1])->val == 0 > >> + && op->args[3] == 0) { > >> + uint64_t mask = MAKE_64BIT_MASK(0, op->args[4]); > > > > The docs for the TCG deposit op don't say what the restrictions on the > > immediate args are, but this will be UB for QEMU if args[4] is 0. > > Have we already sanitized those somewhere? > > tcg_gen_deposit_{i32,i64} do so. Cool. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 16/8/23 16:55, Richard Henderson wrote: > Inserting a zero into a value, or inserting a value > into zero at offset 0 my be implemented with AND. Typo "may". > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > tcg/optimize.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/tcg/optimize.c b/tcg/optimize.c > index d2156367a3..956114b631 100644 > --- a/tcg/optimize.c > +++ b/tcg/optimize.c > @@ -1279,6 +1279,8 @@ static bool fold_ctpop(OptContext *ctx, TCGOp *op) > > static bool fold_deposit(OptContext *ctx, TCGOp *op) > { > + TCGOpcode and_opc; > + > if (arg_is_const(op->args[1]) && arg_is_const(op->args[2])) { > uint64_t t1 = arg_info(op->args[1])->val; > uint64_t t2 = arg_info(op->args[2])->val; > @@ -1287,6 +1289,39 @@ static bool fold_deposit(OptContext *ctx, TCGOp *op) > return tcg_opt_gen_movi(ctx, op, op->args[0], t1); > } > > + switch (ctx->type) { > + case TCG_TYPE_I32: > + and_opc = INDEX_op_and_i32; > + break; > + case TCG_TYPE_I64: > + and_opc = INDEX_op_and_i64; > + break; > + default: > + g_assert_not_reached(); > + } > + > + if (arg_is_const(op->args[1]) > + && arg_info(op->args[1])->val == 0 > + && op->args[3] == 0) { /* Inserting a value into zero at offset 0. */ > + uint64_t mask = MAKE_64BIT_MASK(0, op->args[4]); > + > + op->opc = and_opc; > + op->args[1] = op->args[2]; > + op->args[2] = temp_arg(tcg_constant_internal(ctx->type, mask)); > + ctx->z_mask = mask & arg_info(op->args[1])->z_mask; > + return false; > + } > + > + if (arg_is_const(op->args[2]) > + && arg_info(op->args[2])->val == 0) { /* Inserting a zero into a value. */ > + uint64_t mask = deposit64(-1, op->args[3], op->args[4], 0); > + > + op->opc = and_opc; > + op->args[2] = temp_arg(tcg_constant_internal(ctx->type, mask)); > + ctx->z_mask = mask & arg_info(op->args[1])->z_mask; > + return false; > + } Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/tcg/optimize.c b/tcg/optimize.c index d2156367a3..956114b631 100644 --- a/tcg/optimize.c +++ b/tcg/optimize.c @@ -1279,6 +1279,8 @@ static bool fold_ctpop(OptContext *ctx, TCGOp *op) static bool fold_deposit(OptContext *ctx, TCGOp *op) { + TCGOpcode and_opc; + if (arg_is_const(op->args[1]) && arg_is_const(op->args[2])) { uint64_t t1 = arg_info(op->args[1])->val; uint64_t t2 = arg_info(op->args[2])->val; @@ -1287,6 +1289,39 @@ static bool fold_deposit(OptContext *ctx, TCGOp *op) return tcg_opt_gen_movi(ctx, op, op->args[0], t1); } + switch (ctx->type) { + case TCG_TYPE_I32: + and_opc = INDEX_op_and_i32; + break; + case TCG_TYPE_I64: + and_opc = INDEX_op_and_i64; + break; + default: + g_assert_not_reached(); + } + + if (arg_is_const(op->args[1]) + && arg_info(op->args[1])->val == 0 + && op->args[3] == 0) { + uint64_t mask = MAKE_64BIT_MASK(0, op->args[4]); + + op->opc = and_opc; + op->args[1] = op->args[2]; + op->args[2] = temp_arg(tcg_constant_internal(ctx->type, mask)); + ctx->z_mask = mask & arg_info(op->args[1])->z_mask; + return false; + } + + if (arg_is_const(op->args[2]) + && arg_info(op->args[2])->val == 0) { + uint64_t mask = deposit64(-1, op->args[3], op->args[4], 0); + + op->opc = and_opc; + op->args[2] = temp_arg(tcg_constant_internal(ctx->type, mask)); + ctx->z_mask = mask & arg_info(op->args[1])->z_mask; + return false; + } + ctx->z_mask = deposit64(arg_info(op->args[1])->z_mask, op->args[3], op->args[4], arg_info(op->args[2])->z_mask);
Inserting a zero into a value, or inserting a value into zero at offset 0 my be implemented with AND. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- tcg/optimize.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)