diff mbox series

[v2,2/4] target/arm: Introduce store_cpu_field_constant() helper

Message ID 20211027045607.1261526-3-f4bug@amsat.org
State New
Headers show
Series target/arm: Use tcg_constant_* | expand

Commit Message

Philippe Mathieu-Daudé Oct. 27, 2021, 4:56 a.m. UTC 
Similarly to the store_cpu_field() helper which takes a TCG
temporary, store its value to the CPUState, introduce the
store_cpu_field_constant() helper which store a constant to
CPUState (without using any TCG temporary).

Update the single store_cpu_offset() user in do_coproc_insn().

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/arm/translate-a32.h | 11 ++++++++---
 target/arm/translate.c     |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

Comments

Richard Henderson Oct. 28, 2021, 1:17 a.m. UTC | #1 
On 10/26/21 9:56 PM, Philippe Mathieu-Daudé wrote:
> -static inline void store_cpu_offset(TCGv_i32 var, int offset)
> +static inline void store_cpu_offset(TCGv_i32 var, int offset, bool is_temp)
>   {
>       tcg_gen_st_i32(var, cpu_env, offset);
> -    tcg_temp_free_i32(var);
> +    if (is_temp) {
> +        tcg_temp_free_i32(var);
> +    }
>   }

You don't need to change the function interface; tcg_constant_* is ignored by free.

> +#define store_cpu_field_constant(val, name) \
> +    store_cpu_offset(tcg_constant_i32(val), offsetof(CPUARMState, name), false)

But this could become simply

     tcg_gen_st_i32(tcg_constant_i32(val), cpu_env,
                    offsetof(CPUARMState, name))

without the wrapper inline.


r~
Philippe Mathieu-Daudé Oct. 29, 2021, 11:07 p.m. UTC | #2 
On 10/28/21 03:17, Richard Henderson wrote:
> On 10/26/21 9:56 PM, Philippe Mathieu-Daudé wrote:
>> -static inline void store_cpu_offset(TCGv_i32 var, int offset)
>> +static inline void store_cpu_offset(TCGv_i32 var, int offset, bool
>> is_temp)
>>   {
>>       tcg_gen_st_i32(var, cpu_env, offset);
>> -    tcg_temp_free_i32(var);
>> +    if (is_temp) {
>> +        tcg_temp_free_i32(var);
>> +    }
>>   }
> 
> You don't need to change the function interface; tcg_constant_* is
> ignored by free.

Now I see that in c0522136adf ("tcg: Introduce TYPE_CONST temporaries").

>> +#define store_cpu_field_constant(val, name) \
>> +    store_cpu_offset(tcg_constant_i32(val), offsetof(CPUARMState,
>> name), false)
> 
> But this could become simply
> 
>     tcg_gen_st_i32(tcg_constant_i32(val), cpu_env,
>                    offsetof(CPUARMState, name))
> 
> without the wrapper inline.

Yep :)
diff mbox series

Patch

diff --git a/target/arm/translate-a32.h b/target/arm/translate-a32.h
index 88f15df60e8..2e708ca3dbc 100644
--- a/target/arm/translate-a32.h
+++ b/target/arm/translate-a32.h
@@ -61,14 +61,19 @@  static inline TCGv_i32 load_cpu_offset(int offset)
 
 #define load_cpu_field(name) load_cpu_offset(offsetof(CPUARMState, name))
 
-static inline void store_cpu_offset(TCGv_i32 var, int offset)
+static inline void store_cpu_offset(TCGv_i32 var, int offset, bool is_temp)
 {
     tcg_gen_st_i32(var, cpu_env, offset);
-    tcg_temp_free_i32(var);
+    if (is_temp) {
+        tcg_temp_free_i32(var);
+    }
 }
 
 #define store_cpu_field(var, name) \
-    store_cpu_offset(var, offsetof(CPUARMState, name))
+    store_cpu_offset(var, offsetof(CPUARMState, name), true)
+
+#define store_cpu_field_constant(val, name) \
+    store_cpu_offset(tcg_constant_i32(val), offsetof(CPUARMState, name), false)
 
 /* Create a new temporary and set it to the value of a CPU register.  */
 static inline TCGv_i32 load_reg(DisasContext *s, int reg)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 083a6d6ed77..5061e55f2c0 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -4855,7 +4855,7 @@  static void do_coproc_insn(DisasContext *s, int cpnum, int is64,
                     tcg_temp_free_i32(tmp);
                 } else {
                     TCGv_i32 tmp = load_reg(s, rt);
-                    store_cpu_offset(tmp, ri->fieldoffset);
+                    store_cpu_offset(tmp, ri->fieldoffset, true);
                 }
             }
         }