Re: [PATCH] gdb/riscv: Split ISA and ABI features

[John Baldwin <jhb at FreeBSD dot org>]

On 12/15/18 4:30 PM, Andrew Burgess wrote:
> When running on an RV64 target which only has 32-bit float then the
> integer field could be 64-bits, while if the float field is 32-bits
> the overall size of the structure can be 128-bits (with 32-bits of
> padding).  In this case the assertion would fail, however, the code
> isn't incorrect, so its safe to just remove the assertion.
> 
> This was tested by running on an RV64IMFDC target using a compiler
> configured for RV64IMFC, and comparing the results with those obtained
> when using a compiler configured for RV64IMFDC.  The only regressions
> I see (now) are in gdb.base/store.exp and are related too different
> code generation choices GCC makes between the two targets.
> 
> gdb/ChangeLog:
> 
> 	* arch/riscv.h (struct riscv_gdbarch_features) <hw_float_abi>:
> 	Delete.
> 	<operator==>: Update with for removed field.
> 	<hash>: Likewise.
> 	* riscv-tdep.h (struct gdbarch_tdep) <features>: Renamed to...
> 	<isa_features>: ...this.
> 	<abi_features>: New field.
> 	(riscv_isa_flen): Update comment.
> 	(riscv_abi_flen): New declaration.x

Extra 'x'?

> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 88b79af866f..aeb9f2e5627 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -361,7 +361,18 @@ static unsigned int riscv_debug_gdbarch = 0;
>  int
>  riscv_isa_xlen (struct gdbarch *gdbarch)
>  {
> -  return gdbarch_tdep (gdbarch)->features.xlen;
> +  return gdbarch_tdep (gdbarch)->isa_features.xlen;
> +}
> +
> +/* Return the x-register length that is being used for the abi.  This
> +   exists only so we have similar interfaces for flen and xlen, however,
> +   this always calls RISCV_ISA_XLEN, as running a binary compiled for RV32
> +   on an RV64 target doesn't make sense.  */

I think in theory you could do this just fine?  It would be akin to running
an i386 binary on x86_64.  I thought the XLEN field in one of the status
registers was allowed to be writable so that you could switch modes at
least for U?  That said, I'm not sure if that will actually get used.  I'm
not sure if Linux is planning on supporting RV32.  FreeBSD will likely only
ever support RV64.  I think having riscv_abi_xlen is fine in its current
form.

Note that I think the current patch will always report the floating point
registers as the target width?  I think for other architectures in GDB we
store the target registers as "raw" registers, but would present the
abi registers as "cooked".  I think this is how we treat an i386 binary on
x86_64 for example where 'info registers' shows 32-bit registers instead of
64-bit.  MIPS does some similar things to handle 32-bit MIPS binaries on
64-bit targets.  Perhaps this is less important for FP registers, but I'm
curious if GDB will DTRT if you have a 'float' variable that DWARF indicates
is stored in $f0 if $f0 is actually a 64-bit double register?

-- 
John Baldwin