On 7/10/19 8:12 PM, Aleksandar Markovic wrote:
> On Tue, Jul 9, 2019 at 8:56 PM Richard Henderson
> <richard.hender...@linaro.org> 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~
