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

[Aleksandar Markovic]

On Thu, Jul 11, 2019 at 1:45 PM Richard Henderson
<address@hidden> wrote:
>
> On 7/10/19 8:12 PM, Aleksandar Markovic wrote:
> > 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."
>
> That's fair.
>
> > 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.
>
> Well, nothing between the faulty commit (Fixes: 464c2969d5d) and the second of
> the two prospective patches is really bisectable.  For the given test case, 
> all
> points in between will fail at runtime, even if each point compiles.
>
> Therefore I don't see the point in separating the two changes.
>
> > Would you be so kind to consider my opinion?
>
> Yes.  Thanks for the expanded opinion.
>
> I plan to change the commit message to:
>
> ---
> tcg/aarch64: Fix output of extract2 opcodes
>
> This patch fixes two problems:
> (1) The inputs to the EXTR insn were reversed,
> (2) The input constraints use rZ, which means that we need to use
>     the REG0 macro in order to supply XZR for a constant 0 input.
>
> r-b, s-o-b, etc
> ---
>
> Does that seem sufficient to you?
>

It does. That's super. Thank you very much!

Yours,
Aleksandar

>
> r~