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

[Anton Johansson via]

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<a...@rev.ng>
Sent: Wednesday, February 9, 2022 11:03 AM
To:qemu-devel@nongnu.org
Cc:a...@rev.ng; Taylor Simpson<tsimp...@quicinc.com>; Brian Cain
<bc...@quicinc.com>; Michael Lambert<mlamb...@quicinc.com>;
bab...@rev.ng;ni...@rev.ng;richard.hender...@linaro.org
Subject: [PATCH v8 10/12] target/hexagon: import parser for idef-parser

Signed-off-by: Alessandro Di Federico<a...@rev.ng>
Signed-off-by: Paolo Montesel<bab...@rev.ng>
Signed-off-by: Anton Johansson<a...@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);
+}