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:	Alexander Graf
Subject:	Re: [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics for non TARGET_PPC64
Date:	Fri, 12 Sep 2014 16:56:15 +0200
User-agent:	Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:31.0) Gecko/20100101 Thunderbird/31.1.1


On 12.09.14 16:40, Tom Musta wrote:
> 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)

Yes, I can't apply patches without SoB line. Pierre, please check out

  http://wiki.qemu.org/Contribute/SubmitAPatch

with some hints on how to do great patch submissions :).


Alex

[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, 2014/09/12
    - Re: [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics for non TARGET_PPC64, Alexander Graf <=
- [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: [Qemu-devel] [PATCH] configure: Build GDB XML for 32 bit ARM CPUs into qemu aarch64 binaries
Next by Date: Re: [Qemu-devel] ui causes latest master build failure on rhel6
Previous by thread: Re: [Qemu-devel] [PATCH 1/2] target-ppc : Allow fc[tf]id[*] mnemonics for non TARGET_PPC64
Next by thread: [Qemu-devel] [PATCH 2/2] target-ppc : Add new processor type 440x5wDFPU
Index(es):
- Date
- Thread