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/10/2013 05:59 AM, Alex Bennée wrote:
>>>> +    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?
>
> It's not obvious from this patch, since its more about the later gpr loads.
> I was thinking about size==4, signed, 32-bit.
>
> I.e. size=10, opc=11.  But reading closer that's unallocated_encoding.  So the
> answer for this patch is "no it's not worth it".
>
> But looking forward through the other patches I see
>
>> +        if (size == 3 && opc == 2) {
>> +            /* PRFM - prefetch */
>> +            return;
>> +        }
>> +        is_store = (opc == 0);
>> +        is_signed = opc & (1<<1);
>> +        is_extended = (size < 3) && (opc & 1);
>
> whereas from the ARM I see
>
> if opc<1> == '0' then
>   // store or zero-extending load
> else
>   if size == '11' then
>      memop = MemOp_PREFETCH;
>      if opc<0> == '1' then UnallocatedEncoding();
>   else
>      // sign-extending load
>      memop = MemOp_LOAD;
>      if size == '10' && opc<0> == '1' then UnallocatedEncoding();
>
> I.e. two undiagnosed unallocated_encoding.
>
> BTW, I find (1<<1) harder to scan here than 2, but I wasn't going to mention
> that while there was nothing else in the code to fix.

Yeah it's uglier than it could be, I'll see if I can make it cleaner and
catch those missing unallocs. I've just finished the lpd/stp today and it's
running validation overnight so I'll go through the rest of the patches
tomorrow.

Cheers,

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