Re: [Qemu-devel] [PATCHv1 11/14] target/s390x: convert to DisasContextBase

2018-03-02 Thread Cornelia Huck
On Thu,  1 Mar 2018 17:53:55 -0500
"Emilio G. Cota"  wrote:

> Notes:
> 
> - Did not convert {num,max}_insns and is_jmp, since the corresponding
>   code will go away in the next patch.
> 
> - Avoided a checkpatch error in use_exit_tb.
> 
> - As suggested by David, (1) Drop ctx.pc and use
>   ctx.base.pc_next instead, and (2) Rename ctx.next_pc to
>   ctx.pc_tmp and add a comment about it.
> 
> Suggested-by: David Hildenbrand 
> Cc: David Hildenbrand 
> Cc: Cornelia Huck 
> Cc: Alexander Graf 
> Cc: qemu-s3...@nongnu.org
> Signed-off-by: Emilio G. Cota 
> ---
>  target/s390x/translate.c | 146 
> ---
>  1 file changed, 75 insertions(+), 71 deletions(-)

Acked-by: Cornelia Huck 



Re: [Qemu-devel] [PATCHv1 11/14] target/s390x: convert to DisasContextBase

2018-03-02 Thread David Hildenbrand
On 01.03.2018 23:53, Emilio G. Cota wrote:
> Notes:
> 
> - Did not convert {num,max}_insns and is_jmp, since the corresponding
>   code will go away in the next patch.
> 
> - Avoided a checkpatch error in use_exit_tb.
> 
> - As suggested by David, (1) Drop ctx.pc and use
>   ctx.base.pc_next instead, and (2) Rename ctx.next_pc to
>   ctx.pc_tmp and add a comment about it.
> 
> Suggested-by: David Hildenbrand 
> Cc: David Hildenbrand 
> Cc: Cornelia Huck 
> Cc: Alexander Graf 
> Cc: qemu-s3...@nongnu.org
> Signed-off-by: Emilio G. Cota 
> ---
>  target/s390x/translate.c | 146 
> ---
>  1 file changed, 75 insertions(+), 71 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 5346791..c83a57f 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -52,14 +52,18 @@ typedef struct DisasInsn DisasInsn;
>  typedef struct DisasFields DisasFields;
>  
>  struct DisasContext {
> -struct TranslationBlock *tb;
> +DisasContextBase base;
>  const DisasInsn *insn;
>  DisasFields *fields;
>  uint64_t ex_value;
> -uint64_t pc, next_pc;
> +/*
> + * During translate_one(), pc_tmp is used to determine the instruction
> + * to be executed after base.pc_next - e.g. next sequential instruction
> + * or a branch target.
> + */
> +uint64_t pc_tmp;
>  uint32_t ilen;
>  enum cc_op cc_op;
> -bool singlestep_enabled;
>  };
>  
>  /* Information carried about a condition to be evaluated.  */
> @@ -81,8 +85,8 @@ static uint64_t inline_branch_miss[CC_OP_MAX];
>  
>  static uint64_t pc_to_link_info(DisasContext *s, uint64_t pc)
>  {
> -if (!(s->tb->flags & FLAG_MASK_64)) {
> -if (s->tb->flags & FLAG_MASK_32) {
> +if (!(s->base.tb->flags & FLAG_MASK_64)) {
> +if (s->base.tb->flags & FLAG_MASK_32) {
>  return pc | 0x8000;
>  }
>  }
> @@ -188,16 +192,16 @@ static void return_low128(TCGv_i64 dest)
>  static void update_psw_addr(DisasContext *s)
>  {
>  /* psw.addr */
> -tcg_gen_movi_i64(psw_addr, s->pc);
> +tcg_gen_movi_i64(psw_addr, s->base.pc_next);
>  }
>  
>  static void per_branch(DisasContext *s, bool to_next)
>  {
>  #ifndef CONFIG_USER_ONLY
> -tcg_gen_movi_i64(gbea, s->pc);
> +tcg_gen_movi_i64(gbea, s->base.pc_next);
>  
> -if (s->tb->flags & FLAG_MASK_PER) {
> -TCGv_i64 next_pc = to_next ? tcg_const_i64(s->next_pc) : psw_addr;
> +if (s->base.tb->flags & FLAG_MASK_PER) {
> +TCGv_i64 next_pc = to_next ? tcg_const_i64(s->pc_tmp) : psw_addr;
>  gen_helper_per_branch(cpu_env, gbea, next_pc);
>  if (to_next) {
>  tcg_temp_free_i64(next_pc);
> @@ -210,16 +214,16 @@ static void per_branch_cond(DisasContext *s, TCGCond 
> cond,
>  TCGv_i64 arg1, TCGv_i64 arg2)
>  {
>  #ifndef CONFIG_USER_ONLY
> -if (s->tb->flags & FLAG_MASK_PER) {
> +if (s->base.tb->flags & FLAG_MASK_PER) {
>  TCGLabel *lab = gen_new_label();
>  tcg_gen_brcond_i64(tcg_invert_cond(cond), arg1, arg2, lab);
>  
> -tcg_gen_movi_i64(gbea, s->pc);
> +tcg_gen_movi_i64(gbea, s->base.pc_next);
>  gen_helper_per_branch(cpu_env, gbea, psw_addr);
>  
>  gen_set_label(lab);
>  } else {
> -TCGv_i64 pc = tcg_const_i64(s->pc);
> +TCGv_i64 pc = tcg_const_i64(s->base.pc_next);
>  tcg_gen_movcond_i64(cond, gbea, arg1, arg2, gbea, pc);
>  tcg_temp_free_i64(pc);
>  }
> @@ -228,7 +232,7 @@ static void per_branch_cond(DisasContext *s, TCGCond cond,
>  
>  static void per_breaking_event(DisasContext *s)
>  {
> -tcg_gen_movi_i64(gbea, s->pc);
> +tcg_gen_movi_i64(gbea, s->base.pc_next);
>  }
>  
>  static void update_cc_op(DisasContext *s)
> @@ -250,7 +254,7 @@ static inline uint64_t ld_code4(CPUS390XState *env, 
> uint64_t pc)
>  
>  static int get_mem_index(DisasContext *s)
>  {
> -switch (s->tb->flags & FLAG_MASK_ASC) {
> +switch (s->base.tb->flags & FLAG_MASK_ASC) {
>  case PSW_ASC_PRIMARY >> FLAG_MASK_PSW_SHIFT:
>  return 0;
>  case PSW_ASC_SECONDARY >> FLAG_MASK_PSW_SHIFT:
> @@ -315,7 +319,7 @@ static inline void gen_trap(DisasContext *s)
>  #ifndef CONFIG_USER_ONLY
>  static void check_privileged(DisasContext *s)
>  {
> -if (s->tb->flags & FLAG_MASK_PSTATE) {
> +if (s->base.tb->flags & FLAG_MASK_PSTATE) {
>  gen_program_exception(s, PGM_PRIVILEGED);
>  }
>  }
> @@ -324,7 +328,7 @@ static void check_privileged(DisasContext *s)
>  static TCGv_i64 get_address(DisasContext *s, int x2, int b2, int d2)
>  {
>  TCGv_i64 tmp = tcg_temp_new_i64();
> -bool need_31 = !(s->tb->flags & FLAG_MASK_64);
> +bool need_31 = !(s->base.tb->flags & FLAG_MASK_64);
>  
>  /* Note that d2 is limited to 20 bits, signed.  If we crop negative
> displacements early we create larger immedate addends.  */
> @@ -537,9 +541,9 @@ static void gen_op_