RE: [PATCH] ppc/pvn.c: fix "system-id" FDT when -uuid is set

[Luis Fernando Fujita Pires]

From: Daniel Henrique Barboza 
> Subject: [PATCH] ppc/pvn.c: fix "system-id" FDT when -uuid is set

I don't know enough to review this, but there's a typo on the commit message 
(pvn.c -> pnv.c). :)

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: Fwd: New Defects reported by Coverity Scan for QEMU

[Luis Fernando Fujita Pires]

From: Matheus K. Ferst 
> Hi Cédric,
> 
> The only change was the helper name that is now uppercase, so nothing new
> here. The underlying cause is that dfp_finalize_decimal64 only sets
> dfp->vt.VsrD(1) and set_dfp64 receives a pointer to the complete struct.
> 
> But since set_dfp64 also only access VsrD(1), it shouldn't be a real
> problem AFAICT. The same applies to CID 1465776~1465786 and
> 1465788~1465790.

Right. Coverity is probably reporting these as new just because the helper 
macros were re-written as part of the move to decodetree.
I believe these should be marked as false positives.

We *could* also wrap set_dfp{64,128} in new macros that would then reference 
only the appropriate parts of dfp, but, in this case, I don't think it's worth 
the trouble.

Thanks,

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH] tcg: Extend call args using the correct opcodes

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Pretending that the source is i64 when it is in fact i32 is incorrect; we 
> have type-
> changing opcodes that must be used.
> This bug trips up the subsequent change to the optimizer.
> 
> Fixes: 4f2331e5b67a
> Signed-off-by: Richard Henderson 
> ---
> 
> This fixes a problem found in s390x host testing, and should be considered 
> patch
> 41.5/51 in
> 
>   [PATCH v4 00/51] tcg: optimize redundant sign extensions
> 
> just before
> 
>   tcg/optimize: Stop forcing z_mask to "garbage" for 32-bit values
> 
> 
> r~
> 
> ---
>  tcg/tcg.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 51/51] tcg/optimize: Propagate sign info for shifting

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> For constant shifts, we can simply shift the s_mask.
> 
> For variable shifts, we know that sar does not reduce the s_mask, which helps
> for sequences like
> 
> ext32s_i64  t, in
> sar_i64 t, t, v
> ext32s_i64  out, t
> 
> allowing the final extend to be eliminated.
> 
> Reviewed-by: Alex Bennée 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 50 +++--
> -
>  1 file changed, 47 insertions(+), 3 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 49/51] tcg/optimize: Propagate sign info for setcond

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> The result is either 0 or 1, which means that we have a 2 bit signed result, 
> and
> thus 62 bits of sign.
> For clarity, use the smask_from_zmask function.
> 
> Reviewed-by: Alex Bennée 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 50/51] tcg/optimize: Propagate sign info for bit counting

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> The results are generally 6 bit unsigned values, though the count leading and
> trailing bits may produce any value for a zero input.
> 
> Reviewed-by: Alex Bennée 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 47/51] tcg/optimize: Optimize sign extensions

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Certain targets, like riscv, produce signed 32-bit results.
> This can lead to lots of redundant extensions as values are manipulated.
> 
> Begin by tracking only the obvious sign-extensions, and converting them to
> simple copies when possible.
> 
> Reviewed-by: Alex Bennée 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 123 -
>  1 file changed, 102 insertions(+), 21 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 48/51] tcg/optimize: Propagate sign info for logical operations

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Sign repetitions are perforce all identical, whether they are 1 or 0.
> Bitwise operations preserve the relative quantity of the repetitions.
> 
> Reviewed-by: Alex Bennée 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 29 +
>  1 file changed, 29 insertions(+)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 46/51] tcg/optimize: Use fold_xx_to_i for rem

[Luis Fernando Fujita Pires]

From: Richard Henderson 

>  static bool fold_remainder(OptContext *ctx, TCGOp *op)  {
> -return fold_const2(ctx, op);
> +if (fold_const2(ctx, op) ||
> +fold_xx_to_i(ctx, op, 1)) {

Should this be fold_xx_to_i(ctx, op, 0)?
If arg[2] is 0, we would have different results than do_constant_folding(), but 
not sure we care, since the result is documented as undefined.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 43/51] tcg/optimize: Use fold_xx_to_i for orc

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Recognize the constant function for or-compliment.

Minor typo: or-complement.

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 45/51] tcg/optimize: Use fold_xi_to_x for div

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Recognize the identity function for division.
> 
> Suggested-by: Luis Pires 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 44/51] tcg/optimize: Use fold_xi_to_x for mul

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Recognize the identity function for low-part multiply.
> 
> Suggested-by: Luis Pires 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 41/51] tcg/optimize: Sink commutative operand swapping into fold functions

[Luis Fernando Fujita Pires]

From: Richard Henderson 

>  static bool fold_add2(OptContext *ctx, TCGOp *op)  {
> +swap_commutative(op->args[0], &op->args[2], &op->args[4]);
> +swap_commutative(op->args[1], &op->args[3], &op->args[5]);

This was existing code, but I would've understood it easier if it had a comment 
noting that, even though it would be possible for this code to swap args[2] <-> 
args[4] and not args[3] <-> arg5 (and vice-versa), this would be okay for an 
add. :)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 42/51] tcg/optimize: Stop forcing z_mask to "garbage" for 32-bit values

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> This "garbage" setting pre-dates the addition of the type changing opcodes
> INDEX_op_ext_i32_i64, INDEX_op_extu_i32_i64, and
> INDEX_op_extr{l,h}_i64_i32.
> 
> So now we have a definitive points at which to adjust z_mask to eliminate such
> bits from the 32-bit operands.
> 
> Reviewed-by: Alex Bennée 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 35 ---
>  1 file changed, 16 insertions(+), 19 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 40/51] tcg/optimize: Expand fold_addsub2_i32 to 64-bit ops

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Rename to fold_addsub2.
> Use Int128 to implement the wider operation.
> 
> Reviewed-by: Alex Bennée 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 65 ++
>  1 file changed, 44 insertions(+), 21 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 39/51] tcg/optimize: Expand fold_mulu2_i32 to all 4-arg multiplies

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Rename to fold_multiply2, and handle muls2_i32, mulu2_i64, and muls2_i64.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 44 +++-
>  1 file changed, 35 insertions(+), 9 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 38/51] tcg/optimize: Split out fold_masks

[Luis Fernando Fujita Pires]

From: Richard Henderson 

> @@ -1084,7 +1215,15 @@ static bool fold_extract(OptContext *ctx, TCGOp
> *op)
>  t = extract64(t, op->args[2], op->args[3]);
>  return tcg_opt_gen_movi(ctx, op, op->args[0], t);
>  }
> -return false;
> +
> +z_mask_old = arg_info(op->args[1])->z_mask;
> +z_mask = sextract64(z_mask_old, op->args[2], op->args[3]);

Should this be extract64 instead of sextract64?

Otherwise,
Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 34/51] tcg/optimize: Split out fold_to_not

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Split out the conditional conversion from a more complex logical operation to 
> a
> simple NOT.  Create a couple more helpers to make this easy for the outer-most
> logical operations.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 158 +++--
>  1 file changed, 86 insertions(+), 72 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v4 36/51] tcg/optimize: Split out fold_xi_to_x

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Pull the "op r, a, i => mov r, a" optimization into a function, and use them 
> in the
> outer-most logical operations.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 61 +-
>  1 file changed, 26 insertions(+), 35 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 07/22] host-utils: add 128-bit quotient support to divu128/divs128

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> > A new argument, prem, was added to divu128/divs128 to receive the
> > remainder, freeing up phigh to receive the high 64 bits of the
> > quotient.
> >
> > Signed-off-by: Luis Pires 
> 
> Why not return the remainder?  That would avoid the need for an extra
> argument, and the need for a conditional vs prem inside the division 
> functions.

No good reason, in fact. :)
I've changed that for the next version of the patch series. Thanks!

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 16/22] target/ppc: Move dtstdc[q]/dtstdg[q] to decodetree

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> On 9/10/21 4:26 AM, Luis Pires wrote:
> > +&Z22_bf_fra bf fra dm
> > +@Z22_bf_fra .. bf:3 .. fra:5 dm:6 . .   &Z22_bf_fra
> > +
> > +%z22_frap   17:4 !function=times_2
> > +@Z22_bf_frap.. bf:3 .. 0 dm:6 . .   &Z22_bf_fra
> fra=%z22_frap
> 
> How confusing.  There's a typo in the manual for these insns, with the minor
> opcode (XO) field at the wrong location.  It's correct in the summary of
> instruction formats at the beginning of the manual.

Right! This was also the case for dscli[q]/dscri[q]. I've reported this, and it 
should be fixed in the next release of the ISA.

> Functional change: you're no longer storing nip.  It does seem wrong, but that
> fix should be broken out to a separate patch.

Thanks, I've moved the nip update changes to a separate patch in the new series.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 15/22] target/ppc: Implement DCTFIXQQ

[Luis Fernando Fujita Pires]

From: Richard Henderson 

> > +static void set_dfp128_to_avr(ppc_avr_t *dst, ppc_vsr_t *src) {
> > +dst->VsrD(0) = src->VsrD(0);
> > +dst->VsrD(1) = src->VsrD(1);
> > +}
> 
> Given that these two are typedef of one another, I would think this is
> unnecessary and you should just write *dst = *src.

Done!

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 34/48] tcg/optimize: Split out fold_to_not

[Luis Fernando Fujita Pires]

From: Richard Henderson 

> >>   static bool fold_eqv(OptContext *ctx, TCGOp *op)  {
> >> -return fold_const2(ctx, op);
> >> +if (fold_const2(ctx, op) ||
> >> +fold_xi_to_not(ctx, op, 0)) {
> >
> > Should be fold_ix_to_not (not fold xi_to_not).
> 
> No, because for eqv we expect the second operand to be the constant -- eqv is
> commutative.

Ah, got it! The previous code was wrong, and I failed to notice that eqv 
would've had its arguments swapped to have the constant as second.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 37/48] tcg/optimize: Split out fold_ix_to_i

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Pull the "op r, 0, b => movi r, 0" optimization into a function, and use it in
> fold_shift.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 28 ++--
>  1 file changed, 10 insertions(+), 18 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 36/48] tcg/optimize: Split out fold_xi_to_x

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Pull the "op r, a, i => mov r, a" optimization into a function, and use them 
> int the
> outer-most logical operations.

Typo: "int" -> "in".

> -/* Simplify expression for "op r, a, const => mov r, a" cases */
> -switch (opc) {
> -CASE_OP_32_64_VEC(add):
> -CASE_OP_32_64_VEC(sub):
> -CASE_OP_32_64_VEC(or):
> -CASE_OP_32_64_VEC(xor):
> -CASE_OP_32_64_VEC(andc):
> -CASE_OP_32_64(shl):
> -CASE_OP_32_64(shr):
> -CASE_OP_32_64(sar):
> -CASE_OP_32_64(rotl):
> -CASE_OP_32_64(rotr):
> -if (!arg_is_const(op->args[1])
> -&& arg_is_const(op->args[2])
> -&& arg_info(op->args[2])->val == 0) {
> -tcg_opt_gen_mov(&ctx, op, op->args[0], op->args[1]);
> -continue;
> -}
> -break;
> -CASE_OP_32_64_VEC(and):
> -CASE_OP_32_64_VEC(orc):

You missed adding 'fold_xi_to_x(ctx, op, -1)' to fold_orc() in this commit.
It ended up being added in patch 42, but it should be here.

And should we use fold_xi_to_x() to optimize multiply and divide when i==1, too?

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 34/48] tcg/optimize: Split out fold_to_not

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Split out the conditional conversion from a more complex logical operation to 
> a
> simple NOT.  Create a couple more helpers to make this easy for the outer-most
> logical operations.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 154 +++--
>  1 file changed, 86 insertions(+), 68 deletions(-)

>  static bool fold_eqv(OptContext *ctx, TCGOp *op)  {
> -return fold_const2(ctx, op);
> +if (fold_const2(ctx, op) ||
> +fold_xi_to_not(ctx, op, 0)) {

Should be fold_ix_to_not (not fold xi_to_not).

>  static bool fold_orc(OptContext *ctx, TCGOp *op)  {
> -return fold_const2(ctx, op);
> +if (fold_const2(ctx, op) ||
> +fold_xi_to_not(ctx, op, 0)) {

Same here.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 35/48] tcg/optimize: Split out fold_sub_to_neg

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Even though there is only one user, place this more complex conversion into 
> its
> own helper.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 84 --
>  1 file changed, 47 insertions(+), 37 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 33/48] tcg/optimize: Add type to OptContext

[Luis Fernando Fujita Pires]

From: Richard Henderson 

> @@ -1392,18 +1408,18 @@ void tcg_optimize(TCGContext *s)
>  /* Proceed with possible constant folding. */
>  break;
>  }
> -if (opc == INDEX_op_sub_i32) {
> +switch (ctx.type) {
> +case TCG_TYPE_I32:
>  neg_op = INDEX_op_neg_i32;
>  have_neg = TCG_TARGET_HAS_neg_i32;
> -} else if (opc == INDEX_op_sub_i64) {
> +break;
> +case TCG_TYPE_I64:
>  neg_op = INDEX_op_neg_i64;
>  have_neg = TCG_TARGET_HAS_neg_i64;
> -} else if (TCG_TARGET_HAS_neg_vec) {
> -TCGType type = TCGOP_VECL(op) + TCG_TYPE_V64;
> -unsigned vece = TCGOP_VECE(op);
> +break;
> +default:
>  neg_op = INDEX_op_neg_vec;
> -have_neg = tcg_can_emit_vec_op(neg_op, type, vece) > 0;
> -} else {
> +have_neg = tcg_can_emit_vec_op(neg_op, ctx.type,
> + TCGOP_VECE(op)) > 0;

Should we replace the 'default' here with a case for TCG_TYPE_V{64,128,256} and 
add a new 'default' with g_assert_not_reached()?

> @@ -1457,15 +1473,19 @@ void tcg_optimize(TCGContext *s)
>  TCGOpcode not_op;
>  bool have_not;
> 
> -if (def->flags & TCG_OPF_VECTOR) {
> +switch (ctx.type) {
> +default:
>  not_op = INDEX_op_not_vec;
>  have_not = TCG_TARGET_HAS_not_vec;

And here too?

Either way,

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 32/48] tcg/optimize: Split out fold_xi_to_i

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Pull the "op r, a, 0 => movi r, 0" optimization into a function, and use it 
> in the
> outer opcode fold functions.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 32 +++-
>  1 file changed, 15 insertions(+), 17 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 31/48] tcg/optimize: Split out fold_xx_to_x

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Pull the "op r, a, a => mov r, a" optimization into a function, and use it in 
> the
> outer opcode fold functions.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 39 ---
>  1 file changed, 24 insertions(+), 15 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 30/48] tcg/optimize: Split out fold_xx_to_i

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Pull the "op r, a, a => movi r, 0" optimization into a function, and use it 
> in the
> outer opcode fold functions.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 41 -
>  1 file changed, 24 insertions(+), 17 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 29/48] tcg/optimize: Split out fold_mov

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> This is the final entry in the main switch that was in a different form.  
> After this,
> we have the option to convert the switch into a function dispatch table.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 27 ++-
>  1 file changed, 14 insertions(+), 13 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 26/48] tcg/optimize: Split out fold_count_zeros

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 32 ++--
>  1 file changed, 18 insertions(+), 14 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 28/48] tcg/optimize: Split out fold_dup, fold_dup2

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 53 +-
>  1 file changed, 31 insertions(+), 22 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 24/48] tcg/optimize: Split out fold_extract, fold_sextract

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 48 ++--
>  1 file changed, 30 insertions(+), 18 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 27/48] tcg/optimize: Split out fold_bswap

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 27 ---
>  1 file changed, 16 insertions(+), 11 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 22/48] tcg/optimize: Split out fold_movcond

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 56 --
>  1 file changed, 31 insertions(+), 25 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 25/48] tcg/optimize: Split out fold_deposit

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 25 +++--
>  1 file changed, 15 insertions(+), 10 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 23/48] tcg/optimize: Split out fold_extract2

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 39 ++-
>  1 file changed, 22 insertions(+), 17 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 21/48] tcg/optimize: Split out fold_addsub2_i32

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Add two additional helpers, fold_add2_i32 and fold_sub2_i32 which will not be
> simple wrappers forever.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 70 +++---
>  1 file changed, 44 insertions(+), 26 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 20/48] tcg/optimize: Split out fold_mulu2_i32

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 37 +
>  1 file changed, 21 insertions(+), 16 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 17/48] tcg/optimize: Split out fold_brcond2

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Reduce some code duplication by folding the NE and EQ cases.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 159 +
>  1 file changed, 81 insertions(+), 78 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 18/48] tcg/optimize: Split out fold_brcond

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 33 +++--
>  1 file changed, 19 insertions(+), 14 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 01/48] tcg/optimize: Rename "mask" to "z_mask"

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Prepare for tracking different masks by renaming this one.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 142 +
>  1 file changed, 72 insertions(+), 70 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 19/48] tcg/optimize: Split out fold_setcond

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 23 ++-
>  1 file changed, 14 insertions(+), 9 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 11/48] tcg/optimize: Return true from tcg_opt_gen_{mov, movi}

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> This will allow callers to tail call to these functions and return true 
> indicating
> processing complete.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 17/48] tcg/optimize: Split out fold_brcond2

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Reduce some code duplication by folding the NE and EQ cases.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 161 +
>  1 file changed, 83 insertions(+), 78 deletions(-)

> +case TCG_COND_NE:
> +inv = 1;
> +QEMU_FALLTHROUGH;
> +case TCG_COND_EQ:
> +/*
> + * Simplify EQ/NE comparisons where one of the pairs
> + * can be simplified.
> + */
> +i = do_constant_folding_cond(INDEX_op_brcond_i32, op->args[0],
> + op->args[2], cond);
> +switch (i ^ inv) {
> +case 0:
> +goto do_brcond_false;

I believe this should go to do_brcond_true when cond==TCG_COND_NE.

> +i = do_constant_folding_cond(INDEX_op_brcond_i32, op->args[1],
> + op->args[3], cond);
> +switch (i ^ inv) {
> +case 0:
> +goto do_brcond_false;

Same here.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 13/48] tcg/optimize: Use a boolean to avoid a mass of continues

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 15/48] tcg/optimize: Split out fold_const{1,2}

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Split out a whole bunch of placeholder functions, which are currently 
> identical.
> That won't last as more code gets moved.
> 
> Use CASE_32_64_VEC for some logical operators that previously missed the
> addition of vectors.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 254 +++--
>  1 file changed, 202 insertions(+), 52 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 12/48] tcg/optimize: Split out finish_folding

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Copy z_mask into OptContext, for writeback to the first output within the new
> function.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 49 +
>  1 file changed, 33 insertions(+), 16 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 16/48] tcg/optimize: Split out fold_setcond2

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Reduce some code duplication by folding the NE and EQ cases.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 145 -
>  1 file changed, 72 insertions(+), 73 deletions(-)

> -i = do_constant_folding_cond(INDEX_op_setcond_i32,
> - op->args[2], op->args[4],
> - TCG_COND_EQ);
> -if (i == 0) {
> -goto do_setcond_high;
^
> -} else if (i < 0) {
> -break;
> -}

I'll just note that the goto in the old code above is wrong - it should be a 
goto to do_setcond_const. But you fixed that in your new implementation.

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 06/48] tcg/optimize: Split out init_arguments

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> There was no real reason for calls to have separate code here.
> Unify init for calls vs non-calls using the call path, which handles
> TCG_CALL_DUMMY_ARG.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 25 +++--
>  1 file changed, 11 insertions(+), 14 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 07/48] tcg/optimize: Split out copy_propagate

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Continue splitting tcg_optimize.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 22 ++
>  1 file changed, 14 insertions(+), 8 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 09/48] tcg/optimize: Drop nb_oargs, nb_iargs locals

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Rather than try to keep these up-to-date across folding, re-read nb_oargs at 
> the
> end, after re-reading the opcode.
> 
> A couple of asserts need dropping, but that will take care of itself as we 
> split the
> function further.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 14 --
>  1 file changed, 4 insertions(+), 10 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 10/48] tcg/optimize: Change fail return for do_constant_folding_cond*

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Return -1 instead of 2 for failure.
> This us to use comparisons against 0 for all cases.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 145 +
>  1 file changed, 74 insertions(+), 71 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 14/48] tcg/optimize: Split out fold_mb, fold_qemu_{ld,st}

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> This puts the separate mb optimization into the same framework as the others.
> While fold_qemu_{ld,st} are currently identical, that won't last as more code
> gets moved.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 89 +-
>  1 file changed, 51 insertions(+), 38 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 05/48] tcg/optimize: Move prev_mb into OptContext

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> This will expose the variable to subroutines that will be broken out of
> tcg_optimize.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 04/48] tcg/optimize: Change tcg_opt_gen_{mov, movi} interface

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Adjust the interface to take the OptContext parameter instead of TCGContext or
> both.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 67 +-
>  1 file changed, 34 insertions(+), 33 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 03/48] tcg/optimize: Remove do_default label

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Break the final cleanup clause out of the main switch statement.  When fully
> folding an opcode to mov/movi, use "continue" to process the next opcode, else
> break to fall into the final cleanup.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 190 -
>  1 file changed, 94 insertions(+), 96 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 02/48] tcg/optimize: Split out OptContext

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Provide what will become a larger context for splitting the very large
> tcg_optimize function.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 77 ++
>  1 file changed, 40 insertions(+), 37 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 08/48] tcg/optimize: Split out fold_call

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Calls are special in that they have a variable number of arguments, and need 
> to
> be able to clobber globals.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 63 --
>  1 file changed, 41 insertions(+), 22 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 00/22] target/ppc: DFP instructions using decodetree

[Luis Fernando Fujita Pires]

Sorry, that was in the e-mail I replied to. I'm asking for reviews on patches 
5, 6, 7, 8, 12, 15 and 18:

5:  host-utils: move checks out of divu128/divs128
6:  host-utils: move udiv_qrnnd() to host-utils
7:  host-utils: add 128-bit quotient support to divu128/divs128
8:  host-utils: add unit tests for divu128/divs128
12:  target/ppc: Implement DCFFIXQQ
15:  target/ppc: Implement DCTFIXQQ
18:  target/ppc: Move dcmp{u,o}[q],dts{tex,tsf,tsfi}[q] to decodetree

> > > Patches 1-4 were already applied, and patches 5-8, 12, 15, 18 are
> > > missing reviews.

Thanks,

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>

> -Original Message-
> From: da...@gibson.dropbear.id.au 
> Sent: sexta-feira, 15 de outubro de 2021 00:15
> To: Luis Fernando Fujita Pires 
> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org; gr...@kaod.org;
> richard.hender...@linaro.org
> Subject: Re: [PATCH v3 00/22] target/ppc: DFP instructions using decodetree
> 
> On Thu, Oct 14, 2021 at 05:02:59PM +, Luis Fernando Fujita Pires wrote:
> > Ping?
> 
> I'm not sure who you're asking for what.  From my PoV, I'm waiting for 
> reviews.
> 
> >
> > > -Original Message-
> > > From: Luis Fernando Fujita Pires 
> > > Sent: segunda-feira, 20 de setembro de 2021 15:51
> > > To: Luis Fernando Fujita Pires ; qemu-
> > > de...@nongnu.org; qemu-...@nongnu.org
> > > Cc: da...@gibson.dropbear.id.au; gr...@kaod.org;
> > > richard.hender...@linaro.org
> > > Subject: RE: [PATCH v3 00/22] target/ppc: DFP instructions using
> > > decodetree
> > >
> > > Ping.
> > >
> > > Patches 1-4 were already applied, and patches 5-8, 12, 15, 18 are
> > > missing reviews.
> > >
> > > Thanks,
> > >
> >
> 
> --
> David Gibson  | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au| minimalist, thank you.  NOT _the_ 
> _other_
>   | _way_ _around_!
> http://www.ozlabs.org/~dgibson

RE: [PATCH v3 00/22] target/ppc: DFP instructions using decodetree

[Luis Fernando Fujita Pires]

Ping?

> -Original Message-
> From: Luis Fernando Fujita Pires 
> Sent: segunda-feira, 20 de setembro de 2021 15:51
> To: Luis Fernando Fujita Pires ; qemu-
> de...@nongnu.org; qemu-...@nongnu.org
> Cc: da...@gibson.dropbear.id.au; gr...@kaod.org;
> richard.hender...@linaro.org
> Subject: RE: [PATCH v3 00/22] target/ppc: DFP instructions using decodetree
> 
> Ping.
> 
> Patches 1-4 were already applied, and patches 5-8, 12, 15, 18 are missing
> reviews.
> 
> Thanks,
> 
> --
> Luis Pires
> Instituto de Pesquisas ELDORADO
> Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
> 
> From: Luis Pires 
> > This series moves all existing DFP instructions to decodetree and
> > implements the
> > 2 new instructions (dcffixqq and dctfixqq) from Power ISA 3.1.
> >
> > In order to implement dcffixqq, divu128/divs128 were modified to
> > support 128- bit quotients (previously, they were limited to 64-bit
> > quotients), along with adjustments being made to their existing callers.
> > libdecnumber was also expanded to allow creating decimal numbers from
> > 128- bit integers.
> >
> > Similarly, for dctfixqq, mulu128 (host-utils) and
> > decNumberIntegralToInt128
> > (libdecnumber) were introduced to support 128-bit integers.
> >
> > The remaining patches of this series move all of the already existing
> > DFP instructions to decodetree, and end up removing dfp-ops.c.inc,
> > which is no longer needed.
> >
> > NOTE 1: The previous, non-decodetree code, was updating ctx->nip for
> > all the DFP instructions. I've removed that, but it would be great if
> > someone could confirm that updating nip really wasn't necessary.
> >
> > NOTE 2: Some arithmetic function support for 128-bit integers was
> > added, for now, still using 64-bit pairs. In the near future, I think
> > we should modify all of them to use Int128 (and introduce UInt128).
> > But I'll send out an RFC to discuss how to do that in another patch series.
> >
> > NOTE 3: The helper names are in uppercase, to match the instruction
> > names and to simplify the macros that define trans* functions.
> > Previously, this wasn't the case, as we were using lowercase
> > instruction names in the pre-decodetree code. Another standalone patch
> > will be sent later on, changing to uppercase the other new
> > (decodetree) helpers whose names are directly related to instruction
> > names, eventually making PPC helper names consistent.
> >
> > Based-on: 20210823150235.35759-1-luis.pi...@eldorado.org.br
> > (target/ppc: fix setting of CR flags in bcdcfsq) This series assumes
> > bcdcfsq's fix is already in.
> >
> > Changes in v3:
> > - Split the uabs64 patch in 2
> > - Included patch to fix missing zero-extension in divs128
> > - Folded divisor == 0 into the dhi == 0 case in divu128
> > - Moved udiv_qrnnd from softfloat-macros.h to host-utils.h
> > - Used udiv_qrnnd in divu128
> > - Replaced int with bool in divs128
> > - Added unit test to check the divisor normalization in divu128
> > - Removed 'inline' from times_* functions in ppc/translate.c
> > - Used uadd64_overflow in mulu128
> > - Removed unnecessary 'else' from decNumberIntegralToInt128
> >
> > Changes in v2:
> > - Renamed abs64() to uabs64()
> >
> > Patches missing review:
> >   host-utils: fix missing zero-extension in divs128
> >   host-utils: move checks out of divu128/divs128
> >   host-utils: move udiv_qrnnd() to host-utils
> >   host-utils: add 128-bit quotient support to divu128/divs128
> >   host-utils: add unit tests for divu128/divs128
> >   target/ppc: Implement DCFFIXQQ
> >   target/ppc: Implement DCTFIXQQ
> >   target/ppc: Move dcmp{u,o}[q],dts{tex,tsf,tsfi}[q] to decodetree
> >
> > --
> > Luis Pires
> > Instituto de Pesquisas ELDORADO
> > Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
> >
> > Bruno Larsen (1):
> >   target/ppc: Move REQUIRE_ALTIVEC/VECTOR to translate.c
> >
> > Fernando Valle (1):
> >   target/ppc: Introduce REQUIRE_FPU
> >
> > Luis Pires (20):
> >   host-utils: Fix overflow detection in divu128()
> >   host-utils: fix missing zero-extension in divs128
> >   host-utils: introduce uabs64()
> >   i386/kvm: Replace abs64() with uabs64() from host-utils
> >   host-utils: move checks out of divu128/divs128
> >   host-utils: move udiv_qrnnd() to host-utils
> >   host-utils: add 128-bit quotient support to divu128/divs128
> &g

RE: [PATCH v3 00/22] target/ppc: DFP instructions using decodetree

[Luis Fernando Fujita Pires]

Ping.

Patches 1-4 were already applied, and patches 5-8, 12, 15, 18 are missing 
reviews.

Thanks,

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

From: Luis Pires 
> This series moves all existing DFP instructions to decodetree and implements 
> the
> 2 new instructions (dcffixqq and dctfixqq) from Power ISA 3.1.
> 
> In order to implement dcffixqq, divu128/divs128 were modified to support 128-
> bit quotients (previously, they were limited to 64-bit quotients), along with
> adjustments being made to their existing callers.
> libdecnumber was also expanded to allow creating decimal numbers from 128-
> bit integers.
> 
> Similarly, for dctfixqq, mulu128 (host-utils) and decNumberIntegralToInt128
> (libdecnumber) were introduced to support 128-bit integers.
> 
> The remaining patches of this series move all of the already existing DFP
> instructions to decodetree, and end up removing dfp-ops.c.inc, which is no
> longer needed.
> 
> NOTE 1: The previous, non-decodetree code, was updating ctx->nip for all the
> DFP instructions. I've removed that, but it would be great if someone could
> confirm that updating nip really wasn't necessary.
> 
> NOTE 2: Some arithmetic function support for 128-bit integers was added, for
> now, still using 64-bit pairs. In the near future, I think we should modify 
> all of
> them to use Int128 (and introduce UInt128). But I'll send out an RFC to 
> discuss
> how to do that in another patch series.
> 
> NOTE 3: The helper names are in uppercase, to match the instruction names and
> to simplify the macros that define trans* functions.
> Previously, this wasn't the case, as we were using lowercase instruction names
> in the pre-decodetree code. Another standalone patch will be sent later on,
> changing to uppercase the other new (decodetree) helpers whose names are
> directly related to instruction names, eventually making PPC helper names
> consistent.
> 
> Based-on: 20210823150235.35759-1-luis.pi...@eldorado.org.br
> (target/ppc: fix setting of CR flags in bcdcfsq) This series assumes 
> bcdcfsq's fix is
> already in.
> 
> Changes in v3:
> - Split the uabs64 patch in 2
> - Included patch to fix missing zero-extension in divs128
> - Folded divisor == 0 into the dhi == 0 case in divu128
> - Moved udiv_qrnnd from softfloat-macros.h to host-utils.h
> - Used udiv_qrnnd in divu128
> - Replaced int with bool in divs128
> - Added unit test to check the divisor normalization in divu128
> - Removed 'inline' from times_* functions in ppc/translate.c
> - Used uadd64_overflow in mulu128
> - Removed unnecessary 'else' from decNumberIntegralToInt128
> 
> Changes in v2:
> - Renamed abs64() to uabs64()
> 
> Patches missing review:
>   host-utils: fix missing zero-extension in divs128
>   host-utils: move checks out of divu128/divs128
>   host-utils: move udiv_qrnnd() to host-utils
>   host-utils: add 128-bit quotient support to divu128/divs128
>   host-utils: add unit tests for divu128/divs128
>   target/ppc: Implement DCFFIXQQ
>   target/ppc: Implement DCTFIXQQ
>   target/ppc: Move dcmp{u,o}[q],dts{tex,tsf,tsfi}[q] to decodetree
> 
> --
> Luis Pires
> Instituto de Pesquisas ELDORADO
> Aviso Legal - Disclaimer 
> 
> Bruno Larsen (1):
>   target/ppc: Move REQUIRE_ALTIVEC/VECTOR to translate.c
> 
> Fernando Valle (1):
>   target/ppc: Introduce REQUIRE_FPU
> 
> Luis Pires (20):
>   host-utils: Fix overflow detection in divu128()
>   host-utils: fix missing zero-extension in divs128
>   host-utils: introduce uabs64()
>   i386/kvm: Replace abs64() with uabs64() from host-utils
>   host-utils: move checks out of divu128/divs128
>   host-utils: move udiv_qrnnd() to host-utils
>   host-utils: add 128-bit quotient support to divu128/divs128
>   host-utils: add unit tests for divu128/divs128
>   libdecnumber: introduce decNumberFrom[U]Int128
>   target/ppc: Implement DCFFIXQQ
>   host-utils: Introduce mulu128
>   libdecnumber: Introduce decNumberIntegralToInt128
>   target/ppc: Implement DCTFIXQQ
>   target/ppc: Move dtstdc[q]/dtstdg[q] to decodetree
>   target/ppc: Move d{add,sub,mul,div,iex}[q] to decodetree
>   target/ppc: Move dcmp{u,o}[q],dts{tex,tsf,tsfi}[q] to decodetree
>   target/ppc: Move dquai[q], drint{x,n}[q] to decodetree
>   target/ppc: Move dqua[q], drrnd[q] to decodetree
>   target/ppc: Move dct{dp,qpq},dr{sp,dpq},dc{f,t}fix[q],dxex[q] to
> decodetree
>   target/ppc: Move ddedpd[q],denbcd[q],dscli[q],dscri[q] to decodetree
> 
>  hw/i386/kvm/i8254.c|   7 +-
>  include/fpu/softfloat-macros.h |  82 -
>  include/hw/clock.h |   7 +-
>  include/libdecnumber/decNumber.h   |   4 +
>  include/libdecnumber/decNumberLocal.h  |   2 +-
>  include/qemu/host-utils.h  | 163 --
>  libdecnumber/decContext.c  |   7 +-
>  libdecnumber/decNumber.c   | 131 
>  target/ppc/dfp_helper.c  

RE: [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer

[Luis Fernando Fujita Pires]

From: Cédric Le Goater 
> The current way the mask is built can overflow with a 64-bit decrementer.
> Use sextract64() to extract the signed values and remove the logic to handle
> negative values which has become useless.
> 
> Cc: Luis Fernando Fujita Pires 
> Fixes: a8dafa525181 ("target/ppc: Implement large decrementer support for
> TCG")
> Signed-off-by: Cédric Le Goater 

Reviewed-by: Luis Pires 

Thanks,

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>

RE: [PATCH v3] target/ppc: Fix 64-bit decrementer

[Luis Fernando Fujita Pires]

From: Cédric Le Goater 
> >> +target_long signed_value;
> >> +target_long signed_decr;
> >
> > Since these values will be the results of sextract64, it's probably better 
> > to
> define them as int64_t.
> 
> but then it breaks the code doing the logging on PPC32 targets :/

You mean here?
> >>   LOG_TB("%s: " TARGET_FMT_lx " => " TARGET_FMT_lx "\n", __func__,
> >> -decr, value);
> >> +decr, signed_value);
> >
> > While this reproduces the behavior we previously had, I think it would be 
> > more
> consistent if we logged what we had before the sign-extension ('value' instead
> of 'signed_value'). And 'decr' is okay, which is also not sign-extended.

It won't break if you log 'value' instead of 'signed_value', right?

> > The diff < 0 case:
> >  decr = -muldiv64(-diff, tb_env->decr_freq,
> > NANOSECONDS_PER_SECOND); should probably just be:
> >  decr = -1;
> > to comply with the minimum possible values specified by the ISA.
> > But, again, this doesn't impact your patch directly.
> 
> We can try to address that in a followup patch.

Agreed.

Thanks!

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3] target/ppc: Fix 64-bit decrementer

[Luis Fernando Fujita Pires]

Hi Cédric,

These changes look good and seem to replicate the exact behavior we had before, 
while fixing the 64-bit dec from MicroWatt.

A few notes, in case they help others, too:

According to the Power ISA:
  When not in Large Decrementer mode:
- the maximum positive value for the decrementer is the maximum possible 
signed 32-bit int (2^31 - 1)
- the minimum possible value for the decrementer is 0x
  When in Large Decrementer mode:
- the maximum positive value for the decrementer is 2^(nr_bits - 1) - 1
- the minimum possible value for the decrementer is 0x

And the decrementer is decremented until its value becomes 0, and then once 
more, to the minimum possible value (0x or 0x, 
depending on the Large Decrementer mode).

From what I understood from the code, the 'decr' value passed to 
__cpu_ppc_store_decr is the value of DECR before the change, and 'value' is the 
new one.

Now, back to the code... :)

> +target_long signed_value;
> +target_long signed_decr;

Since these values will be the results of sextract64, it's probably better to 
define them as int64_t.

>  LOG_TB("%s: " TARGET_FMT_lx " => " TARGET_FMT_lx "\n", __func__,
> -decr, value);
> +decr, signed_value);

While this reproduces the behavior we previously had, I think it would be more 
consistent if we logged what we had before the sign-extension ('value' instead 
of 'signed_value'). And 'decr' is okay, which is also not sign-extended.

> ||
> +((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && signed_value
> < 0
> +  && signed_decr >= 0)) {

The 'signed_decr >= 0' is ok. It catches the case where the previous value (now 
sign-extended as signed_decr) was >= 0 and we signed_value just got negative 
(which should be exactly 0xFF, after being sign-extended to 64 
bits).

One point that I found odd, but not directly related to your patch, is the 
implementation of _cpu_ppc_load_decr:

static inline int64_t _cpu_ppc_load_decr(CPUPPCState *env, uint64_t next)
{
ppc_tb_t *tb_env = env->tb_env;
int64_t decr, diff;

diff = next - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
if (diff >= 0) {
decr = muldiv64(diff, tb_env->decr_freq, NANOSECONDS_PER_SECOND);
} else if (tb_env->flags & PPC_TIMER_BOOKE) {
decr = 0;
}  else {
decr = -muldiv64(-diff, tb_env->decr_freq, NANOSECONDS_PER_SECOND);
}
LOG_TB("%s: %016" PRIx64 "\n", __func__, decr);

return decr;
}

The diff < 0 case:
decr = -muldiv64(-diff, tb_env->decr_freq, NANOSECONDS_PER_SECOND);
should probably just be:
decr = -1;
to comply with the minimum possible values specified by the ISA.
But, again, this doesn't impact your patch directly.

And also:
Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH] target/ppc: Fix 64-bit decrementer

[Luis Fernando Fujita Pires]

> > value = extract64(value, 0, nr_bits);
> > value = ((target_long)value << (64 - nr_bits)) >> (64 - nr_bits);
> 
> Oops, sorry. 64 might not be correct here. It would depend on the target being
> either 32 or 64.

In fact, sextract already does the sign extension, so this should be all that's 
needed, right?
value = sextract<32,64>(value, 0, nr_bits);

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH] target/ppc: Fix 64-bit decrementer

[Luis Fernando Fujita Pires]

> value = extract64(value, 0, nr_bits);
> value = ((target_long)value << (64 - nr_bits)) >> (64 - nr_bits);

Oops, sorry. 64 might not be correct here. It would depend on the target being 
either 32 or 64.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH] target/ppc: Fix 64-bit decrementer

[Luis Fernando Fujita Pires]

> >  bool negative;
> >
> >  /* Truncate value to decr_width and sign extend for simplicity */
> > -value &= ((1ULL << nr_bits) - 1);
> > +value &= MAKE_64BIT_MASK(0, nr_bits);
> 
> What about:
> 
>value = extract64(value, 0, nr_bits);
>if (value != sextract64(value, 0, nr_bits)) { ...

Or:
value = extract64(value, 0, nr_bits);
value = ((target_long)value << (64 - nr_bits)) >> (64 - nr_bits);

Also avoiding the problem with an invalid 64-bit shift with:
> >  value |= (0xULL << nr_bits);

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 04/19] host-utils: add 128-bit quotient support to divu128/divs128

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Hmm.  I'll note that we have a better divmod primitive in tree, but we aren't
> using it
> here: udiv_qrnnd in include/fpu/softfloat-macros.h.

Good to know! I'll change to a (much simpler) implementation using udiv_qrnnd.
Any pointers on what would be a good place to put udiv_qrnnd, so it can be used 
by softloat.c and host-utils.c? Would host-utils.h be ok?

> Given that none of the existing uses require the high part, should we be 
> creating
> a new interface?  The bug you highlight wrt truncation could be fixed 
> separately.

Although it does fix the bug, the motivation for the new interface is not 
really that bug. I wanted a 128-bit division that could return quotients larger 
than 64-bit, so I could use it in decNumberFrom[U]Int128, introduced in the 
next commit.

> > -void divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
> > +void divs128(uint64_t *plow, int64_t *phigh, int64_t *prem, int64_t
> > +divisor)
> >   {
> > -int sgn_dvdnd = *phigh < 0;
> > -int sgn_divsr = divisor < 0;
> > +int neg_quotient = 0, neg_remainder = 0;
> 
> You might as well use bool.

Sure, will do.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 03/19] host-utils: move checks out of divu128/divs128

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> > -static inline int divs128(int64_t *plow, int64_t *phigh, int64_t
> > divisor)
> > +static inline void divs128(int64_t *plow, int64_t *phigh, int64_t
> > +divisor)
> >   {
> > -if (divisor == 0) {
> > -return 1;
> > -} else {
> > -__int128_t dividend = ((__int128_t)*phigh << 64) | *plow;
> > -__int128_t result = dividend / divisor;
> > -*plow = result;
> > -*phigh = dividend % divisor;
> > -return result != *plow;
> > -}
> > +__int128_t dividend = ((__int128_t)*phigh << 64) | *plow;
>
> This is incorrect, before and after: *plow must be zero-extended here.

This will no longer be a problem after patch 4, when plow is changed to be 
uint64_t*, but I can fix it here, too.

> > @@ -97,13 +101,11 @@ int divu128(uint64_t *plow, uint64_t *phigh, uint64_t
> divisor)
> >   uint64_t carry = 0;
> >
> >   if (divisor == 0) {
> > -return 1;
> > +/* intentionally cause a division by 0 */
> > +*plow = 1 / divisor;
> >   } else if (dhi == 0) {
> >   *plow  = dlo / divisor;
> >   *phigh = dlo % divisor;
> 
> Let's not do two undefined things and leave *phigh uninitialized (e.g. riscv 
> host,
> where div-by-zero does not trap).  Just fold the div-by-zero case into the 
> dhi == 0
> case.

Will do.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 09/19] target/ppc: Implement DCFFIXQQ

[Luis Fernando Fujita Pires]

From: Matheus K. Ferst 
> > On 8/31/21 9:39 AM, Luis Pires wrote:
> >> +DEF_HELPER_3(DCFFIXQQ, void, env, fprp, avr)
> >
> > Shouldn't be upcase.  None of the others are.
> >
> 
> The reason for this change is on patch 13 and onwards. Matching the case of 
> the
> instruction name in the trans_ method and the helper makes it easier to
> create macros, e.g. TRANS_DFP_BF_A_DCM on patch 13. The idea was to
> change the helpers as we moved instructions to decodetree.
> 
> Alternatively, the macro could receive the instruction name and the
> gen_helper_, or we could drop this kind of macro usage in favor of
> something else. The former is a bit repetitive, while the latter would require
> more changes in the current code structure.

And our intention is also to send a standalone patch later on, changing to 
uppercase the other new (decodetree) helpers whose names are directly related 
to instruction names, making them consistent.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH 02/19] host-utils: move abs64() to host-utils

[Luis Fernando Fujita Pires]

> >> Oh, I wasn't referring to any specific users. What I meant is that,
> >> if we make abs64() generically available from host-utils, callers
> >> could expect it to behave the same way as abs() in stdlib, for
> >> example.
> >
> > That would be surprising, but do you think there are cases where that
> > would be a bad surprise?
> >
> > I don't think anybody who is aware of the abs(INT_MIN),
> > labs(LONG_MIN), and llabs(LLONG_MIN) edge cases actually _like_ that
> > behaviour.
> >
> > If you really want to avoid surprises, providing a saner function with
> > a different name seems better than trying to emulate the edge cases of
> > abs()/labs()/llabs().
> 
> Agreed. See do_strtosz() for example.

I'll make this change when I submit the next version of this patch series.

Thanks!

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH 02/19] host-utils: move abs64() to host-utils

[Luis Fernando Fujita Pires]

From: Eduardo Habkost 

> > Right, that's true of any standard implementation of abs().
> > I thought about making it return uint64_t, but that could make it
> > weird for other uses of abs64(), where callers wouldn't expect a type
> > change from int64_t to uint64_t. Maybe create a separate uabs64() that
> > returns uint64_t? Or is that even weirder? :)
> 
> Which users of abs64 would expect it to return int64_t?
> kvm_pit_update_clock_offset() doesn't seem to.

Oh, I wasn't referring to any specific users. What I meant is that, if we make 
abs64() generically available from host-utils, callers could expect it to 
behave the same way as abs() in stdlib, for example.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH 02/19] host-utils: move abs64() to host-utils

[Luis Fernando Fujita Pires]

From: David Gibson 
> Hrm..  I'm a bit concerned about mkaing this a more widespread function,
> because it has a nasty edge case... which is basically unavoidable in an 
> abs64()
> implementation.  Specifically:
> 
> abs64(0x800___0) == 0x800___ < 0
> 
> At least in the most likely 2's complement implementation.

Right, that's true of any standard implementation of abs().
I thought about making it return uint64_t, but that could make it weird for 
other uses of abs64(), where callers wouldn't expect a type change from int64_t 
to uint64_t. Maybe create a separate uabs64() that returns uint64_t? Or is that 
even weirder? :)

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH for-6.2 v2 2/2] target/ppc: fix vector registers access in gdbstub for little-endian

[Luis Fernando Fujita Pires]

> Maybe we should fix this by making the 'struct Int128' field order depend on
> HOST_WORDS_BIGENDIAN...

+1 for that.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH 01/26] accel/tcg: Introduce translator_use_goto_tb

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Add a generic version of the common use_goto_tb test.
> 
> Signed-off-by: Richard Henderson 
> ---
>  include/exec/translator.h | 10 ++
>  accel/tcg/translator.c| 11 +++
>  2 files changed, 21 insertions(+)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH 17/26] target/ppc: Use translator_use_goto_tb

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Cc: David Gibson 
> Signed-off-by: Richard Henderson 
> ---
>  target/ppc/translate.c | 10 +-
>  1 file changed, 1 insertion(+), 9 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH] tcg: Fix documentation for tcg_constant_* vs tcg_temp_free_*

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> At some point during the development of tcg_constant_*, I changed my mind
> about whether such temps should be able to be passed to tcg_temp_free_*.
> The final version committed allows this, but the commentary was not updated
> to match.
> 
> Fixes: c0522136adf
> Reported-by: Peter Maydell 
> Signed-off-by: Richard Henderson 
> ---
>  include/tcg/tcg.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH] accel/tcg: Use MiB in tcg_init_machine

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Suggested-by: Alex Bennée 
> Signed-off-by: Richard Henderson 
> ---
> 
> This sits in the middle of my "Clean up code_gen_buffer allocation"
> patch set.  Alex mentioned it during review, and I had already made the 
> change.
> 
> This is the only patch in the set that has not been posted and reviewed.  
> Rather
> than re-posting the entire set, I'm just sending this one and will queue the 
> whole
> thing to tcg-next.
> 
> 
> r~
> 
> ---
>  accel/tcg/tcg-all.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 26/28] tcg: Merge buffer protection and guard page protection

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Do not handle protections on a case-by-case basis in the various
> alloc_code_gen_buffer instances; do it within a single loop in 
> tcg_region_init.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/region.c | 45 +++--
>  1 file changed, 31 insertions(+), 14 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 25/28] tcg: Round the tb_size default from qemu_get_host_physmem

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> If qemu_get_host_physmem returns an odd number of pages, then physmem / 8
> will not be a multiple of the page size.
> 
> The following was observed on a gitlab runner:
> 
> ERROR qtest-arm/boot-serial-test - Bail out!
> ERROR:../util/osdep.c:80:qemu_mprotect__osdep: \
>   assertion failed: (!(size & ~qemu_real_host_page_mask))
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/region.c | 47 +--
>  1 file changed, 21 insertions(+), 26 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 28/28] tcg: Move tcg_init_ctx and tcg_ctx from accel/tcg/

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> These variables belong to the jit side, not the user side.
> 
> Since tcg_init_ctx is no longer used outside of tcg/, move the declaration to
> tcg/internal.h.
> 
> Suggested-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  include/tcg/tcg.h | 1 -
>  tcg/internal.h| 1 +
>  accel/tcg/translate-all.c | 3 ---
>  tcg/tcg.c | 3 +++
>  4 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 24/28] util/osdep: Add qemu_mprotect_rw

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> For --enable-tcg-interpreter on Windows, we will need this.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  include/qemu/osdep.h | 1 +
>  util/osdep.c | 9 +
>  2 files changed, 10 insertions(+)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 21/28] tcg: Allocate code_gen_buffer into struct tcg_region_state

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Do not mess around with setting values within tcg_init_ctx.
> Put the values into 'region' directly, which is where they will live for the 
> lifetime
> of the program.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/region.c | 64 ++--
>  1 file changed, 27 insertions(+), 37 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 27/28] tcg: When allocating for !splitwx, begin with PROT_NONE

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> There's a change in mprotect() behaviour [1] in the latest macOS on M1 and 
> it's
> not yet clear if it's going to be fixed by Apple.
> 
> In this case, instead of changing permissions of N guard pages, we change
> permissions of N rwx regions.  The same number of syscalls are required either
> way.
> 
> [1] https://gist.github.com/hikalium/75ae822466ee4da13cbbe486498a191f
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/region.c | 19 +--
>  1 file changed, 9 insertions(+), 10 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 18/28] tcg: Tidy tcg_n_regions

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Compute the value using straight division and bounds, rather than a loop.  
> Pass
> in tb_size rather than reading from tcg_init_ctx.code_gen_buffer_size,
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/region.c | 29 -
>  1 file changed, 12 insertions(+), 17 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 16/28] tcg: Replace region.end with region.total_size

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> A size is easier to work with than an end point, particularly during initial 
> buffer
> allocation.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/region.c | 29 +
>  1 file changed, 17 insertions(+), 12 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 23/28] tcg: Sink qemu_madvise call to common code

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Move the call out of the N versions of alloc_code_gen_buffer and into
> tcg_region_init.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/region.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 14/28] tcg: Introduce tcg_max_ctxs

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Finish the divorce of tcg/ from hw/, and do not take the max cpu value from
> MachineState; just remember what we were passed in tcg_init.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/internal.h |  3 ++-
>  tcg/region.c   |  6 +++---
>  tcg/tcg.c  | 23 ++-
>  3 files changed, 15 insertions(+), 17 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 13/28] accel/tcg: Pass down max_cpus to tcg_init

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Start removing the include of hw/boards.h from tcg/.
> Pass down the max_cpus value from tcg_init_machine, where we have the
> MachineState already.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  include/tcg/tcg.h   |  2 +-
>  tcg/internal.h  |  2 +-
>  accel/tcg/tcg-all.c | 10 +-
>  tcg/region.c| 32 +++-
>  tcg/tcg.c   | 10 --
>  5 files changed, 26 insertions(+), 30 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 20/28] tcg: Move in_code_gen_buffer and tests to region.c

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Shortly, the full code_gen_buffer will only be visible to region.c, so move
> in_code_gen_buffer out-of-line.
> 
> Move the debugging versions of tcg_splitwx_to_{rx,rw} to region.c as well, so
> that the compiler gets to see the implementation of in_code_gen_buffer.
> 
> This leaves exactly one use of in_code_gen_buffer outside of region.c, in
> cpu_restore_state.  Which, being on the exception path, is not performance
> critical.
> 
> Signed-off-by: Richard Henderson 
> ---
>  include/tcg/tcg.h | 11 +--
>  tcg/region.c  | 34 ++
>  tcg/tcg.c | 23 ---
>  3 files changed, 35 insertions(+), 33 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 22/28] tcg: Return the map protection from alloc_code_gen_buffer

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Change the interface from a boolean error indication to a negative error vs a
> non-negative protection.  For the moment this is only interface change, not
> making use of the new data.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/region.c | 63 +++-
>  1 file changed, 33 insertions(+), 30 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 11/28] tcg: Create tcg_init

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Perform both tcg_context_init and tcg_region_init.
> Do not leave this split to the caller.
> 
> Signed-off-by: Richard Henderson 
> ---
>  include/tcg/tcg.h | 3 +--
>  tcg/internal.h| 1 +
>  accel/tcg/translate-all.c | 3 +--
>  tcg/tcg.c | 9 -
>  4 files changed, 11 insertions(+), 5 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 09/28] accel/tcg: Move alloc_code_gen_buffer to tcg/region.c

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Buffer management is integral to tcg.  Do not leave the allocation to code
> outside of tcg/.  This is code movement, with further cleanups to follow.
> 
> Signed-off-by: Richard Henderson 
> ---
>  include/tcg/tcg.h |   2 +-
>  accel/tcg/translate-all.c | 414 +
>  tcg/region.c  | 421 +-
>  3 files changed, 418 insertions(+), 419 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 19/28] tcg: Tidy split_cross_256mb

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Return output buffer and size via output pointer arguments, rather than
> returning size via tcg_ctx->code_gen_buffer_size.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/region.c | 15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/tcg/region.c b/tcg/region.c index b44246e1aa..652f328d2c 100644
> --- a/tcg/region.c
> +++ b/tcg/region.c
> @@ -467,7 +467,8 @@ static inline bool cross_256mb(void *addr, size_t size)
>  /* We weren't able to allocate a buffer without crossing that boundary,
> so make do with the larger portion of the buffer that doesn't cross.
> Returns the new base of the buffer, and adjusts code_gen_buffer_size.  */ 
> -
> static inline void *split_cross_256mb(void *buf1, size_t size1)
> +static inline void split_cross_256mb(void **obuf, size_t *osize,
> + void *buf1, size_t size1)

Need to adjust the comment, now that we're no longer adjusting 
code_gen_buffer_size in here.


> @@ -583,8 +583,7 @@ static bool alloc_code_gen_buffer_anon(size_t size, int
> prot,
>  /* fallthru */
>  default:
>  /* Split the original buffer.  Free the smaller half.  */
> -buf2 = split_cross_256mb(buf, size);
> -size2 = tcg_ctx->code_gen_buffer_size;
> +split_cross_256mb(&buf2, &size2, buf, size);

This will be fixed by patch 21 (tcg: Allocate code_gen_buffer into struct 
tcg_region_state), but shouldn't we update tcg_ctx->code_gen_buffer_size here?

Other than that,

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 17/28] tcg: Rename region.start to region.after_prologue

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Give the field a name reflecting its actual meaning.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/region.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 10/28] accel/tcg: Rename tcg_init to tcg_init_machine

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> We shortly want to use tcg_init for something else.
> Since the hook is called init_machine, match that.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  accel/tcg/tcg-all.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v3 07/28] tcg: Split out region.c

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/internal.h  |  37 
>  tcg/region.c| 572 
>  tcg/tcg.c   | 547 +
>  tcg/meson.build |   1 +
>  4 files changed, 613 insertions(+), 544 deletions(-)  create mode 100644
> tcg/internal.h  create mode 100644 tcg/region.c

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer