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

[Hans-Peter Nilsson]

> Date: Tue, 13 Sep 2016 00:49:51 +0200
> From: "Edgar E. Iglesias" <address@hidden>
> 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.

I've forgotten most details, and have to refer to Rabin for
useful answers.

However, from the few glimpses I remember, basically I had to
key on "singlestep" and force a re-translation to get usable
dumps that didn't skip dump of state from all instructions
inside a translated hunk on subsequent executions.  There were
some hacks^Wpatches left-out (for being too invasive and
hackish, IIRC) to that effect, not sure how they affected this
particular code here.

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

No: I'd rather just not have prefix instructions dumped
separately whenever possible as dumps get really big really
quick, but maybe both this patch and a prefix indicator then, to
reduce confusion.  I guess a better deal would be a method that
enables consistent dumps; page boundaries and icount shouldn't
affect dumps.  Maybe qemu has improved and there's a generic
method to get useful register dumps *from every instruction at
every execution* these days.

> BTW, are you guys doing post-processing on this

Certainly.  The output was massaged to have the same format as
dumps from another simulator, then fed into a script for use
together with the executable to get readable output (somewhat at
call-graph-level).

> 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
> > 
> 

brgds, H-P