RE: [PATCH] Hexagon: lldb read/write predicate registers p0/p1/p2/p3

[Ted Woodward]

> -----Original Message-----
> From: ltaylorsimpson@gmail.com <ltaylorsimpson@gmail.com>
> Sent: Wednesday, June 12, 2024 9:49 PM
> To: Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>
> Cc: qemu-devel@nongnu.org; Brian Cain <bcain@quicinc.com>; Ted
> Woodward <tedwood@quicinc.com>; alex.bennee@linaro.org; Sid Manning
> <sidneym@quicinc.com>; Marco Liebel (QUIC) <quic_mliebel@quicinc.com>;
> richard.henderson@linaro.org; philmd@linaro.org; ale@rev.ng; anjo@rev.ng
> Subject: RE: [PATCH] Hexagon: lldb read/write predicate registers
> p0/p1/p2/p3
> 
> > -----Original Message-----
> > From: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> > Sent: Wednesday, June 12, 2024 12:30 PM
> > To: ltaylorsimpson@gmail.com
> > Cc: qemu-devel@nongnu.org; bcain@quicinc.com; tedwood@quicinc.com;
> > alex.bennee@linaro.org; quic_mathbern@quicinc.com;
> > sidneym@quicinc.com; quic_mliebel@quicinc.com;
> > richard.henderson@linaro.org; philmd@linaro.org; ale@rev.ng;
> > anjo@rev.ng
> > Subject: Re: [PATCH] Hexagon: lldb read/write predicate registers
> > p0/p1/p2/p3
> >
> > On Wed, 12 Jun 2024 10:42:39 -0600 Taylor Simpson
> > <ltaylorsimpson@gmail.com> wrote:
> > >
> > > diff --git a/target/hexagon/gdbstub.c b/target/hexagon/gdbstub.c
> > > index
> > > 502c6987f0..e67e627fc9 100644
> > > --- a/target/hexagon/gdbstub.c
> > > +++ b/target/hexagon/gdbstub.c
> > > @@ -56,6 +64,15 @@ int hexagon_gdb_write_register(CPUState *cs,
> > uint8_t *mem_buf, int n)
> > >          return sizeof(target_ulong);
> > >      }
> > >
> > > +    n -= TOTAL_PER_THREAD_REGS;
> > > +
> > > +    if (n < NUM_PREGS) {
> > > +        env->pred[n] = ldtul_p(mem_buf);
> > > +        return sizeof(uint8_t);
> >
> > I wonder, shouldn't this be sizeof(target_ulong) since we wrote a
> > target_ulong?
> 
> Good question.
> 
> From the architecture point of view, predicates are 8 bits (Section 2.2.5 of 
> the
> v73 Hexagon PRM).  However, we model them in QEMU as target_ulong
> because TCG variables must be either 32 bits or 64 bits.  There isn't an 
> option
> for 8 bits.  Whenever we write to a predicate, do "and" with 0xff first to 
> ensure
> there are only 8 bits written (see gen_log_pred_write in
> target/hexagon/genptr.c).
> 
> I did some more digging and here is what I found:
> - Since we have bitsize="8" in hexagon-core.xml, lldb will reject any attempt 
> to
> write something larger.
>   (lldb) reg write p1 0x1ff
>   error: Failed to write register 'p1' with value '0x1ff': value 0x1ff is too 
> large to
> fit in a 1 byte unsigned integer value
> - For the lldb "reg write" command, the return value from
> hexagon_gdb_write_register isn't used.
> - The only place the return value is used is in handle_write_all_regs.  This
> function is called in response to a "G" packet from the debugger.  I don't 
> know
> if/when lldb uses this packet, but it seems like it would count on it being 8 
> bits
> since that's what is in hexagon-core.xml.
> 
> Ted <tedwood@quicinc.com>, when would lldb generate a "G" packet, and
> what assumptions will it make about the size of predicate registers?

When you use the expression parser to call a function, lldb will save the 
current state,
set up the function call, set a breakpoint on a return (by changing the lr 
register and setting
a breakpoint on the new address), set the PC to the function address, and 
resume. After the
breakpoint is hit, lldb will restore the saved state.

Since QEMU doesn't support the lldb RSP extension 
QSaveRegisterState/QRestoreRegisterState,
lldb will use G/g packets to save and restore the register state.

lldb doesn't interpret the values from the G/g packets. It just saves and 
restores them, so I don't
think the new predicate definitions will matter for that. You can test this out 
by changing the
predicate registers, then calling a function with the expression parser. Not a 
varargs function,
since the IR interpreter doesn't handle those.

Ted

> Thanks,
> Taylor
> 
>