Re: [Qemu-devel] Sparc32: Mask writes to the WIM register

[Blue Swirl]

On 8/20/08, Luis Pureza <address@hidden> wrote:
> On Wed, Aug 20, 2008 at 4:30 PM, Blue Swirl <address@hidden> wrote:
>  > On 8/20/08, Luis Pureza <address@hidden> wrote:
>  >> OK, the following patch works for me:
>  >>
>  >>  ---
>  >>   target-sparc/translate.c |    7 +++++++
>  >>   1 files changed, 7 insertions(+), 0 deletions(-)
>  >>
>  >>  diff --git a/target-sparc/translate.c b/target-sparc/translate.c
>  >>  index 8286d36..02e497f 100644
>  >>
>  >> --- a/target-sparc/translate.c
>  >>  +++ b/target-sparc/translate.c
>  >>
>  >> @@ -73,6 +73,7 @@ typedef struct DisasContext {
>  >>      struct TranslationBlock *tb;
>  >>      uint32_t features;
>  >>      int extra_cpu_cycles;
>  >>  +    uint32_t wim_mask;
>  >>   } DisasContext;
>  >>
>  >>   // This function uses non-native bit order
>  >>  @@ -3485,6 +3486,7 @@ static void disas_sparc_insn(DisasContext * dc)
>  >>
>  >>                              }
>  >>   #else
>  >>                              tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_tmp0);
>  >>
>  >> +                            tcg_gen_andi_tl(cpu_tmp32, cpu_tmp32,
>  >>  dc->wim_mask);
>  >>
>  >>                              tcg_gen_st_i32(cpu_tmp32, cpu_env,
>  >>                                             offsetof(CPUSPARCState, wim));
>  >>
>  >>  #endif
>  >>  @@ -4848,6 +4850,11 @@ static inline void
>  >>  gen_intermediate_code_internal(TranslationBlock * tb,
>  >>   #ifdef TARGET_SPARC64
>  >>      dc->address_mask_32bit = env->pstate & PS_AM;
>  >>   #endif
>  >>  +    if (env->nwindows == 32) {
>  >>  +        dc->wim_mask = 0xFFFFFFFF;
>  >>  +    } else {
>  >>  +        dc->wim_mask = (1 << env->nwindows) - 1;
>  >>  +    }
>  >>      gen_opc_end = gen_opc_buf + OPC_MAX_SIZE;
>  >>
>  >>      cpu_tmp0 = tcg_temp_new(TCG_TYPE_TL);
>  >
>  > Thanks. But now every translation would be slowed down by a tiny
>  > amount because the wim mask is calculated, even if it will never be
>  > used. More optimally, the mask should be calculated as needed and only
>  > nwindows should be copied to dc.
>  >
>  > A more complex solution would be to avoid most of the copying
>  > entirely, so that env would have a pointer to sparc_def_t structure
>  > and dc could copy only this "def" pointer. Then instead of
>  > dc->features and dc->nwindows, dc->def->features and dc->def->nwindows
>  > would be used. This would be more useful in the future, if I add code
>  > to select the ASI helper at translation time.
>
>
> Complex please. One of those "integration problems" I had was
>  precisely related to different ASI mappings.
>
>  The following patch tries to do what you said. I guess more stuff
>  could be removed from CPUSPARCState now that it has a pointer to a
>  sparc_def_t. But I leave that to your judgment. The only thing I
>  removed was "features".
>
>  Note that the wrwim fix is also included.

Thanks, applied. The wrwim patch should have been separate for easier
bisection and debugging but I needed to adjust the other parts anyway.