Re: [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics fo

qemu-devel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics fo

From:	Tom Musta
Subject:	Re: [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics for non TARGET_PPC64
Date:	Fri, 12 Sep 2014 09:40:21 -0500
User-agent:	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0

On 9/12/2014 9:28 AM, Tom Musta wrote:
> On 9/11/2014 2:17 PM, Pierre Mallard wrote:
>> This patch remove limitation for fc[tf]id[*] on 32 bits targets and
>> add a new insn flag for signed integer 64 conversion PPC2_FP_CVT_S64
>> ---
>>  target-ppc/cpu.h            |    5 ++++-
>>  target-ppc/fpu_helper.c     |    6 ------
>>  target-ppc/helper.h         |    4 +---
>>  target-ppc/translate.c      |   18 +++++++-----------
>>  target-ppc/translate_init.c |    9 ++++++---
>>  5 files changed, 18 insertions(+), 24 deletions(-)
>>
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index b64c652..fa50c32 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -2008,13 +2008,16 @@ enum {
>>      PPC2_ALTIVEC_207   = 0x0000000000004000ULL,
>>      /* PowerISA 2.07 Book3s specification                                   
>>  */
>>      PPC2_ISA207S       = 0x0000000000008000ULL,
>> +    /* Double precision floating point conversion for signed integer 64     
>>  */
>> +    PPC2_FP_CVT_S64    = 0x0000000000010000ULL,
>>  
>>  #define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX 
>> | \
>>                          PPC2_ISA205 | PPC2_VSX207 | PPC2_PERM_ISA206 | \
>>                          PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
>>                          PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | \
>>                          PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207 | \
>> -                        PPC2_ALTIVEC_207 | PPC2_ISA207S | PPC2_DFP)
>> +                        PPC2_ALTIVEC_207 | PPC2_ISA207S | PPC2_DFP | \
>> +                        PPC2_FP_CVT_S64)
>>  };
>>  
>>  
>> /*****************************************************************************/
>> diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
>> index da93d12..7f74466 100644
>> --- a/target-ppc/fpu_helper.c
>> +++ b/target-ppc/fpu_helper.c
>> @@ -649,14 +649,10 @@ FPU_FCTI(fctiw, int32, 0x80000000U)
>>  FPU_FCTI(fctiwz, int32_round_to_zero, 0x80000000U)
>>  FPU_FCTI(fctiwu, uint32, 0x00000000U)
>>  FPU_FCTI(fctiwuz, uint32_round_to_zero, 0x00000000U)
>> -#if defined(TARGET_PPC64)
>>  FPU_FCTI(fctid, int64, 0x8000000000000000ULL)
>>  FPU_FCTI(fctidz, int64_round_to_zero, 0x8000000000000000ULL)
>>  FPU_FCTI(fctidu, uint64, 0x0000000000000000ULL)
>>  FPU_FCTI(fctiduz, uint64_round_to_zero, 0x0000000000000000ULL)
>> -#endif
>> -
>> -#if defined(TARGET_PPC64)
>>  
>>  #define FPU_FCFI(op, cvtr, is_single)                      \
>>  uint64_t helper_##op(CPUPPCState *env, uint64_t arg)       \
>> @@ -678,8 +674,6 @@ FPU_FCFI(fcfids, int64_to_float32, 1)
>>  FPU_FCFI(fcfidu, uint64_to_float64, 0)
>>  FPU_FCFI(fcfidus, uint64_to_float32, 1)
>>  
>> -#endif
>> -
>>  static inline uint64_t do_fri(CPUPPCState *env, uint64_t arg,
>>                                int rounding_mode)
>>  {
>> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
>> index 509eae5..52402ef 100644
>> --- a/target-ppc/helper.h
>> +++ b/target-ppc/helper.h
>> @@ -67,16 +67,14 @@ DEF_HELPER_2(fctiw, i64, env, i64)
>>  DEF_HELPER_2(fctiwu, i64, env, i64)
>>  DEF_HELPER_2(fctiwz, i64, env, i64)
>>  DEF_HELPER_2(fctiwuz, i64, env, i64)
>> -#if defined(TARGET_PPC64)
>>  DEF_HELPER_2(fcfid, i64, env, i64)
>>  DEF_HELPER_2(fcfidu, i64, env, i64)
>>  DEF_HELPER_2(fcfids, i64, env, i64)
>>  DEF_HELPER_2(fcfidus, i64, env, i64)
>>  DEF_HELPER_2(fctid, i64, env, i64)
>> -DEF_HELPER_2(fctidu, i64, env, i64)
>>  DEF_HELPER_2(fctidz, i64, env, i64)
>> +DEF_HELPER_2(fctidu, i64, env, i64)
> 
> NIT:  I would not have re-arranged fctidu/fctidz like this since it only 
> makes the patch larger without actually accomplishing anything (unless, of 
> course, the point of your patch is to do clean up).  You seem to have done 
> this in other places as well.
> 
>>  DEF_HELPER_2(fctiduz, i64, env, i64)
>> -#endif
>>  DEF_HELPER_2(frsp, i64, env, i64)
>>  DEF_HELPER_2(frin, i64, env, i64)
>>  DEF_HELPER_2(friz, i64, env, i64)
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index c07bb01..1fe82ce 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -2246,9 +2246,8 @@ GEN_FLOAT_B(ctiwz, 0x0F, 0x00, 0, PPC_FLOAT);
>>  GEN_FLOAT_B(ctiwuz, 0x0F, 0x04, 0, PPC2_FP_CVT_ISA206);
>>  /* frsp */
>>  GEN_FLOAT_B(rsp, 0x0C, 0x00, 1, PPC_FLOAT);
>> -#if defined(TARGET_PPC64)
>>  /* fcfid */
>> -GEN_FLOAT_B(cfid, 0x0E, 0x1A, 1, PPC_64B);
>> +GEN_FLOAT_B(cfid, 0x0E, 0x1A, 1, PPC2_FP_CVT_S64);
>>  /* fcfids */
>>  GEN_FLOAT_B(cfids, 0x0E, 0x1A, 0, PPC2_FP_CVT_ISA206);
>>  /* fcfidu */
>> @@ -2256,14 +2255,13 @@ GEN_FLOAT_B(cfidu, 0x0E, 0x1E, 0, 
>> PPC2_FP_CVT_ISA206);
>>  /* fcfidus */
>>  GEN_FLOAT_B(cfidus, 0x0E, 0x1E, 0, PPC2_FP_CVT_ISA206);
>>  /* fctid */
>> -GEN_FLOAT_B(ctid, 0x0E, 0x19, 0, PPC_64B);
>> +GEN_FLOAT_B(ctid, 0x0E, 0x19, 0, PPC2_FP_CVT_S64);
>> +/* fctidz */
>> +GEN_FLOAT_B(ctidz, 0x0F, 0x19, 0, PPC2_FP_CVT_S64);
>>  /* fctidu */
>>  GEN_FLOAT_B(ctidu, 0x0E, 0x1D, 0, PPC2_FP_CVT_ISA206);
>> -/* fctidz */
>> -GEN_FLOAT_B(ctidz, 0x0F, 0x19, 0, PPC_64B);
>>  /* fctidu */
>>  GEN_FLOAT_B(ctiduz, 0x0F, 0x1D, 0, PPC2_FP_CVT_ISA206);
>> -#endif
>>  
>>  /* frin */
>>  GEN_FLOAT_B(rin, 0x08, 0x0C, 1, PPC_FLOAT_EXT);
>> @@ -10050,16 +10048,14 @@ GEN_HANDLER_E(fctiwu, 0x3F, 0x0E, 0x04, 0, 
>> PPC_NONE, PPC2_FP_CVT_ISA206),
>>  GEN_FLOAT_B(ctiwz, 0x0F, 0x00, 0, PPC_FLOAT),
>>  GEN_HANDLER_E(fctiwuz, 0x3F, 0x0F, 0x04, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
>>  GEN_FLOAT_B(rsp, 0x0C, 0x00, 1, PPC_FLOAT),
>> -#if defined(TARGET_PPC64)
>> -GEN_FLOAT_B(cfid, 0x0E, 0x1A, 1, PPC_64B),
>> +GEN_HANDLER_E(fcfid, 0x3F, 0x0E, 0x1A, 0x001F0000, PPC_NONE, 
>> PPC2_FP_CVT_S64),
>>  GEN_HANDLER_E(fcfids, 0x3B, 0x0E, 0x1A, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
>>  GEN_HANDLER_E(fcfidu, 0x3F, 0x0E, 0x1E, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
>>  GEN_HANDLER_E(fcfidus, 0x3B, 0x0E, 0x1E, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
>> -GEN_FLOAT_B(ctid, 0x0E, 0x19, 0, PPC_64B),
>> +GEN_HANDLER_E(fctid, 0x3F, 0x0E, 0x19, 0x001F0000, PPC_NONE, 
>> PPC2_FP_CVT_S64),
>> +GEN_HANDLER_E(fctidz, 0x3F, 0x0F, 0x19, 0x001F0000, PPC_NONE, 
>> PPC2_FP_CVT_S64),
>>  GEN_HANDLER_E(fctidu, 0x3F, 0x0E, 0x1D, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
>> -GEN_FLOAT_B(ctidz, 0x0F, 0x19, 0, PPC_64B),
>>  GEN_HANDLER_E(fctiduz, 0x3F, 0x0F, 0x1D, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
>> -#endif
>>  GEN_FLOAT_B(rin, 0x08, 0x0C, 1, PPC_FLOAT_EXT),
>>  GEN_FLOAT_B(riz, 0x08, 0x0D, 1, PPC_FLOAT_EXT),
>>  GEN_FLOAT_B(rip, 0x08, 0x0E, 1, PPC_FLOAT_EXT),
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 48177ed..ac4d12a 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -5010,7 +5010,8 @@ POWERPC_FAMILY(e5500)(ObjectClass *oc, void *data)
>>                         PPC_FLOAT_STFIWX | PPC_WAIT |
>>                         PPC_MEM_TLBSYNC | PPC_TLBIVAX | PPC_MEM_SYNC |
>>                         PPC_64B | PPC_POPCNTB | PPC_POPCNTWD;
>> -    pcc->insns_flags2 = PPC2_BOOKE206 | PPC2_PRCNTL | PPC2_PERM_ISA206;
>> +    pcc->insns_flags2 = PPC2_BOOKE206 | PPC2_PRCNTL | PPC2_PERM_ISA206 | \
>> +                        PPC2_FP_CVT_S64;
>>      pcc->msr_mask = (1ull << MSR_CM) |
>>                      (1ull << MSR_GS) |
>>                      (1ull << MSR_UCLE) |
>> @@ -7906,6 +7907,7 @@ POWERPC_FAMILY(970)(ObjectClass *oc, void *data)
>>                         PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>>                         PPC_64B | PPC_ALTIVEC |
>>                         PPC_SEGMENT_64B | PPC_SLBI;
>> +    pcc->insns_flags2 = PPC2_FP_CVT_S64;
>>      pcc->msr_mask = (1ull << MSR_SF) |
>>                      (1ull << MSR_VR) |
>>                      (1ull << MSR_POW) |
>> @@ -7958,6 +7960,7 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
>>                         PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>>                         PPC_64B |
>>                         PPC_SEGMENT_64B | PPC_SLBI;
>> +    pcc->insns_flags2 = PPC2_FP_CVT_S64;
>>      pcc->msr_mask = (1ull << MSR_SF) |
>>                      (1ull << MSR_VR) |
>>                      (1ull << MSR_POW) |
>> @@ -8100,7 +8103,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>>      pcc->insns_flags2 = PPC2_VSX | PPC2_DFP | PPC2_DBRX | PPC2_ISA205 |
>>                          PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>>                          PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>> -                        PPC2_FP_TST_ISA206;
>> +                        PPC2_FP_TST_ISA206 | PPC2_FP_CVT_S64;
>>      pcc->msr_mask = (1ull << MSR_SF) |
>>                      (1ull << MSR_VR) |
>>                      (1ull << MSR_VSX) |
>> @@ -8178,7 +8181,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>>                          PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>>                          PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>>                          PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>> -                        PPC2_ISA205 | PPC2_ISA207S;
>> +                        PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64;
>>      pcc->msr_mask = (1ull << MSR_SF) |
>>                      (1ull << MSR_TM) |
>>                      (1ull << MSR_VR) |
>>
> 
> Other than the minor comments ....
> 
> Reviewed-by: Tom Musta <address@hidden>
> Tested-by: Tom Musta <address@hidden>
> 

One more issue .... the patch is missing the signoff (Alex will need one, I 
believe)

[Prev in Thread]

Current Thread

[Next in Thread]

[Qemu-devel] [PATCH 0/2] Enabling floating point instruction to 440x5 CPUs, Pierre Mallard, 2014/09/11
- [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics for non TARGET_PPC64, Pierre Mallard, 2014/09/11
  - Re: [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics for non TARGET_PPC64, Tom Musta, 2014/09/12
    - Re: [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics for non TARGET_PPC64, Tom Musta <=
    - Re: [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics for non TARGET_PPC64, Alexander Graf, 2014/09/12
- [Qemu-devel] [PATCH 2/2] target-ppc : Add new processor type 440x5wDFPU, Pierre Mallard, 2014/09/11
  - Re: [Qemu-devel] [PATCH 2/2] target-ppc : Add new processor type 440x5wDFPU, Tom Musta, 2014/09/12
- Re: [Qemu-devel] [PATCH 0/2] Enabling floating point instruction to 440x5 CPUs, Tom Musta, 2014/09/12
  - Re: [Qemu-devel] [PATCH 0/2] Enabling floating point instruction to 440x5 CPUs, Pierre Mallard, 2014/09/12

Prev by Date: Re: [Qemu-devel] [PATCH] cpu-exec: Don't mask out external interrupts when single-stepping an invalid instruction.
Next by Date: Re: [Qemu-devel] [PATCH 2/2] target-ppc : Add new processor type 440x5wDFPU
Previous by thread: Re: [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics for non TARGET_PPC64
Next by thread: Re: [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics for non TARGET_PPC64
Index(es):
- Date
- Thread