Re: [Qemu-devel] [PATCH 7/9] target-cris: ignore prefix insns in singlestep

[Edgar E. Iglesias]

On Mon, Sep 05, 2016 at 01:54:10PM +0200, Rabin Vincent wrote:
> From: Hans-Peter Nilsson <address@hidden>
> 
> Don't count prefix instructions as separate when singlestepping.
> 
> For example, for this following instruction
> 
>  1ad8:       a011 e00b               move.d r0,[r1-96]
> 
> before this patch, we get two register dumps:
> 
>  PC=1ad8 CCS=0 btaken=1 btarget=1ac6
>  cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
>  $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
>  $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
>  $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
>  $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4
> 
>  PC=1ada CCS=800 btaken=1 btarget=1ac6
>  cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
>  $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
>  $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
>  $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
>  $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4
> 
> With the patch, we get only one:
> 
>  PC=1ad8 CCS=0 btaken=1 btarget=1ac6
>  cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
>  $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
>  $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
>  $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
>  $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4

Hi,

A concern I have is that we can't guard against all split prefix
sequences (e.g at page boundaries or with icount). So it may be more
confusing to see the prefix insns sometimes than every time.

Perhaps we should more clearly be showing prefix state in the logs?

BTW, are you guys doing post-processing on this or is it only
for human inspection?

Cheers,
Edgar


> 
> Signed-off-by: Hans-Peter Nilsson <address@hidden>
> Signed-off-by: Rabin Vincent <address@hidden>
> ---
>  target-cris/translate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target-cris/translate.c b/target-cris/translate.c
> index a4512b5..c9b1e65 100644
> --- a/target-cris/translate.c
> +++ b/target-cris/translate.c
> @@ -3262,7 +3262,8 @@ void gen_intermediate_code(CPUCRISState *env, struct 
> TranslationBlock *tb)
>          }
>      } while (!dc->is_jmp && !dc->cpustate_changed
>              && !tcg_op_buf_full()
> -            && !singlestep
> +            /* We don't count prefix insns as separate wrt. singlestep.  */
> +            && (!singlestep || (dc->tb_flags & PFIX_FLAG))
>              && (dc->pc < next_page_start)
>              && num_insns < max_insns);
>  
> -- 
> 2.1.4
>