|
From: | Daniel Henrique Barboza |
Subject: | Re: [PATCH 11/13] trans_rvv.c.inc: remove vlmax arg from vec_element_loadx() |
Date: | Mon, 15 Jan 2024 14:57:06 -0300 |
User-agent: | Mozilla Thunderbird |
On 1/12/24 19:51, Richard Henderson wrote:
On 1/13/24 08:38, Daniel Henrique Barboza wrote:Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/insn_trans/trans_rvv.c.inc | 26 +++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc index 804cfd6c7f..3782d0fa2f 100644 --- a/target/riscv/insn_trans/trans_rvv.c.inc +++ b/target/riscv/insn_trans/trans_rvv.c.inc @@ -3265,21 +3265,28 @@ static void endian_adjust(TCGv_i32 ofs, int sew) #endif } -/* Load idx >= VLMAX ? 0 : vreg[idx] */ +/* + * Load idx >= VLMAX ? 0 : vreg[idx] + * + * This function assumes ctx->vl_eq_vlmax = true. + */ static void vec_element_loadx(DisasContext *s, TCGv_i64 dest, - int vreg, TCGv idx, int vlmax) + int vreg, TCGv idx)I think removing the cpu configuration constant is a mistake. Compile-time constants are always better than computation...
Apparently my commit msg is AWOL ... The 'vlmax' used in vec_element_loadx() is being calculated here, in trans_vrgather_vx(): - int scale = s->lmul - (s->sew + 3); - int vlmax = s->cfg_ptr->vlen >> -scale; My idea was to eliminate the use of 'vlen' since, in this block, 'vl_eq_vlmax' is true and we have 'vl' in the TCG global 'cpu_vl'. I didn't find a way of reading 'cpu_vl' into an int variable and passing it as 'vlmax' to vec_element_loadx(), but inside vec_element_loadx() we can operate 'cpu_vl' normally since we're using TCG ops there. This is the reasoning behind this change. I am now wondering if this is worth the trouble, and we should instead do: + int vlmax = cpu->cfg.vlenb >> (s->sew - s->lmul); Like we're already doing in patch 9. Patch 12 would be a similar case. Thanks, Daniel
+#ifdef TARGET_RISCV64 + tcg_gen_mov_i64(t_vlmax, cpu_vl); +#else + tcg_gen_extu_tl_i64(t_vlmax, cpu_vl); +#endifThat said, no ifdef required -- the second statement should always work. r~
[Prev in Thread] | Current Thread | [Next in Thread] |