qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] tcg/aarch64: Fix output of extract2 opcodes


From: Aleksandar Markovic
Subject: Re: [Qemu-devel] [PATCH] tcg/aarch64: Fix output of extract2 opcodes
Date: Wed, 10 Jul 2019 20:12:37 +0200

On Tue, Jul 9, 2019 at 8:56 PM Richard Henderson
<address@hidden> wrote:
>
> The aarch64 argument ordering for the operands is big-endian,
> whereas the tcg argument ordering is little-endian.  Use REG0
> so that we honor the rZ constraints.

Hello, Richard.

If endian and rZ constraints are unrelated problems, then I think the
commit message
should be:

"This patch fixes two problem:

- endianness: the aarch64 argument ordering for the operands is
big-endian, whereas the tcg argument ordering is little-endian.

- rZ constrains: REG0() macro should be applied to the affected
arguments."

One could argue that in this case the patch this should be actually two patches.
This is better because of bisectability. The number of line in the
patch doesn't matter.

If, on the other hand, endian and rZ constraints are related problems, then you
should explain how in the commit message.

In general, your commit messages appear too terse, and it is hard to establish
logical sense of the changes in question.

Would you be so kind to consider my opinion?

Regards,
Aleksandar

>
> Fixes: 464c2969d5d
> Reported-by: Peter Maydell <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  tcg/aarch64/tcg-target.inc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
> index b0f8106642..0713448bf5 100644
> --- a/tcg/aarch64/tcg-target.inc.c
> +++ b/tcg/aarch64/tcg-target.inc.c
> @@ -2226,7 +2226,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
>
>      case INDEX_op_extract2_i64:
>      case INDEX_op_extract2_i32:
> -        tcg_out_extr(s, ext, a0, a1, a2, args[3]);
> +        tcg_out_extr(s, ext, a0, REG0(2), REG0(1), args[3]);
>          break;
>
>      case INDEX_op_add2_i32:
> --
> 2.17.1
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]