Re: [PATCH v6 18/20] target/arm: Rebuild hflags at CPSR writes

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> Continue setting, but not relying upon, env->hflags.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/op_helper.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index ccc2cecb46..b529d6c1bf 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -224,6 +224,7 @@ uint32_t HELPER(usat16)(CPUARMState *env, uint32_t x, uint32_t shift)
>  void HELPER(setend)(CPUARMState *env)
>  {
>      env->uncached_cpsr ^= CPSR_E;
> +    arm_rebuild_hflags(env);
>  }
>
>  /* Function checks whether WFx (WFI/WFE) instructions are set up to be trapped.
> @@ -387,6 +388,8 @@ uint32_t HELPER(cpsr_read)(CPUARMState *env)
>  void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, uint32_t mask)
>  {
>      cpsr_write(env, val, mask, CPSRWriteByInstr);
> +    /* TODO: Not all cpsr bits are relevant to hflags.  */

Do you mean by this we could check which bits changed and avoid a
re-compute if we wanted to? Is it likely to be anything other than the
SS_ACTIVE bit?

> +    arm_rebuild_hflags(env);
>  }
>
>  /* Write the CPSR for a 32-bit exception return */

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


--
Alex Bennée