[PATCH v10 15/21] target/arm/tcg/translate.c: replace TCGv with TCGv_va

Pierrick Bouvier posted 21 patches 3 days, 22 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>, Peter Maydell <peter.maydell@linaro.org>, Michael Rolnik <mrolnik@gmail.com>, Brian Cain <brian.cain@oss.qualcomm.com>, Helge Deller <deller@gmx.de>, Song Gao <gaosong@loongson.cn>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <arikalo@gmail.com>, Stafford Horne <shorne@gmail.com>, Nicholas Piggin <npiggin@gmail.com>, Chinmay Rath <rathc@linux.ibm.com>, Glenn Miles <milesg@linux.ibm.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu.zevorn@gmail.com>, Yoshinori Sato <yoshinori.sato@nifty.com>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>, Cornelia Huck <cohuck@redhat.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Bastian Koppelmann <kbastian@rumtueddeln.de>, Max Filippov <jcmvbkbc@gmail.com>
There is a newer version of this series
[PATCH v10 15/21] target/arm/tcg/translate.c: replace TCGv with TCGv_va
Posted by Pierrick Bouvier 3 days, 22 hours ago
We know this file is for 32-bit runtime target, so we can set
TCG_ADDRESS_BITS. TCG_TYPE_VA is derived accordingly and is already
passed to translator_loop.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/tcg/translate.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index fa4c7907dcd..0b3b4ab86be 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -22,7 +22,8 @@
 
 #include "translate.h"
 #include "translate-a32.h"
-#include "tcg/tcg-op.h"
+#define TCG_ADDRESS_BITS 32
+#include "tcg/tcg-op-mem.h"
 #include "qemu/log.h"
 #include "arm_ldst.h"
 #include "semihosting/semihost.h"
@@ -910,14 +911,14 @@ MemOp pow2_align(unsigned i)
  * that the address argument is TCGv_i32 rather than TCGv.
  */
 
-static TCGv gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
+static TCGv_va gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
 {
-    TCGv addr = tcg_temp_new();
-    tcg_gen_extu_i32_tl(addr, a32);
+    TCGv_va addr = tcgv_va_temp_new();
+    tcg_gen_mov_i32(addr, a32);
 
     /* Not needed for user-mode BE32, where we use MO_BE instead.  */
     if (!IS_USER_ONLY && s->sctlr_b && (op & MO_SIZE) < MO_32) {
-        tcg_gen_xori_tl(addr, addr, 4 - (1 << (op & MO_SIZE)));
+        tcg_gen_xori_i32(addr, addr, 4 - (1 << (op & MO_SIZE)));
     }
     return addr;
 }
@@ -929,21 +930,21 @@ static TCGv gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
 void gen_aa32_ld_internal_i32(DisasContext *s, TCGv_i32 val,
                               TCGv_i32 a32, int index, MemOp opc)
 {
-    TCGv addr = gen_aa32_addr(s, a32, opc);
+    TCGv_va addr = gen_aa32_addr(s, a32, opc);
     tcg_gen_qemu_ld_i32(val, addr, index, opc);
 }
 
 void gen_aa32_st_internal_i32(DisasContext *s, TCGv_i32 val,
                               TCGv_i32 a32, int index, MemOp opc)
 {
-    TCGv addr = gen_aa32_addr(s, a32, opc);
+    TCGv_va addr = gen_aa32_addr(s, a32, opc);
     tcg_gen_qemu_st_i32(val, addr, index, opc);
 }
 
 void gen_aa32_ld_internal_i64(DisasContext *s, TCGv_i64 val,
                               TCGv_i32 a32, int index, MemOp opc)
 {
-    TCGv addr = gen_aa32_addr(s, a32, opc);
+    TCGv_va addr = gen_aa32_addr(s, a32, opc);
 
     tcg_gen_qemu_ld_i64(val, addr, index, opc);
 
@@ -956,7 +957,7 @@ void gen_aa32_ld_internal_i64(DisasContext *s, TCGv_i64 val,
 void gen_aa32_st_internal_i64(DisasContext *s, TCGv_i64 val,
                               TCGv_i32 a32, int index, MemOp opc)
 {
-    TCGv addr = gen_aa32_addr(s, a32, opc);
+    TCGv_va addr = gen_aa32_addr(s, a32, opc);
 
     /* Not needed for user-mode BE32, where we use MO_BE instead.  */
     if (!IS_USER_ONLY && s->sctlr_b && (opc & MO_SIZE) == MO_64) {
@@ -2036,7 +2037,7 @@ static void gen_load_exclusive(DisasContext *s, int rt, int rt2,
          * architecturally 64-bit access, but instead do a 64-bit access
          * using MO_BE if appropriate and then split the two halves.
          */
-        TCGv taddr = gen_aa32_addr(s, addr, opc);
+        TCGv_va taddr = gen_aa32_addr(s, addr, opc);
 
         tcg_gen_qemu_ld_i64(t64, taddr, get_mem_index(s), opc);
         tcg_gen_mov_i64(cpu_exclusive_val, t64);
@@ -2065,7 +2066,7 @@ static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
 {
     TCGv_i32 t0, t1, t2;
     TCGv_i64 extaddr;
-    TCGv taddr;
+    TCGv_va taddr;
     TCGLabel *done_label;
     TCGLabel *fail_label;
     MemOp opc = size | MO_ALIGN | s->be_data;
@@ -3792,7 +3793,7 @@ static void do_ldrd_load(DisasContext *s, TCGv_i32 addr, int rt, int rt2)
      */
     int mem_idx = get_mem_index(s);
     MemOp opc = MO_64 | MO_ALIGN_4 | MO_ATOM_SUBALIGN | s->be_data;
-    TCGv taddr = gen_aa32_addr(s, addr, opc);
+    TCGv_va taddr = gen_aa32_addr(s, addr, opc);
     TCGv_i64 t64 = tcg_temp_new_i64();
     TCGv_i32 tmp = tcg_temp_new_i32();
     TCGv_i32 tmp2 = tcg_temp_new_i32();
@@ -3847,7 +3848,7 @@ static void do_strd_store(DisasContext *s, TCGv_i32 addr, int rt, int rt2)
      */
     int mem_idx = get_mem_index(s);
     MemOp opc = MO_64 | MO_ALIGN_4 | MO_ATOM_SUBALIGN | s->be_data;
-    TCGv taddr = gen_aa32_addr(s, addr, opc);
+    TCGv_va taddr = gen_aa32_addr(s, addr, opc);
     TCGv_i32 t1 = load_reg(s, rt);
     TCGv_i32 t2 = load_reg(s, rt2);
     TCGv_i64 t64 = tcg_temp_new_i64();
@@ -4068,7 +4069,7 @@ DO_LDST(STRH, store, MO_UW)
 static bool op_swp(DisasContext *s, arg_SWP *a, MemOp opc)
 {
     TCGv_i32 addr, tmp;
-    TCGv taddr;
+    TCGv_va taddr;
 
     opc |= s->be_data;
     addr = load_reg(s, a->rn);
-- 
2.47.3
Re: [PATCH v10 15/21] target/arm/tcg/translate.c: replace TCGv with TCGv_va
Posted by Philippe Mathieu-Daudé 3 days, 21 hours ago
On 7/4/26 21:59, Pierrick Bouvier wrote:
> We know this file is for 32-bit runtime target, so we can set
> TCG_ADDRESS_BITS. TCG_TYPE_VA is derived accordingly and is already
> passed to translator_loop.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   target/arm/tcg/translate.c | 29 +++++++++++++++--------------
>   1 file changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
> index fa4c7907dcd..0b3b4ab86be 100644
> --- a/target/arm/tcg/translate.c
> +++ b/target/arm/tcg/translate.c
> @@ -22,7 +22,8 @@
>   
>   #include "translate.h"
>   #include "translate-a32.h"
> -#include "tcg/tcg-op.h"
> +#define TCG_ADDRESS_BITS 32
> +#include "tcg/tcg-op-mem.h"
>   #include "qemu/log.h"
>   #include "arm_ldst.h"
>   #include "semihosting/semihost.h"
> @@ -910,14 +911,14 @@ MemOp pow2_align(unsigned i)
>    * that the address argument is TCGv_i32 rather than TCGv.
>    */
>   
> -static TCGv gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
> +static TCGv_va gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
>   {
> -    TCGv addr = tcg_temp_new();
> -    tcg_gen_extu_i32_tl(addr, a32);
> +    TCGv_va addr = tcgv_va_temp_new();
> +    tcg_gen_mov_i32(addr, a32);

The implicit TCGv_va -> TCGv_i32 bugs me here, but the overall change
will make my sleep better :) So:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>   
>       /* Not needed for user-mode BE32, where we use MO_BE instead.  */
>       if (!IS_USER_ONLY && s->sctlr_b && (op & MO_SIZE) < MO_32) {
> -        tcg_gen_xori_tl(addr, addr, 4 - (1 << (op & MO_SIZE)));
> +        tcg_gen_xori_i32(addr, addr, 4 - (1 << (op & MO_SIZE)));
>       }
>       return addr;
>   }
> @@ -929,21 +930,21 @@ static TCGv gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
>   void gen_aa32_ld_internal_i32(DisasContext *s, TCGv_i32 val,
>                                 TCGv_i32 a32, int index, MemOp opc)
>   {
> -    TCGv addr = gen_aa32_addr(s, a32, opc);
> +    TCGv_va addr = gen_aa32_addr(s, a32, opc);
>       tcg_gen_qemu_ld_i32(val, addr, index, opc);
>   }


Re: [PATCH v10 15/21] target/arm/tcg/translate.c: replace TCGv with TCGv_va
Posted by Pierrick Bouvier 3 days, 20 hours ago
On 4/7/26 2:02 PM, Philippe Mathieu-Daudé wrote:
> On 7/4/26 21:59, Pierrick Bouvier wrote:
>> We know this file is for 32-bit runtime target, so we can set
>> TCG_ADDRESS_BITS. TCG_TYPE_VA is derived accordingly and is already
>> passed to translator_loop.
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    target/arm/tcg/translate.c | 29 +++++++++++++++--------------
>>    1 file changed, 15 insertions(+), 14 deletions(-)
>>
>> diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
>> index fa4c7907dcd..0b3b4ab86be 100644
>> --- a/target/arm/tcg/translate.c
>> +++ b/target/arm/tcg/translate.c
>> @@ -22,7 +22,8 @@
>>    
>>    #include "translate.h"
>>    #include "translate-a32.h"
>> -#include "tcg/tcg-op.h"
>> +#define TCG_ADDRESS_BITS 32
>> +#include "tcg/tcg-op-mem.h"
>>    #include "qemu/log.h"
>>    #include "arm_ldst.h"
>>    #include "semihosting/semihost.h"
>> @@ -910,14 +911,14 @@ MemOp pow2_align(unsigned i)
>>     * that the address argument is TCGv_i32 rather than TCGv.
>>     */
>>    
>> -static TCGv gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
>> +static TCGv_va gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
>>    {
>> -    TCGv addr = tcg_temp_new();
>> -    tcg_gen_extu_i32_tl(addr, a32);
>> +    TCGv_va addr = tcgv_va_temp_new();
>> +    tcg_gen_mov_i32(addr, a32);
> 
> The implicit TCGv_va -> TCGv_i32 bugs me here, but the overall change
> will make my sleep better :) So:
>

We could use TCGv_i32 directly, TCGv_va is just useful here to document 
the intention. I can update this if Richard and you prefer this.

> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
>>    
>>        /* Not needed for user-mode BE32, where we use MO_BE instead.  */
>>        if (!IS_USER_ONLY && s->sctlr_b && (op & MO_SIZE) < MO_32) {
>> -        tcg_gen_xori_tl(addr, addr, 4 - (1 << (op & MO_SIZE)));
>> +        tcg_gen_xori_i32(addr, addr, 4 - (1 << (op & MO_SIZE)));
>>        }
>>        return addr;
>>    }
>> @@ -929,21 +930,21 @@ static TCGv gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
>>    void gen_aa32_ld_internal_i32(DisasContext *s, TCGv_i32 val,
>>                                  TCGv_i32 a32, int index, MemOp opc)
>>    {
>> -    TCGv addr = gen_aa32_addr(s, a32, opc);
>> +    TCGv_va addr = gen_aa32_addr(s, a32, opc);
>>        tcg_gen_qemu_ld_i32(val, addr, index, opc);
>>    }
> 


Re: [PATCH v10 15/21] target/arm/tcg/translate.c: replace TCGv with TCGv_va
Posted by Richard Henderson 3 days, 20 hours ago
On 4/8/26 08:20, Pierrick Bouvier wrote:
> On 4/7/26 2:02 PM, Philippe Mathieu-Daudé wrote:
>> On 7/4/26 21:59, Pierrick Bouvier wrote:
>>> We know this file is for 32-bit runtime target, so we can set
>>> TCG_ADDRESS_BITS. TCG_TYPE_VA is derived accordingly and is already
>>> passed to translator_loop.
>>>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>>    target/arm/tcg/translate.c | 29 +++++++++++++++--------------
>>>    1 file changed, 15 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
>>> index fa4c7907dcd..0b3b4ab86be 100644
>>> --- a/target/arm/tcg/translate.c
>>> +++ b/target/arm/tcg/translate.c
>>> @@ -22,7 +22,8 @@
>>>    #include "translate.h"
>>>    #include "translate-a32.h"
>>> -#include "tcg/tcg-op.h"
>>> +#define TCG_ADDRESS_BITS 32
>>> +#include "tcg/tcg-op-mem.h"
>>>    #include "qemu/log.h"
>>>    #include "arm_ldst.h"
>>>    #include "semihosting/semihost.h"
>>> @@ -910,14 +911,14 @@ MemOp pow2_align(unsigned i)
>>>     * that the address argument is TCGv_i32 rather than TCGv.
>>>     */
>>> -static TCGv gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
>>> +static TCGv_va gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
>>>    {
>>> -    TCGv addr = tcg_temp_new();
>>> -    tcg_gen_extu_i32_tl(addr, a32);
>>> +    TCGv_va addr = tcgv_va_temp_new();
>>> +    tcg_gen_mov_i32(addr, a32);
>>
>> The implicit TCGv_va -> TCGv_i32 bugs me here, but the overall change
>> will make my sleep better :) So:
>>
> 
> We could use TCGv_i32 directly, TCGv_va is just useful here to document the intention. I 
> can update this if Richard and you prefer this.

I think this is fine, as I'm not keen on introducing a complete set of tcg_gen_foo_va 
macros.  As is, any mismatch will be caught at compile-time.


r~

Re: [PATCH v10 15/21] target/arm/tcg/translate.c: replace TCGv with TCGv_va
Posted by Pierrick Bouvier 3 days, 20 hours ago
On 4/7/26 3:27 PM, Richard Henderson wrote:
> On 4/8/26 08:20, Pierrick Bouvier wrote:
>> On 4/7/26 2:02 PM, Philippe Mathieu-Daudé wrote:
>>> On 7/4/26 21:59, Pierrick Bouvier wrote:
>>>> We know this file is for 32-bit runtime target, so we can set
>>>> TCG_ADDRESS_BITS. TCG_TYPE_VA is derived accordingly and is already
>>>> passed to translator_loop.
>>>>
>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> ---
>>>>     target/arm/tcg/translate.c | 29 +++++++++++++++--------------
>>>>     1 file changed, 15 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
>>>> index fa4c7907dcd..0b3b4ab86be 100644
>>>> --- a/target/arm/tcg/translate.c
>>>> +++ b/target/arm/tcg/translate.c
>>>> @@ -22,7 +22,8 @@
>>>>     #include "translate.h"
>>>>     #include "translate-a32.h"
>>>> -#include "tcg/tcg-op.h"
>>>> +#define TCG_ADDRESS_BITS 32
>>>> +#include "tcg/tcg-op-mem.h"
>>>>     #include "qemu/log.h"
>>>>     #include "arm_ldst.h"
>>>>     #include "semihosting/semihost.h"
>>>> @@ -910,14 +911,14 @@ MemOp pow2_align(unsigned i)
>>>>      * that the address argument is TCGv_i32 rather than TCGv.
>>>>      */
>>>> -static TCGv gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
>>>> +static TCGv_va gen_aa32_addr(DisasContext *s, TCGv_i32 a32, MemOp op)
>>>>     {
>>>> -    TCGv addr = tcg_temp_new();
>>>> -    tcg_gen_extu_i32_tl(addr, a32);
>>>> +    TCGv_va addr = tcgv_va_temp_new();
>>>> +    tcg_gen_mov_i32(addr, a32);
>>>
>>> The implicit TCGv_va -> TCGv_i32 bugs me here, but the overall change
>>> will make my sleep better :) So:
>>>
>>
>> We could use TCGv_i32 directly, TCGv_va is just useful here to document the intention. I
>> can update this if Richard and you prefer this.
> 
> I think this is fine, as I'm not keen on introducing a complete set of tcg_gen_foo_va
> macros.  As is, any mismatch will be caught at compile-time.
>

Agreed. Usage should be reduced to address related functions.

> 
> r~