RE: [PATCH v8 10/12] target/hexagon: import parser for idef-parser

[Taylor Simpson]

> From: Anton Johansson <anjo@rev.ng> 
> Sent: Tuesday, April 12, 2022 10:11 AM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: ale@rev.ng; Brian Cain <bcain@quicinc.com>; Michael Lambert 
> <mlambert@quicinc.com>; babush@rev.ng; nizzo@rev.ng; 
> richard.henderson@linaro.org
> Subject: Re: [PATCH v8 10/12] target/hexagon: import parser for idef-parser
> 
> Very nice catch, this is the bug that plagued us a few weeks ago when 
> rebasing,
> it has since been fixed. Actually the `gen_set_overflow` fucntion has been 
> removed
> completely as it was only called when we handled `asl/asr_r_r_sat`.
> Current way we handle overflow:
>
> Overflow is now only set by saturates, this assumption holds for the 
> instructions
> we parse in phase 1. In the parser, all saturates call `gen_rvalue_sat` which 
> emits
> a call to `gen_set_usr_field_if` in `genptr.c` to set USR_OVF only if the 
> value is
> non-zero. It does this via OR'ing with the previous value.
>
> See 
> https://gitlab.com/AntonJohansson/qemu/-/blob/feature/idef-parser/target/hexagon/idef-parser/parser-helpers.c#L2109
>  for `gen_rvalue_sat`
> and 
> https://gitlab.com/AntonJohansson/qemu/-/blob/feature/idef-parser/target/hexagon/genptr.c#L669
>  for `gen_set_usr_field_if`

Your implementation of gen_set_usr_field_if is not correct.  Don't extract the 
bits from the value based on the reg_field_info.  The easiest thing to do is 
compare val with zero and jump over a call to gen_set_usr_field.  If you are 
determined to do an "or", you have to assert that ref_field_info[field].width 
== 1.  Then, you can extract 1 bit from val starting at offset zero and shift 
the result left by ref_field_info[field].offset and then "or" with USR.

Thanks,
Taylor