All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tom Musta <tommusta@gmail.com>
To: Alexander Graf <agraf@suse.de>,
	Pierre Mallard <mallard.pierre@gmail.com>,
	qemu-devel@nongnu.org, qemu-ppc@nongnu.org
Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] target-ppc : Add PPC_FLOAT_64 type to fctid, fctidz and fcfid and remove their TARGET_PPC64 restriction
Date: Wed, 10 Sep 2014 11:44:33 -0500	[thread overview]
Message-ID: <54107FF1.6000004@gmail.com> (raw)
In-Reply-To: <5410178F.6020703@suse.de>

On 9/10/2014 4:19 AM, Alexander Graf wrote:
> 
> 
> On 10.09.14 07:03, Pierre Mallard wrote:
>> Apply the new PPC_FLOAT_64 flag to fctid[z] and fcfid. 
>> May also be applyed to fctidu[z] and fcfid[su][z], but since they are not 
>> mentionned in xilinx documentation it might not be needed yet.
>>
>> Signed-off-by: Pierre Mallard <mallard.pierre@gmail.com>
>> ---
>>  target-ppc/fpu_helper.c |    7 +++----
>>  target-ppc/helper.h     |    6 ++++--
>>  target-ppc/translate.c  |   20 ++++++++++++--------
>>  3 files changed, 19 insertions(+), 14 deletions(-)
>>
>> diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
>> index da93d12..4e0e9e2 100644
>> --- a/target-ppc/fpu_helper.c
>> +++ b/target-ppc/fpu_helper.c
>> @@ -649,15 +649,13 @@ 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)
>> +#if defined(TARGET_PPC64)
>>  FPU_FCTI(fctidu, uint64, 0x0000000000000000ULL)
>>  FPU_FCTI(fctiduz, uint64_round_to_zero, 0x0000000000000000ULL)
>>  #endif

So fctid[z] are being handled by this new flag but fctidu[z] are not?  Uggh.
>>  
>> -#if defined(TARGET_PPC64)
>> -
>>  #define FPU_FCFI(op, cvtr, is_single)                      \
>>  uint64_t helper_##op(CPUPPCState *env, uint64_t arg)       \
>>  {                                                          \
>> @@ -674,10 +672,11 @@ uint64_t helper_##op(CPUPPCState *env, uint64_t arg)       \
>>  }
>>  
>>  FPU_FCFI(fcfid, int64_to_float64, 0)
>> +
>> +#if defined(TARGET_PPC64)
>>  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,
>> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
>> index 509eae5..e51aa69 100644
>> --- a/target-ppc/helper.h
>> +++ b/target-ppc/helper.h
>> @@ -67,14 +67,16 @@ 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)
>> +#if defined(TARGET_PPC64)
>>  DEF_HELPER_2(fcfidu, i64, env, i64)
>>  DEF_HELPER_2(fcfids, i64, env, i64)
>>  DEF_HELPER_2(fcfidus, i64, env, i64)
>> +#endif
>>  DEF_HELPER_2(fctid, i64, env, i64)
>> -DEF_HELPER_2(fctidu, i64, env, i64)
>>  DEF_HELPER_2(fctidz, i64, env, i64)
>> +#if defined(TARGET_PPC64)
>> +DEF_HELPER_2(fctidu, i64, env, i64)
>>  DEF_HELPER_2(fctiduz, i64, env, i64)
>>  #endif
>>  DEF_HELPER_2(frsp, i64, env, i64)
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index c07bb01..6af25fe 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -2246,21 +2246,23 @@ 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, PPC_FLOAT_64|PPC_64B);

Given the limited scope of the flag (see my previous comment), I dont think PPC_FLOAT_64 is a very good name for this.  The semantic of this flag derived from your implementation is really limited to fcfid/fctid, fctidz
>> +#if defined(TARGET_PPC64)
>>  /* fcfids */
>>  GEN_FLOAT_B(cfids, 0x0E, 0x1A, 0, PPC2_FP_CVT_ISA206);
>>  /* fcfidu */
>>  GEN_FLOAT_B(cfidu, 0x0E, 0x1E, 0, PPC2_FP_CVT_ISA206);
>>  /* fcfidus */
>>  GEN_FLOAT_B(cfidus, 0x0E, 0x1E, 0, PPC2_FP_CVT_ISA206);
>> +#endif
>>  /* fctid */
>> -GEN_FLOAT_B(ctid, 0x0E, 0x19, 0, PPC_64B);
>> +GEN_FLOAT_B(ctid, 0x0E, 0x19, 0, PPC_FLOAT_64|PPC_64B);
>> +/* fctidz */
>> +GEN_FLOAT_B(ctidz, 0x0F, 0x19, 0, PPC_FLOAT_64|PPC_64B);
>> +#if defined(TARGET_PPC64)
>>  /* 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
>> @@ -10050,14 +10052,16 @@ 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),
>> +GEN_FLOAT_B(cfid, 0x0E, 0x1A, 1, PPC_FLOAT_64|PPC_64B),
>>  #if defined(TARGET_PPC64)
>> -GEN_FLOAT_B(cfid, 0x0E, 0x1A, 1, PPC_64B),
>>  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),
>> +#endif
>> +GEN_FLOAT_B(ctid, 0x0E, 0x19, 0, PPC_FLOAT_64|PPC_64B),
>> +GEN_FLOAT_B(ctidz, 0x0F, 0x19, 0, PPC_FLOAT_64|PPC_64B),
> 
> I think we're better off with only a single bit. Just make all 64bit
> CPUs that have an FPU also set PPC_FLOAT_64 and only check for that.
> 
> 
> Alex
> 

  reply	other threads:[~2014-09-10 16:44 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-10  5:03 [Qemu-devel] [PATCH 0/3] Enabling floating point instruction to 440x5 CPUs Pierre Mallard
2014-09-10  5:03 ` [Qemu-devel] [PATCH 1/3] target-ppc : Add floating point ability to 440x5 PPC CPU Pierre Mallard
2014-09-10  9:13   ` [Qemu-devel] [Qemu-ppc] " Alexander Graf
2014-09-10  5:03 ` [Qemu-devel] [PATCH 2/3] target-ppc : Add PPC_FLOAT_64 flag to instructions type Pierre Mallard
2014-09-10  9:18   ` [Qemu-devel] [Qemu-ppc] " Alexander Graf
2014-09-10 16:23     ` Tom Musta
2014-09-10  5:03 ` [Qemu-devel] [PATCH 3/3] target-ppc : Add PPC_FLOAT_64 type to fctid, fctidz and fcfid and remove their TARGET_PPC64 restriction Pierre Mallard
2014-09-10  9:19   ` [Qemu-devel] [Qemu-ppc] " Alexander Graf
2014-09-10 16:44     ` Tom Musta [this message]
2014-09-10  9:20 ` [Qemu-devel] [Qemu-ppc] [PATCH 0/3] Enabling floating point instruction to 440x5 CPUs Alexander Graf
2014-09-10 17:15   ` Tom Musta
2014-09-10 18:02     ` Pierre Mallard
2014-09-10 22:43     ` Pierre Mallard
2014-09-11 12:30       ` Tom Musta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54107FF1.6000004@gmail.com \
    --to=tommusta@gmail.com \
    --cc=agraf@suse.de \
    --cc=mallard.pierre@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.