Re: [PATCH 3/7] target/i386: Use tcg_gen_ext_tl

[Paolo Bonzini <pbonzini@redhat.com>]

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