Re: [Qemu-devel] [PATCH 2/9] target-arm: A64: add support for ldp (load pair)

["Alex Bennée" <alex.bennee@linaro.org>]

rth@twiddle.net writes:

> On 12/09/2013 10:12 AM, Peter Maydell wrote:
>> +static void do_gpr_ld(DisasContext *s, TCGv_i64 dest, TCGv_i64 tcg_addr,
>> +                      int size, bool is_signed, bool extend)
>> +{
<snip>
>
> Likewise, combine all of this with tcg_gen_qemu_ld_i64.

Cool, that cleans up a bunch of logic.

>
>> +    if (extend && is_signed) {
>> +        g_assert(size < 3);
>> +        tcg_gen_ext32u_i64(dest, dest);
>> +    }
>
> Is it worth noticing in size==2 && !extend that is_signed can be forced false
> to avoid the extra extension.

Sorry I don't quite follow, the extension only occurs if it's an explict
extension into a 64 bit register. Or are you talking about avoiding
using the extension logic in the generic tcg_gen_qemu_ld_i64 code?

>
>> +static void handle_ldp(DisasContext *s, uint32_t insn)
>> +{
<snip>
>> +}
>
> There's so much overlap with STP, I think these ought not be separate
> functions.  Just merge the middle bit where the actual load/store happens.

Yeah that's a function of the way the patches were ported across
piecemeal as required for risu testing. I'll merge the two together.

There are other potential common parts that could be cleaned up later.
However the original ldst patches did get a little gnarly thanks to the
structure of the original decoder and I wanted to avoid things like imm
fields being optionally chopped apart depending on the decode.

-- 
Alex Bennée
QEMU/KVM Hacker for Linaro