Re: [Qemu-devel] [PATCH 00/34] tcg, target/ppc vector improvements

[Mark Cave-Ayland]

On 18/12/2018 15:07, Richard Henderson wrote:

>> This one appears to be a sign extension issue - if I make use of the same 
>> technique
>> used by the previous helper then this problem goes away. Below is my 
>> experimental
>> diff to be squashed into "target/ppc: convert vspltis[bhw] to use vector 
>> operations":
>>
>> diff --git a/target/ppc/translate/vmx-impl.inc.c 
>> b/target/ppc/translate/vmx-impl.inc.c
>> index be638cdb1a..6cd25c8dc6 100644
>> --- a/target/ppc/translate/vmx-impl.inc.c
>> +++ b/target/ppc/translate/vmx-impl.inc.c
>> @@ -723,12 +723,12 @@ GEN_VXRFORM_DUAL(vcmpgtfp, PPC_ALTIVEC, PPC_NONE, \
>>  #define GEN_VXFORM_DUPI(name, tcg_op, opc2, opc3)                       \
>>  static void glue(gen_, name)(DisasContext *ctx)                         \
>>      {                                                                   \
>> -        int simm;                                                       \
>> +        int8_t simm; 
> 
> This shouldn't matter.
>                                                    \
>>          if (unlikely(!ctx->altivec_enabled)) {                          \
>>              gen_exception(ctx, POWERPC_EXCP_VPU);                       \
>>              return;                                                     \
>>          }                                                               \
>> -        simm = SIMM5(ctx->opcode);                                      \
>> +        simm = (int8_t)(SIMM5(ctx->opcode) << 3) >> 3;                  \
> 
> This suggests that SIMM5 should be using sextract32.

There's certainly an obvious typo here, but on its own it doesn't fix the issue:

diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index b77d564a65..08eee1cd84 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -124,7 +124,7 @@ EXTRACT_SHELPER(SIMM, 0, 16);
 /* 16 bits unsigned immediate value */
 EXTRACT_HELPER(UIMM, 0, 16);
 /* 5 bits signed immediate value */
-EXTRACT_HELPER(SIMM5, 16, 5);
+EXTRACT_SHELPER(SIMM5, 16, 5);
 /* 5 bits signed immediate value */
 EXTRACT_HELPER(UIMM5, 16, 5);
 /* 4 bits unsigned immediate value */


ATB,

Mark.