[PATCH] tcg: Add 'signed' bit to typecodes

[Keith Packard]

Commit 7319d83a (tcg: Combine dh_is_64bit and dh_is_signed to
dh_typecode) converted the tcg type system to a 3-bit field from two
separate 1-bit fields. This subtly lost the 'signed' information from
the types as it uses the dh_alias macro to reduce the types down to
basic machine types. However, the dh_alias macro also discards sign
information, aliasing 's32' to 'i32'.

I considered two solutions; switching away from using dh_alias and
expressing typecode values for all types or staying with dh_alias and
re-inserting the sign information. The latter approach turns out a bit
cleaner as it doesn't require dealing with machine-dependent types
like 'tl'.

I re-inserted the dh_is_signed macro with its values and 'or' in that
bit to the unsigned typecode.

This bug was detected when running the 'arm' emulator on an s390
system. The s390 uses TCG_TARGET_EXTEND_ARGS which triggers code
in tcg_gen_callN to extend 32 bit values to 64 bits; the incorrect
sign data in the typemask for each argument caused the values to be
extended as unsigned values.

This simple program exhibits the problem:

        static volatile int num = -9;
        static volatile int den = -5;

        int
        main(void)
        {
                int quo = num / den;
                printf("num %d den %d quo %d\n", num, den, quo);
                exit(0);
        }

When run on the broken qemu, this results in:

        num -9 den -5 quo 0

The correct result is:

        num -9 den -5 quo 1

This issue was originally discovered when running snek on s390x under
qemu 6.2:

        https://github.com/keith-packard/snek/issues/58

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 include/exec/helper-head.h | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/include/exec/helper-head.h b/include/exec/helper-head.h
index b974eb394a..eb1066f939 100644
--- a/include/exec/helper-head.h
+++ b/include/exec/helper-head.h
@@ -85,14 +85,33 @@
 #define dh_retvar_ptr tcgv_ptr_temp(retval)
 #define dh_retvar(t) glue(dh_retvar_, dh_alias(t))
 
+#define dh_is_signed_void 0
+#define dh_is_signed_noreturn 0
+#define dh_is_signed_i32 0
+#define dh_is_signed_s32 1
+#define dh_is_signed_i64 0
+#define dh_is_signed_s64 1
+#define dh_is_signed_f16 0
+#define dh_is_signed_f32 0
+#define dh_is_signed_f64 0
+#define dh_is_signed_tl  0
+#define dh_is_signed_int 1
+/*
+ * ??? This is highly specific to the host cpu.  There are even special
+ * extension instructions that may be required, e.g. ia64's addp4.  But
+ * for now we don't support any 64-bit targets with 32-bit pointers.
+ */
+#define dh_is_signed_ptr 0
+#define dh_is_signed_cptr dh_is_signed_ptr
+#define dh_is_signed_env dh_is_signed_ptr
+#define dh_is_signed(t) dh_is_signed_##t
+
 #define dh_typecode_void 0
 #define dh_typecode_noreturn 0
 #define dh_typecode_i32 2
-#define dh_typecode_s32 3
 #define dh_typecode_i64 4
-#define dh_typecode_s64 5
 #define dh_typecode_ptr 6
-#define dh_typecode(t) glue(dh_typecode_, dh_alias(t))
+#define dh_typecode(t) (glue(dh_typecode_, dh_alias(t)) | dh_is_signed(t))
 
 #define dh_callflag_i32  0
 #define dh_callflag_s32  0
-- 
2.34.1