Re: [Qemu-devel] [PATCH v14 9/9] target-avr: adding instruction decoder

[Peter Maydell]

On 29 July 2016 at 16:32, Michael Rolnik <address@hidden> wrote:
> Signed-off-by: Michael Rolnik <address@hidden>
> ---
>  target-avr/Makefile.objs              |    1 +
>  target-avr/cpugen/src/.cpugen.cpp.swp |  Bin 0 -> 24576 bytes

What is this binary ? It looks from the filename like some
editor backup file -- is it supposed to be here?

>  target-avr/decode.c                   |  693 +++++++++++++++
>  target-avr/helper.c                   |    2 +
>  target-avr/helper.h                   |    1 +
>  target-avr/translate-inst.c           | 1529 
> +++++++++++++++++----------------
>  target-avr/translate.c                |   29 +-
>  7 files changed, 1473 insertions(+), 782 deletions(-)
>  create mode 100644 target-avr/cpugen/src/.cpugen.cpp.swp
>  create mode 100644 target-avr/decode.c

> diff --git a/target-avr/helper.c b/target-avr/helper.c
> index 9635d38..bbb725e 100644
> --- a/target-avr/helper.c
> +++ b/target-avr/helper.c
> @@ -153,6 +153,7 @@ void tlb_fill(CPUState *cs, target_ulong vaddr, 
> MMUAccessType access_type,
>
>      tlb_set_page_with_attrs(cs, vaddr, paddr, attrs, prot, mmu_idx, 
> page_size);
>  }
> +
>  void helper_sleep(CPUAVRState *env)
>  {
>      CPUState *cs = CPU(avr_env_get_cpu(env));
> @@ -160,6 +161,7 @@ void helper_sleep(CPUAVRState *env)
>      cs->exception_index = EXCP_HLT;
>      cpu_loop_exit(cs);
>  }
> +
>  void helper_unsupported(CPUAVRState *env)
>  {
>      CPUState *cs = CPU(avr_env_get_cpu(env));
> diff --git a/target-avr/helper.h b/target-avr/helper.h
> index 6036315..a533d2a 100644
> --- a/target-avr/helper.h
> +++ b/target-avr/helper.h
> @@ -26,3 +26,4 @@ DEF_HELPER_3(outb, void, env, i32, i32)
>  DEF_HELPER_2(inb, tl, env, i32)
>  DEF_HELPER_3(fullwr, void, env, i32, i32)
>  DEF_HELPER_2(fullrd, tl, env, i32)
> +
> diff --git a/target-avr/translate-inst.c b/target-avr/translate-inst.c
> index d1dce42..3afb0d8 100644
> --- a/target-avr/translate-inst.c
> +++ b/target-avr/translate-inst.c
> @@ -28,14 +28,14 @@ static void gen_add_CHf(TCGv R, TCGv Rd, TCGv Rr)
>      TCGv t2 = tcg_temp_new_i32();
>      TCGv t3 = tcg_temp_new_i32();
>
> -    tcg_gen_and_tl(t1, Rd, Rr);         /*  t1 = Rd & Rr  */
> -    tcg_gen_andc_tl(t2, Rd, R);         /*  t2 = Rd & ~R  */
> -    tcg_gen_andc_tl(t3, Rr, R);         /*  t3 = Rr & ~R  */
> -    tcg_gen_or_tl(t1, t1, t2);          /*  t1 = t1 | t2 | t3  */
> +    tcg_gen_and_tl(t1, Rd, Rr); /* t1 = Rd & Rr */
> +    tcg_gen_andc_tl(t2, Rd, R); /* t2 = Rd & ~R */
> +    tcg_gen_andc_tl(t3, Rr, R); /* t3 = Rr & ~R */
> +    tcg_gen_or_tl(t1, t1, t2); /* t1 = t1 | t2 | t3 */

Please don't add code in one patch and then change its spacing in a
later patch. Pretty much all of this patch looks like changes that
should have been squashed into preceding patches.

thanks
-- PMM