[RFC PATCH v4 2/5] accel/tcg: Have cpu_ld*_mmu() methods consider the MO_SIGN flag

Philippe Mathieu-Daudé posted 5 patches 5 days, 18 hours ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Laurent Vivier <laurent@vivier.eu>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>
[RFC PATCH v4 2/5] accel/tcg: Have cpu_ld*_mmu() methods consider the MO_SIGN flag
Posted by Philippe Mathieu-Daudé 5 days, 18 hours ago
Sign-extend the result when the MO_SIGN flag is requested
in TCG common CPU TLB handling.

Remote the SPARC specific handling.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/cputlb.c         | 37 +++++++++++++++++++++++++++++++------
 target/sparc/ldst_helper.c |  9 ---------
 2 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 76546c66515..97782997881 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2296,12 +2296,18 @@ static uint8_t do_ld1_mmu(CPUState *cpu, vaddr addr, MemOpIdx oi,
 {
     MMULookupLocals l;
     bool crosspage;
+    uint8_t ret;
 
     cpu_req_mo(cpu, TCG_MO_LD_LD | TCG_MO_ST_LD);
     crosspage = mmu_lookup(cpu, addr, oi, ra, access_type, &l);
     tcg_debug_assert(!crosspage);
 
-    return do_ld_1(cpu, &l.page[0], l.mmu_idx, access_type, ra);
+    ret = do_ld_1(cpu, &l.page[0], l.mmu_idx, access_type, ra);
+
+    if (l.memop & MO_SIGN) {
+        ret = (int8_t)ret;
+    }
+    return ret;
 }
 
 static uint16_t do_ld2_mmu(CPUState *cpu, vaddr addr, MemOpIdx oi,
@@ -2315,7 +2321,8 @@ static uint16_t do_ld2_mmu(CPUState *cpu, vaddr addr, MemOpIdx oi,
     cpu_req_mo(cpu, TCG_MO_LD_LD | TCG_MO_ST_LD);
     crosspage = mmu_lookup(cpu, addr, oi, ra, access_type, &l);
     if (likely(!crosspage)) {
-        return do_ld_2(cpu, &l.page[0], l.mmu_idx, access_type, l.memop, ra);
+        ret = do_ld_2(cpu, &l.page[0], l.mmu_idx, access_type, l.memop, ra);
+        goto return_signextended;
     }
 
     a = do_ld_1(cpu, &l.page[0], l.mmu_idx, access_type, ra);
@@ -2326,6 +2333,10 @@ static uint16_t do_ld2_mmu(CPUState *cpu, vaddr addr, MemOpIdx oi,
     } else {
         ret = b | (a << 8);
     }
+return_signextended:
+    if (l.memop & MO_SIGN) {
+        ret = (int16_t)ret;
+    }
     return ret;
 }
 
@@ -2339,7 +2350,8 @@ static uint32_t do_ld4_mmu(CPUState *cpu, vaddr addr, MemOpIdx oi,
     cpu_req_mo(cpu, TCG_MO_LD_LD | TCG_MO_ST_LD);
     crosspage = mmu_lookup(cpu, addr, oi, ra, access_type, &l);
     if (likely(!crosspage)) {
-        return do_ld_4(cpu, &l.page[0], l.mmu_idx, access_type, l.memop, ra);
+        ret = do_ld_4(cpu, &l.page[0], l.mmu_idx, access_type, l.memop, ra);
+        goto return_signextended;
     }
 
     ret = do_ld_beN(cpu, &l.page[0], 0, l.mmu_idx, access_type, l.memop, ra);
@@ -2347,6 +2359,10 @@ static uint32_t do_ld4_mmu(CPUState *cpu, vaddr addr, MemOpIdx oi,
     if ((l.memop & MO_BSWAP) == MO_LE) {
         ret = bswap32(ret);
     }
+return_signextended:
+    if (l.memop & MO_SIGN) {
+        ret = (int32_t)ret;
+    }
     return ret;
 }
 
@@ -2360,7 +2376,8 @@ static uint64_t do_ld8_mmu(CPUState *cpu, vaddr addr, MemOpIdx oi,
     cpu_req_mo(cpu, TCG_MO_LD_LD | TCG_MO_ST_LD);
     crosspage = mmu_lookup(cpu, addr, oi, ra, access_type, &l);
     if (likely(!crosspage)) {
-        return do_ld_8(cpu, &l.page[0], l.mmu_idx, access_type, l.memop, ra);
+        ret = do_ld_8(cpu, &l.page[0], l.mmu_idx, access_type, l.memop, ra);
+        goto return_signextended;
     }
 
     ret = do_ld_beN(cpu, &l.page[0], 0, l.mmu_idx, access_type, l.memop, ra);
@@ -2368,6 +2385,10 @@ static uint64_t do_ld8_mmu(CPUState *cpu, vaddr addr, MemOpIdx oi,
     if ((l.memop & MO_BSWAP) == MO_LE) {
         ret = bswap64(ret);
     }
+return_signextended:
+    if (l.memop & MO_SIGN) {
+        ret = (int64_t)ret;
+    }
     return ret;
 }
 
@@ -2396,7 +2417,7 @@ static Int128 do_ld16_mmu(CPUState *cpu, vaddr addr,
                 ret = bswap128(ret);
             }
         }
-        return ret;
+        goto return_signextended;
     }
 
     first = l.page[0].size;
@@ -2410,7 +2431,7 @@ static Int128 do_ld16_mmu(CPUState *cpu, vaddr addr,
         } else {
             ret = int128_make128(b, a);
         }
-        return ret;
+        goto return_signextended;
     }
 
     if (first < 8) {
@@ -2429,6 +2450,10 @@ static Int128 do_ld16_mmu(CPUState *cpu, vaddr addr,
     if ((l.memop & MO_BSWAP) == MO_LE) {
         ret = bswap128(ret);
     }
+return_signextended:
+    if (l.memop & MO_SIGN) {
+        ret = int128_signextend(ret);
+    }
     return ret;
 }
 
diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
index 3bc6a6827a4..e1178e53d51 100644
--- a/target/sparc/ldst_helper.c
+++ b/target/sparc/ldst_helper.c
@@ -1163,27 +1163,18 @@ uint64_t helper_ld_code(CPUSPARCState *env, target_ulong addr, uint32_t oi)
     switch (mop & MO_SIZE) {
     case MO_8:
         ret = cpu_ldb_code_mmu(env, addr, oi, ra);
-        if (mop & MO_SIGN) {
-            ret = (int8_t)ret;
-        }
         break;
     case MO_16:
         ret = cpu_ldw_code_mmu(env, addr, oi, ra);
         if ((mop & MO_BSWAP) != MO_TE) {
             ret = bswap16(ret);
         }
-        if (mop & MO_SIGN) {
-            ret = (int16_t)ret;
-        }
         break;
     case MO_32:
         ret = cpu_ldl_code_mmu(env, addr, oi, ra);
         if ((mop & MO_BSWAP) != MO_TE) {
             ret = bswap32(ret);
         }
-        if (mop & MO_SIGN) {
-            ret = (int32_t)ret;
-        }
         break;
     case MO_64:
         ret = cpu_ldq_code_mmu(env, addr, oi, ra);
-- 
2.52.0


Re: [RFC PATCH v4 2/5] accel/tcg: Have cpu_ld*_mmu() methods consider the MO_SIGN flag
Posted by Richard Henderson 5 days, 15 hours ago
On 2/4/26 09:00, Philippe Mathieu-Daudé wrote:
> Sign-extend the result when the MO_SIGN flag is requested
> in TCG common CPU TLB handling.
> 
> Remote the SPARC specific handling.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   accel/tcg/cputlb.c         | 37 +++++++++++++++++++++++++++++++------
>   target/sparc/ldst_helper.c |  9 ---------
>   2 files changed, 31 insertions(+), 15 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 76546c66515..97782997881 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -2296,12 +2296,18 @@ static uint8_t do_ld1_mmu(CPUState *cpu, vaddr addr, MemOpIdx oi,
>   {
>       MMULookupLocals l;
>       bool crosspage;
> +    uint8_t ret;
>   
>       cpu_req_mo(cpu, TCG_MO_LD_LD | TCG_MO_ST_LD);
>       crosspage = mmu_lookup(cpu, addr, oi, ra, access_type, &l);
>       tcg_debug_assert(!crosspage);
>   
> -    return do_ld_1(cpu, &l.page[0], l.mmu_idx, access_type, ra);
> +    ret = do_ld_1(cpu, &l.page[0], l.mmu_idx, access_type, ra);
> +
> +    if (l.memop & MO_SIGN) {
> +        ret = (int8_t)ret;
> +    }
> +    return ret;
>   }

Nope.  The return type is uint8_t, so this doesn't change anything at all.


r~

Re: [RFC PATCH v4 2/5] accel/tcg: Have cpu_ld*_mmu() methods consider the MO_SIGN flag
Posted by Philippe Mathieu-Daudé 4 days, 21 hours ago
On 4/2/26 02:49, Richard Henderson wrote:
> On 2/4/26 09:00, Philippe Mathieu-Daudé wrote:
>> Sign-extend the result when the MO_SIGN flag is requested
>> in TCG common CPU TLB handling.
>>
>> Remote the SPARC specific handling.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   accel/tcg/cputlb.c         | 37 +++++++++++++++++++++++++++++++------
>>   target/sparc/ldst_helper.c |  9 ---------
>>   2 files changed, 31 insertions(+), 15 deletions(-)
>>
>> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
>> index 76546c66515..97782997881 100644
>> --- a/accel/tcg/cputlb.c
>> +++ b/accel/tcg/cputlb.c
>> @@ -2296,12 +2296,18 @@ static uint8_t do_ld1_mmu(CPUState *cpu, vaddr 
>> addr, MemOpIdx oi,
>>   {
>>       MMULookupLocals l;
>>       bool crosspage;
>> +    uint8_t ret;
>>       cpu_req_mo(cpu, TCG_MO_LD_LD | TCG_MO_ST_LD);
>>       crosspage = mmu_lookup(cpu, addr, oi, ra, access_type, &l);
>>       tcg_debug_assert(!crosspage);
>> -    return do_ld_1(cpu, &l.page[0], l.mmu_idx, access_type, ra);
>> +    ret = do_ld_1(cpu, &l.page[0], l.mmu_idx, access_type, ra);
>> +
>> +    if (l.memop & MO_SIGN) {
>> +        ret = (int8_t)ret;
>> +    }
>> +    return ret;
>>   }
> 
> Nope.  The return type is uint8_t, so this doesn't change anything at all.

Oh, right <:)