Re: [Qemu-devel] [PATCH v4 6/8] target/mips: Amend CP0 WatchHi register implementation

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

On 07/06/2018 04:48 AM, Aleksandar Markovic wrote:
> +++ b/target/mips/op_helper.c
> @@ -893,7 +893,12 @@ target_ulong helper_mfc0_watchlo(CPUMIPSState *env, uint32_t sel)
>  
>  target_ulong helper_mfc0_watchhi(CPUMIPSState *env, uint32_t sel)
>  {
> -    return env->CP0_WatchHi[sel];
> +    return (int32_t) env->CP0_WatchHi[sel];
> +}
> +
> +target_ulong helper_mfhc0_watchhi(CPUMIPSState *env, uint32_t sel)
> +{
> +    return env->CP0_WatchHi[sel] >> 32;
>  }

Did you in fact want the high-part sign-extended as well?
It might be more obvious to write

    return sextract64(env->CP0_WatchHi[sel], 32, 32);

in that case.

> +void helper_mthc0_watchhi(CPUMIPSState *env, target_ulong arg1, uint32_t sel)
> +{
> +    env->CP0_WatchHi[sel] = ((uint64_t) (arg1) << 32) |
> +                            (env->CP0_WatchHi[sel] & 0x00000000ffffffffULL);

Cleaner as

    env->CP0_WatchHi[sel] = deposit64(env->CP0_WatchHi[sel], 32, 32, arg1);


For future cleanup, there is nothing in this (or several other) that requires
writing helper code.  This could just as easily be expanded inline with one
single load or store operation.


r~