> From: Anton Johansson <a...@rev.ng> > Sent: Tuesday, April 12, 2022 10:11 AM > To: Taylor Simpson <tsimp...@quicinc.com>; qemu-devel@nongnu.org > Cc: a...@rev.ng; Brian Cain <bc...@quicinc.com>; Michael Lambert > <mlamb...@quicinc.com>; bab...@rev.ng; ni...@rev.ng; > richard.hender...@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
