Re: [PATCH 3/4] Enable tracing of pseudo-registers on ARM

[Marcin KoÅcielnicki <koriakin at 0x04 dot net>]

On 12/02/16 16:13, Yao Qi wrote:
> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
> > +/* Map the pseudo register number REG to the proper register number.  */
> > +
> > +static int
> > +arm_pseudo_register_to_register (struct gdbarch *gdbarch, int reg)
> > +{
> > +  int rawnum = 0;
> > +  int num_regs = gdbarch_num_regs (gdbarch);
> > +
> > +  /* Single precision pseudo registers. s0-s31.  */
> > +  if (reg >= num_regs && reg < num_regs + 32)
> > +    {
> > +      rawnum = (reg - num_regs) / 2 + 26;
>
> We should get double register number via user_reg_map_name_to_regnum,
>
>        xsnprintf (name_buf, sizeof (name_buf), "d%d", (reg - num_regs) / 2);
>        double_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
> 						   strlen (name_buf));
> .
> > +    }
> > +  /* Quadruple precision pseudo regisers. q0-q15.  */
> > +  else if (reg >= num_regs + 32 && reg < num_regs + 32 + 16)
> > +    {
> > +      rawnum = (reg - num_regs - 32) * 2 + 26;
>
> Likewise,
>
>        xsnprintf (name_buf, sizeof (name_buf), "d%d", (reg - num_regs) * 2);
>        double_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
> 						   strlen (name_buf));
>
> additionally, we need to check gdbarch_tdep (gdbarch)->have_neon_pseudos,
>
> > +    }
> > +  /* Error bad register number.  */
> > +  else
> > +    return -1;
> > +
> > +  return rawnum;
> > +}
>
> We also need a test case, and you can extend gdb.trace/tfile-avx.exp.
> Probably, it can be renamed to gdb.trace/tracefile-pseudo-reg.exp, and
> put x86 and arm tests in it.

I'd like to point out that this testcase is near-useless for testing 
ax_pseudo_register_collect or pseudo_register_to_register at the moment 
- while gdb computes a mask of what registers need to be collected, 
gdbserver just ignores it and collects all registers if any register at 
all is to be collected.  In turn, gdb allows you to display the state of 
all registers, even ones not included in the mask.  In fact, the 
tfile-avx.exp test passes just fine if you change it to collect any 
unrelated register.  My commit with ax_pseudo_register_collect only made 
it work because gdb needs to have that function return success, the 
actual returned mask could just as well be wrong...

The other hook, pseudo_register_push_stack, is much easier to test - 
it's invoked when a pseudo is used in an actual agent expression, eg. if 
you use it in a tracepoint condition, or as part of the address of 
collected memory area.  However, it cannot be used on SIMD registers (at 
least on x86, I don't know much about arm), as they don't fit in an 
ULONGEST...

Matter of fact, our support for >64-bit quantities in tracepoints is 
very poor at the moment - they can only be collected wholesale when 
they're single registers or contig memory areas.  Use in expressions is 
out (if you happen to have something interesting in low 32 bits of a 
vector reg, sorry).  Likewise, stiching them together with DW_op_piece 
(or whatever that was called) also fails (see 
https://sourceware.org/bugzilla/show_bug.cgi?id=17015).  We could 
definitely use some improvement there...