On Thu, Feb 17, 2022 at 4:48 AM Richard Henderson <richard.hender...@linaro.org> wrote: > > The dh_alias redirect is intended to handle TCG types as distinguished > from C types. TCG does not distinguish signed int from unsigned int, > because they are the same size. However, we need to retain this > distinction for dh_typecode, lest we fail to extend abi types properly > for the host call parameters.
Thank you Richard and Keith for the fix for - https://github.com/keith-packard/snek/issues/58 - https://gitlab.com/qemu-project/qemu/-/issues/876 I did apply that and tested it on s390x with the load that originally found it. With qemu 6.2 + that patch I ran the full testsuite of snek-arm on s390x and it works great now. If you like feel free to add any combination of the following when committing: Fixes: #876 Fixes: 7319d83a735 ("tcg: Combine dh_is_64bit and dh_is_signed to dh_typecode") Reported-by: Christian Ehrhardt <christian.ehrha...@canonical.com> Tested-by: Christian Ehrhardt <christian.ehrha...@canonical.com> > 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 > > Reported-by: Keith Packard <kei...@keithp.com> > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > --- > include/exec/helper-head.h | 19 ++++++++++--------- > target/hppa/helper.h | 2 ++ > target/i386/ops_sse_header.h | 3 +++ > target/m68k/helper.h | 1 + > target/ppc/helper.h | 3 +++ > 5 files changed, 19 insertions(+), 9 deletions(-) > > diff --git a/include/exec/helper-head.h b/include/exec/helper-head.h > index b974eb394a..734af067fe 100644 > --- a/include/exec/helper-head.h > +++ b/include/exec/helper-head.h > @@ -53,13 +53,16 @@ > # ifdef TARGET_LONG_BITS > # if TARGET_LONG_BITS == 32 > # define dh_alias_tl i32 > +# define dh_typecode_tl dh_typecode_i32 > # else > # define dh_alias_tl i64 > +# define dh_typecode_tl dh_typecode_i64 > # endif > # endif > -# define dh_alias_env ptr > # define dh_ctype_tl target_ulong > +# define dh_alias_env ptr > # define dh_ctype_env CPUArchState * > +# define dh_typecode_env dh_typecode_ptr > #endif > > /* We can't use glue() here because it falls foul of C preprocessor > @@ -92,18 +95,16 @@ > #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_int dh_typecode_s32 > +#define dh_typecode_f16 dh_typecode_i32 > +#define dh_typecode_f32 dh_typecode_i32 > +#define dh_typecode_f64 dh_typecode_i64 > +#define dh_typecode_cptr dh_typecode_ptr > +#define dh_typecode(t) dh_typecode_##t > > #define dh_callflag_i32 0 > -#define dh_callflag_s32 0 > -#define dh_callflag_int 0 > #define dh_callflag_i64 0 > -#define dh_callflag_s64 0 > -#define dh_callflag_f16 0 > -#define dh_callflag_f32 0 > -#define dh_callflag_f64 0 > #define dh_callflag_ptr 0 > -#define dh_callflag_cptr dh_callflag_ptr > #define dh_callflag_void 0 > #define dh_callflag_noreturn TCG_CALL_NO_RETURN > #define dh_callflag(t) glue(dh_callflag_, dh_alias(t)) > diff --git a/target/hppa/helper.h b/target/hppa/helper.h > index fe8a9ce493..c7e35ce8c7 100644 > --- a/target/hppa/helper.h > +++ b/target/hppa/helper.h > @@ -1,7 +1,9 @@ > #if TARGET_REGISTER_BITS == 64 > # define dh_alias_tr i64 > +# define dh_typecode_tr dh_typecode_i64 > #else > # define dh_alias_tr i32 > +# define dh_typecode_tr dh_typecode_i32 > #endif > #define dh_ctype_tr target_ureg > > diff --git a/target/i386/ops_sse_header.h b/target/i386/ops_sse_header.h > index e68af5c403..cef28f2aae 100644 > --- a/target/i386/ops_sse_header.h > +++ b/target/i386/ops_sse_header.h > @@ -30,6 +30,9 @@ > #define dh_ctype_Reg Reg * > #define dh_ctype_ZMMReg ZMMReg * > #define dh_ctype_MMXReg MMXReg * > +#define dh_typecode_Reg dh_typecode_ptr > +#define dh_typecode_ZMMReg dh_typecode_ptr > +#define dh_typecode_MMXReg dh_typecode_ptr > > DEF_HELPER_3(glue(psrlw, SUFFIX), void, env, Reg, Reg) > DEF_HELPER_3(glue(psraw, SUFFIX), void, env, Reg, Reg) > diff --git a/target/m68k/helper.h b/target/m68k/helper.h > index 9842eeaa95..0a6b4146f6 100644 > --- a/target/m68k/helper.h > +++ b/target/m68k/helper.h > @@ -17,6 +17,7 @@ DEF_HELPER_4(cas2l_parallel, void, env, i32, i32, i32) > > #define dh_alias_fp ptr > #define dh_ctype_fp FPReg * > +#define dh_typecode_fp dh_typecode_ptr > > DEF_HELPER_3(exts32, void, env, fp, s32) > DEF_HELPER_3(extf32, void, env, fp, f32) > diff --git a/target/ppc/helper.h b/target/ppc/helper.h > index ab008c9d4e..ae7d503fcf 100644 > --- a/target/ppc/helper.h > +++ b/target/ppc/helper.h > @@ -127,9 +127,11 @@ DEF_HELPER_FLAGS_1(ftsqrt, TCG_CALL_NO_RWG_SE, i32, i64) > > #define dh_alias_avr ptr > #define dh_ctype_avr ppc_avr_t * > +#define dh_typecode_avr dh_typecode_ptr > > #define dh_alias_vsr ptr > #define dh_ctype_vsr ppc_vsr_t * > +#define dh_typecode_vsr dh_typecode_ptr > > DEF_HELPER_3(vavgub, void, avr, avr, avr) > DEF_HELPER_3(vavguh, void, avr, avr, avr) > @@ -708,6 +710,7 @@ DEF_HELPER_3(store_dbatu, void, env, i32, tl) > > #define dh_alias_fprp ptr > #define dh_ctype_fprp ppc_fprp_t * > +#define dh_typecode_fprp dh_typecode_ptr > > DEF_HELPER_4(DADD, void, env, fprp, fprp, fprp) > DEF_HELPER_4(DADDQ, void, env, fprp, fprp, fprp) > -- > 2.25.1 > > -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd
