Re: [PATCH v2 3/4] target/m68k: Implement packed decimal real stores

[Richard Henderson <richard.henderson@linaro.org>]

On 8/16/24 16:37, Philippe Mathieu-Daudé wrote:
> Hi Richard,
> 
> On 12/8/24 02:44, Richard Henderson wrote:
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2488
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/m68k/helper.h     |   1 +
>>   target/m68k/fpu_helper.c | 112 +++++++++++++++++++++++++++++++++++++++
>>   target/m68k/translate.c  |  27 ++++++++--
>>   3 files changed, 137 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/m68k/helper.h b/target/m68k/helper.h
>> index 2c71361451..21af6adb39 100644
>> --- a/target/m68k/helper.h
>> +++ b/target/m68k/helper.h
>> @@ -127,6 +127,7 @@ DEF_HELPER_3(chk, void, env, s32, s32)
>>   DEF_HELPER_4(chk2, void, env, s32, s32, s32)
>>   DEF_HELPER_FLAGS_3(load_pdr_to_fx80, TCG_CALL_NO_RWG, void, env, fp, tl)
>> +DEF_HELPER_FLAGS_4(store_fx80_to_pdr, TCG_CALL_NO_RWG, void, env, tl, fp, int)
>>   #if !defined(CONFIG_USER_ONLY)
>>   DEF_HELPER_3(ptest, void, env, i32, i32)
>> diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
>> index 50b0e36cbe..0c9c3b7b64 100644
>> --- a/target/m68k/fpu_helper.c
>> +++ b/target/m68k/fpu_helper.c
>> @@ -750,6 +750,29 @@ void HELPER(fcosh)(CPUM68KState *env, FPReg *res, FPReg *val)
>>       res->d = floatx80_cosh(val->d, &env->fp_status);
>>   }
>> +/* 10**0 through 10**18 */
>> +static const int64_t i64_pow10[] = {
>> +    1ll,
>> +    10ll,
>> +    100ll,
>> +    1000ll,
>> +    10000ll,
>> +    100000ll,
>> +    1000000ll,
>> +    10000000ll,
>> +    100000000ll,
>> +    1000000000ll,
>> +    10000000000ll,
>> +    100000000000ll,
>> +    1000000000000ll,
>> +    10000000000000ll,
>> +    100000000000000ll,
>> +    1000000000000000ll,
>> +    10000000000000000ll,
>> +    100000000000000000ll,
>> +    1000000000000000000ll,
> 
> 10**18 is never used, is that expected?

A previous version did.

> Could we define KFACTOR_MAX = 17 and use it as
> array size?

Seems reasonable.

>> @@ -119,6 +119,8 @@ typedef struct DisasContext {
>>       int done_mac;
>>       int writeback_mask;
>>       TCGv writeback[8];
>> +    uint16_t insn;
>> +    uint16_t ext;
> 
> Maybe worth commenting here what this field is used for?

Ok.

r~