| Message ID | 20231019182921.1772928-4-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg: Expose tcg_gen_ext_{i32,i64,tl} | expand |
On 19/10/23 20:29, Richard Henderson wrote: > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/i386/tcg/translate.c | 28 +++------------------------- > 1 file changed, 3 insertions(+), 25 deletions(-) > > diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c > index 0c81e066de..d420ed8f0a 100644 > --- a/target/i386/tcg/translate.c > +++ b/target/i386/tcg/translate.c > @@ -701,33 +701,11 @@ static inline void gen_op_movl_T0_Dshift(DisasContext *s, MemOp ot) > > static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size, bool sign) > { > - switch (size) { > - case MO_8: > - if (sign) { > - tcg_gen_ext8s_tl(dst, src); > - } else { > - tcg_gen_ext8u_tl(dst, src); > - } > - return dst; > - case MO_16: > - if (sign) { > - tcg_gen_ext16s_tl(dst, src); > - } else { > - tcg_gen_ext16u_tl(dst, src); > - } > - return dst; > -#ifdef TARGET_X86_64 > - case MO_32: > - if (sign) { > - tcg_gen_ext32s_tl(dst, src); > - } else { > - tcg_gen_ext32u_tl(dst, src); > - } > - return dst; > -#endif > - default: > + if (memop_size(size) == TARGET_LONG_BITS) { > return src; > } > + tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0)); > + return dst; > } While here, I'd rename 'size' -> 'mop'. Regardless, Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 10/19/23 23:57, Philippe Mathieu-Daudé wrote: > On 19/10/23 20:29, Richard Henderson wrote: >> - default: >> + if (memop_size(size) == TARGET_LONG_BITS) { >> return src; >> } Any opinions about adding something like this on top? ------------------------- 8< ------------------------------- From: Paolo Bonzini <pbonzini@redhat.com> Subject: [PATCH] include, target/i386: define and use MO_TL This will also come in handy later for "less than" comparisons. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> diff --git a/include/exec/target_long.h b/include/exec/target_long.h index 93c9472971f..3cd8e26a23f 100644 --- a/include/exec/target_long.h +++ b/include/exec/target_long.h @@ -29,12 +29,14 @@ typedef uint32_t target_ulong; #define TARGET_FMT_lx "%08x" #define TARGET_FMT_ld "%d" #define TARGET_FMT_lu "%u" +#define MO_TL MO_32 #elif TARGET_LONG_SIZE == 8 typedef int64_t target_long; typedef uint64_t target_ulong; #define TARGET_FMT_lx "%016" PRIx64 #define TARGET_FMT_ld "%" PRId64 #define TARGET_FMT_lu "%" PRIu64 +#define MO_TL MO_64 #else #error TARGET_LONG_SIZE undefined #endif diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index b0d62e83445..7bf7406dd8e 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -701,7 +701,7 @@ static inline void gen_op_movl_T0_Dshift(DisasContext *s, MemOp ot) static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size, bool sign) { - if (memop_size(size) == TARGET_LONG_BITS) { + if (size == MO_TL) { return src; } tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0)); ----------------------------------------------------------- I can add it in my x86 series if desirable (I also have an occurrence of memop_size(ot) < TARGET_LONG_BITS there, and it can become just "ot < MO_TL"). >> + tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0)); >> + return dst; >> } > > While here, I'd rename 'size' -> 'mop'. Regardless, Not sure about that, because "size" should be just the low bits of MemOp (the MO_SIGN bit is passed separately). Paolo
On 10/21/23 00:59, Paolo Bonzini wrote: > On 10/19/23 23:57, Philippe Mathieu-Daudé wrote: >> On 19/10/23 20:29, Richard Henderson wrote: >>> - default: >>> + if (memop_size(size) == TARGET_LONG_BITS) { >>> return src; >>> } > > Any opinions about adding something like this on top? > > ------------------------- 8< ------------------------------- > From: Paolo Bonzini <pbonzini@redhat.com> > Subject: [PATCH] include, target/i386: define and use MO_TL Yes, that looks fine. > static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size, bool sign) > { > - if (memop_size(size) == TARGET_LONG_BITS) { > + if (size == MO_TL) { Yep. > I can add it in my x86 series if desirable ... That's probably fine; you may well get your PR in before my next. >>> + tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0)); >>> + return dst; >>> } >> >> While here, I'd rename 'size' -> 'mop'. Regardless, > > Not sure about that, because "size" should be just the low bits of MemOp (the MO_SIGN bit > is passed separately). Agreed. r~
Il dom 22 ott 2023, 03:29 Richard Henderson <richard.henderson@linaro.org> ha scritto: > > static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size, bool sign) > > { > > - if (memop_size(size) == TARGET_LONG_BITS) { > > + if (size == MO_TL) { > > Yep. > > > I can add it in my x86 series if desirable ... > > That's probably fine; you may well get your PR in before my next. > I will probably keep only SHA instructions for 8.2 (plus the VEX todos and the reorganized checks) and delay the rest. There are a bunch of things I would do in a slightly different manner now, so it's better to clean up the generic x86 decoder code before implementing the less orthogonal instruction formats from the one-byte. I should have time to finish opcodes 0xC0 to 0xFF over the Christmas break in time for 9.0. :) Paolo > >>> + tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0)); > >>> + return dst; > >>> } > >> > >> While here, I'd rename 'size' -> 'mop'. Regardless, > > > > Not sure about that, because "size" should be just the low bits of MemOp > (the MO_SIGN bit > > is passed separately). > > Agreed. > > > r~ > >
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index 0c81e066de..d420ed8f0a 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -701,33 +701,11 @@ static inline void gen_op_movl_T0_Dshift(DisasContext *s, MemOp ot) static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size, bool sign) { - switch (size) { - case MO_8: - if (sign) { - tcg_gen_ext8s_tl(dst, src); - } else { - tcg_gen_ext8u_tl(dst, src); - } - return dst; - case MO_16: - if (sign) { - tcg_gen_ext16s_tl(dst, src); - } else { - tcg_gen_ext16u_tl(dst, src); - } - return dst; -#ifdef TARGET_X86_64 - case MO_32: - if (sign) { - tcg_gen_ext32s_tl(dst, src); - } else { - tcg_gen_ext32u_tl(dst, src); - } - return dst; -#endif - default: + if (memop_size(size) == TARGET_LONG_BITS) { return src; } + tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0)); + return dst; } static void gen_extu(MemOp ot, TCGv reg)
Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/i386/tcg/translate.c | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-)