Re: [Qemu-devel] [PATCH for-2.9 1/2] tcg/sparc: Zero extend data argument to store helpers

[Peter Maydell <peter.maydell@linaro.org>]

On 28 March 2017 at 16:52, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 03/27/2017 01:04 PM, Peter Maydell wrote:

> it seems to me easier to read masking op:
>
>     switch (op & MO_SIZE) {
>     case MO_8:
>         tcg_out_arithi(s, r, r, 0xff, ARITH_AND);
>         break;
>     case MO_16:
>         tcg_out_arithi(s, r, r, 16, SHIFT_SLL);
>         tcg_out_arithi(s, r, r, 16, SHIFT_SRL);
>         break;
>     case MO_32:
>         if (SPARC64) {
>             tcg_out_arith(s, r, r, 0, SHIFT_SRL);
>         }
>         break;
>     case MO_64:
>         break;
>     }

Yes, agreed.

>> +}
>> +
>>  static void build_trampolines(TCGContext *s)
>>  {
>>      static void * const qemu_ld_helpers[16] = {
>> @@ -910,6 +935,7 @@ static void build_trampolines(TCGContext *s)
>>          qemu_st_trampoline[i] = s->code_ptr;
>>
>>          if (SPARC64) {
>> +            emit_extend(s, TCG_REG_O2, i);
>
>
> shouldn't be inverting args?

>                emit_extend(s, i, TCG_REG_O2);

emit_extend() takes the TCG reg first and the 'op' second,
so this call is correct...

>>              ra = TCG_REG_O4;
>>          } else {
>>              ra = TCG_REG_O1;
>> @@ -925,6 +951,7 @@ static void build_trampolines(TCGContext *s)
>>                  tcg_out_arithi(s, ra, ra + 1, 32, SHIFT_SRLX);
>>                  ra += 2;
>>              } else {
>> +                emit_extend(s, i, ra);

...but this one isn't; I don't have 32-bit sparc host to test
with so I missed the error.

>>                  ra += 1;
>>              }
>>              /* Skip the oi argument.  */

thanks
-- PMM