* [PATCH v2 0/4] target/arm: Use tcg_constant_*
@ 2021-10-27 4:56 Philippe Mathieu-Daudé
2021-10-27 4:56 ` [PATCH v2 1/4] target/arm: Use tcg_constant_i32() in op_smlad() Philippe Mathieu-Daudé
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-27 4:56 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-arm, Richard Henderson, Philippe Mathieu-Daudé
Missing review: patch #2
Introduce store_cpu_field_constant() helper to avoid using temporary
when the value is constant (and read-only).
Since v1:
- Really use tcg_constant() in patch 2 (Richard)
Philippe Mathieu-Daudé (4):
target/arm: Use tcg_constant_i32() in op_smlad()
target/arm: Introduce store_cpu_field_constant() helper
target/arm: Use the constant variant of store_cpu_field() when
possible
target/arm: Use tcg_constant_i64() in do_sat_addsub_64()
target/arm/translate-a32.h | 11 ++++++++---
target/arm/translate-sve.c | 17 ++++++++---------
target/arm/translate.c | 26 ++++++++------------------
3 files changed, 24 insertions(+), 30 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/4] target/arm: Use tcg_constant_i32() in op_smlad()
2021-10-27 4:56 [PATCH v2 0/4] target/arm: Use tcg_constant_* Philippe Mathieu-Daudé
@ 2021-10-27 4:56 ` Philippe Mathieu-Daudé
2021-10-27 4:56 ` [PATCH v2 2/4] target/arm: Introduce store_cpu_field_constant() helper Philippe Mathieu-Daudé
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-27 4:56 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-arm, Richard Henderson, Philippe Mathieu-Daudé
Avoid using a TCG temporary for a read-only constant.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
target/arm/translate.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index d6af5b1b039..083a6d6ed77 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -7849,10 +7849,9 @@ static bool op_smlad(DisasContext *s, arg_rrrr *a, bool m_swap, bool sub)
t3 = tcg_temp_new_i32();
tcg_gen_sari_i32(t3, t1, 31);
qf = load_cpu_field(QF);
- one = tcg_const_i32(1);
+ one = tcg_constant_i32(1);
tcg_gen_movcond_i32(TCG_COND_NE, qf, t2, t3, one, qf);
store_cpu_field(qf, QF);
- tcg_temp_free_i32(one);
tcg_temp_free_i32(t3);
tcg_temp_free_i32(t2);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/4] target/arm: Introduce store_cpu_field_constant() helper
2021-10-27 4:56 [PATCH v2 0/4] target/arm: Use tcg_constant_* Philippe Mathieu-Daudé
2021-10-27 4:56 ` [PATCH v2 1/4] target/arm: Use tcg_constant_i32() in op_smlad() Philippe Mathieu-Daudé
@ 2021-10-27 4:56 ` Philippe Mathieu-Daudé
2021-10-28 1:17 ` Richard Henderson
2021-10-27 4:56 ` [PATCH v2 3/4] target/arm: Use the constant variant of store_cpu_field() when possible Philippe Mathieu-Daudé
2021-10-27 4:56 ` [PATCH v2 4/4] target/arm: Use tcg_constant_i64() in do_sat_addsub_64() Philippe Mathieu-Daudé
3 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-27 4:56 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-arm, Richard Henderson, Philippe Mathieu-Daudé
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(-)
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);
}
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/4] target/arm: Use the constant variant of store_cpu_field() when possible
2021-10-27 4:56 [PATCH v2 0/4] target/arm: Use tcg_constant_* Philippe Mathieu-Daudé
2021-10-27 4:56 ` [PATCH v2 1/4] target/arm: Use tcg_constant_i32() in op_smlad() Philippe Mathieu-Daudé
2021-10-27 4:56 ` [PATCH v2 2/4] target/arm: Introduce store_cpu_field_constant() helper Philippe Mathieu-Daudé
@ 2021-10-27 4:56 ` Philippe Mathieu-Daudé
2021-10-27 4:56 ` [PATCH v2 4/4] target/arm: Use tcg_constant_i64() in do_sat_addsub_64() Philippe Mathieu-Daudé
3 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-27 4:56 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-arm, Richard Henderson, Philippe Mathieu-Daudé
When using a constant variable, we can replace the store_cpu_field()
call by store_cpu_field_constant() which avoid using TCG temporaries.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
target/arm/translate.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 5061e55f2c0..484b0eed0cb 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -364,8 +364,7 @@ void clear_eci_state(DisasContext *s)
* multiple insn executes.
*/
if (s->eci) {
- TCGv_i32 tmp = tcg_const_i32(0);
- store_cpu_field(tmp, condexec_bits);
+ store_cpu_field_constant(0, condexec_bits);
s->eci = 0;
}
}
@@ -740,9 +739,8 @@ void gen_set_condexec(DisasContext *s)
{
if (s->condexec_mask) {
uint32_t val = (s->condexec_cond << 4) | (s->condexec_mask >> 1);
- TCGv_i32 tmp = tcg_temp_new_i32();
- tcg_gen_movi_i32(tmp, val);
- store_cpu_field(tmp, condexec_bits);
+
+ store_cpu_field_constant(val, condexec_bits);
}
}
@@ -8362,8 +8360,6 @@ static bool trans_BL(DisasContext *s, arg_i *a)
static bool trans_BLX_i(DisasContext *s, arg_BLX_i *a)
{
- TCGv_i32 tmp;
-
/*
* BLX <imm> would be useless on M-profile; the encoding space
* is used for other insns from v8.1M onward, and UNDEFs before that.
@@ -8377,8 +8373,7 @@ static bool trans_BLX_i(DisasContext *s, arg_BLX_i *a)
return false;
}
tcg_gen_movi_i32(cpu_R[14], s->base.pc_next | s->thumb);
- tmp = tcg_const_i32(!s->thumb);
- store_cpu_field(tmp, thumb);
+ store_cpu_field_constant(!s->thumb, thumb);
gen_jmp(s, (read_pc(s) & ~3) + a->imm);
return true;
}
@@ -8677,7 +8672,6 @@ static bool trans_LCTP(DisasContext *s, arg_LCTP *a)
* doesn't cache branch information, all we need to do is reset
* FPSCR.LTPSIZE to 4.
*/
- TCGv_i32 ltpsize;
if (!dc_isar_feature(aa32_lob, s) ||
!dc_isar_feature(aa32_mve, s)) {
@@ -8688,8 +8682,7 @@ static bool trans_LCTP(DisasContext *s, arg_LCTP *a)
return true;
}
- ltpsize = tcg_const_i32(4);
- store_cpu_field(ltpsize, v7m.ltpsize);
+ store_cpu_field_constant(4, v7m.ltpsize);
return true;
}
@@ -9487,9 +9480,7 @@ static void arm_tr_tb_start(DisasContextBase *dcbase, CPUState *cpu)
/* Reset the conditional execution bits immediately. This avoids
complications trying to do it at the end of the block. */
if (dc->condexec_mask || dc->condexec_cond) {
- TCGv_i32 tmp = tcg_temp_new_i32();
- tcg_gen_movi_i32(tmp, 0);
- store_cpu_field(tmp, condexec_bits);
+ store_cpu_field_constant(0, condexec_bits);
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 4/4] target/arm: Use tcg_constant_i64() in do_sat_addsub_64()
2021-10-27 4:56 [PATCH v2 0/4] target/arm: Use tcg_constant_* Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-10-27 4:56 ` [PATCH v2 3/4] target/arm: Use the constant variant of store_cpu_field() when possible Philippe Mathieu-Daudé
@ 2021-10-27 4:56 ` Philippe Mathieu-Daudé
3 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-27 4:56 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-arm, Richard Henderson, Philippe Mathieu-Daudé
The immediate value used for comparison is constant and
read-only. Move it to the constant pool. This frees a
TCG temporary for unsigned saturation opcodes.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
target/arm/translate-sve.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index bc91a641711..76b5fe9f313 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -1943,20 +1943,20 @@ static void do_sat_addsub_32(TCGv_i64 reg, TCGv_i64 val, bool u, bool d)
static void do_sat_addsub_64(TCGv_i64 reg, TCGv_i64 val, bool u, bool d)
{
TCGv_i64 t0 = tcg_temp_new_i64();
- TCGv_i64 t1 = tcg_temp_new_i64();
TCGv_i64 t2;
if (u) {
if (d) {
tcg_gen_sub_i64(t0, reg, val);
- tcg_gen_movi_i64(t1, 0);
- tcg_gen_movcond_i64(TCG_COND_LTU, reg, reg, val, t1, t0);
+ t2 = tcg_constant_i64(0);
+ tcg_gen_movcond_i64(TCG_COND_LTU, reg, reg, val, t2, t0);
} else {
tcg_gen_add_i64(t0, reg, val);
- tcg_gen_movi_i64(t1, -1);
- tcg_gen_movcond_i64(TCG_COND_LTU, reg, t0, reg, t1, t0);
+ t2 = tcg_constant_i64(-1);
+ tcg_gen_movcond_i64(TCG_COND_LTU, reg, t0, reg, t2, t0);
}
} else {
+ TCGv_i64 t1 = tcg_temp_new_i64();
if (d) {
/* Detect signed overflow for subtraction. */
tcg_gen_xor_i64(t0, reg, val);
@@ -1966,7 +1966,7 @@ static void do_sat_addsub_64(TCGv_i64 reg, TCGv_i64 val, bool u, bool d)
/* Bound the result. */
tcg_gen_movi_i64(reg, INT64_MIN);
- t2 = tcg_const_i64(0);
+ t2 = tcg_constant_i64(0);
tcg_gen_movcond_i64(TCG_COND_LT, reg, t0, t2, reg, t1);
} else {
/* Detect signed overflow for addition. */
@@ -1977,13 +1977,12 @@ static void do_sat_addsub_64(TCGv_i64 reg, TCGv_i64 val, bool u, bool d)
/* Bound the result. */
tcg_gen_movi_i64(t1, INT64_MAX);
- t2 = tcg_const_i64(0);
+ t2 = tcg_constant_i64(0);
tcg_gen_movcond_i64(TCG_COND_LT, reg, t0, t2, t1, reg);
}
- tcg_temp_free_i64(t2);
+ tcg_temp_free_i64(t1);
}
tcg_temp_free_i64(t0);
- tcg_temp_free_i64(t1);
}
/* Similarly with a vector and a scalar operand. */
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/4] target/arm: Introduce store_cpu_field_constant() helper
2021-10-27 4:56 ` [PATCH v2 2/4] target/arm: Introduce store_cpu_field_constant() helper Philippe Mathieu-Daudé
@ 2021-10-28 1:17 ` Richard Henderson
2021-10-29 23:07 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2021-10-28 1:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Peter Maydell, qemu-arm
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~
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/4] target/arm: Introduce store_cpu_field_constant() helper
2021-10-28 1:17 ` Richard Henderson
@ 2021-10-29 23:07 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-29 23:07 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: Peter Maydell, qemu-arm
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 :)
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-29 23:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-27 4:56 [PATCH v2 0/4] target/arm: Use tcg_constant_* Philippe Mathieu-Daudé
2021-10-27 4:56 ` [PATCH v2 1/4] target/arm: Use tcg_constant_i32() in op_smlad() Philippe Mathieu-Daudé
2021-10-27 4:56 ` [PATCH v2 2/4] target/arm: Introduce store_cpu_field_constant() helper Philippe Mathieu-Daudé
2021-10-28 1:17 ` Richard Henderson
2021-10-29 23:07 ` Philippe Mathieu-Daudé
2021-10-27 4:56 ` [PATCH v2 3/4] target/arm: Use the constant variant of store_cpu_field() when possible Philippe Mathieu-Daudé
2021-10-27 4:56 ` [PATCH v2 4/4] target/arm: Use tcg_constant_i64() in do_sat_addsub_64() Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).