Re: [RFC] gdb/riscv: Add target description support

[John Baldwin <jhb at FreeBSD dot org>]

On 11/14/18 6:29 AM, Andrew Burgess wrote:
> * John Baldwin <jhb@FreeBSD.org> [2018-11-08 10:32:57 -0800]:
> 
>> On 11/8/18 8:07 AM, Andrew Burgess wrote:
>>> This commit adds target description support for riscv.
>>>
>>> I've used the split feature approach for specifying the architectural
>>> features, and the CSR feature is auto-generated from the riscv-opc.h
>>> header file.
>>
>> In general this looks fine to me (as far as I am familiar with the
>> target descriptions).  The only possible question/comment I have is if
>> you considered describing fields of specific registers such as the FP
>> status registers or MSTATUS, etc. as fields in the XML to replace the
>> current special cases in riscv_print_one_register_info().
> 
> I took a look at switching over to using flag fields in the xml
> description, but in the end I decided against this.
> 
> The main reason is that there appears to be a bug with registers
> described as a flag type, I was unable to assign to the register.
> What I saw was an invalid cast error.
> 
> I defined the type like this:
> 
>     <flags id="riscv_fflags" size="1">
>       <field name="NX" start="0" end="0"/>
>       <field name="UF" start="1" end="1"/>
>       <field name="OF" start="2" end="2"/>
>       <field name="DZ" start="3" end="3"/>
>       <field name="NV" start="4" end="4"/>
>     </flags>
> 
>     <reg name="fflags" bitsize="32" type="riscv_fflags"/>
> 
> But then, when I try to assign to the register I see this error:
> 
>     (gdb) set $fflags=0x3
>     Invalid cast

Weird, on x86 you can assign raw values to $eflags which is also described
as a flags field:

  <flags id="i386_eflags" size="4">
    <field name="CF" start="0" end="0"/>
    <field name="" start="1" end="1"/>
    <field name="PF" start="2" end="2"/>
    ...

(gdb) p $eflags
$1 = [ PF IF ]
(gdb) p/x $eflags
$2 = 0x206
(gdb) set $eflags = $eflags | 1
(gdb) p $eflags
$3 = [ CF PF IF ]

However, if it doesn't work I think it's fine to not use it.  I just wanted
to make sure you weren't duplicating work you wouldn't need.

>> Some related-ish questions (though not about this patch): I wonder if we
>> can do things with pseudo registers to automatically derive FFLAGS and
>> FRM if a target provides FCSR.
> 
> I like this idea a lot, and I definitely plan to implement this, just
> because I think it would be a neat feature.  However, my understanding
> is that FFLAGS and FRM _are_ real CSRs, at least in the sense that
> there's a real CSR offset from which we can read to extract the FFLAGS
> or FRM part of FCSR.  So, initially I'd prefer to merge this with
> these registers as real registers.
> 
> But I think it would be great if for targets that only announce FCSR,
> we could automatically emulate FRM and FCSR.

Ok.  I had assumed that we would have to use pseudo registers to do this,
but perhaps it can be done another way that allows a target to provide them
explicitly if it chose.

-- 
John Baldwin