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.pie...@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
>
