On Thu, Jul 11, 2019 at 1:45 PM Richard Henderson <richard.hender...@linaro.org> wrote: > > 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? >
It does. That's super. Thank you very much! Yours, Aleksandar > > r~
