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

[Anton Johansson via <qemu-devel@nongnu.org>]

[-- Attachment #1: Type: text/plain, Size: 2896 bytes --]

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 here 
<https://gitlab.com/AntonJohansson/qemu/-/blob/feature/idef-parser/target/hexagon/idef-parser/parser-helpers.c#L2109> 
for `gen_rvalue_sat`
and here 
<https://gitlab.com/AntonJohansson/qemu/-/blob/feature/idef-parser/target/hexagon/genptr.c#L669> 
for `gen_set_usr_field_if`

>
>> -----Original Message-----
>> From: Anton Johansson<anjo@rev.ng>
>> Sent: Wednesday, February 9, 2022 11:03 AM
>> To:qemu-devel@nongnu.org
>> Cc:ale@rev.ng; Taylor Simpson<tsimpson@quicinc.com>; Brian Cain
>> <bcain@quicinc.com>; Michael Lambert<mlambert@quicinc.com>;
>> babush@rev.ng;nizzo@rev.ng;richard.henderson@linaro.org
>> Subject: [PATCH v8 10/12] target/hexagon: import parser for idef-parser
>>
>> Signed-off-by: Alessandro Di Federico<ale@rev.ng>
>> Signed-off-by: Paolo Montesel<babush@rev.ng>
>> Signed-off-by: Anton Johansson<anjo@rev.ng>
>
>> diff --git a/target/hexagon/idef-parser/parser-helpers.c
>> b/target/hexagon/idef-parser/parser-helpers.c
>> new file mode 100644
>
>
>> +void gen_set_overflow(Context *c, YYLTYPE *locp, HexValue *value)
>> +{
>> +    HexValue value_m = *value;
>> +
>> +    if (is_inside_ternary(c)) {
>> +        /* Inside ternary operator, need to take care of the side-effect */
>> +        HexValue cond = get_ternary_cond(c, locp);
>> +        HexValue zero = gen_constant(c, locp, "0", cond.bit_width,
>> UNSIGNED);
>> +        bool is_64bit = cond.bit_width == 64;
>> +        unsigned bit_width = cond.bit_width;
>> +        value_m = rvalue_materialize(c, locp, &value_m);
>> +        if (is_64bit) {
>> +            value_m = gen_rvalue_extend(c, locp, &value_m);
>> +        }
>> +        OUT(c, locp, "tcg_gen_movcond_i", &bit_width,
>> +                     "(TCG_COND_NE, ", &value_m, ", ", &cond);
>> +        OUT(c, locp, ", ", &zero, ", ", &value_m, ", ", &zero, ");\n");
> You shouldn't write zero when the condition is false - you should do nothing.  Try a test where OVF is already set.  You can't overwrite the bit with zero when the current instruction doesn't overflow.
>
>
>
>> +        if (is_64bit) {
>> +            value_m = gen_rvalue_truncate(c, locp, &value_m);
>> +        }
>> +        gen_rvalue_free(c, locp, &cond);
>> +    }
>> +
>> +    OUT(c, locp, "SET_USR_FIELD(USR_OVF, ", &value_m, ");\n");
>> +    gen_rvalue_free(c, locp, &value_m);
>> +}

[-- Attachment #2: Type: text/html, Size: 4983 bytes --]