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

[Richard Henderson]

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?


r~