[v6] Improve the performance of RISC-V vector unit-stride/whole register ld/st instructions

[PATCH v6 1/7] target/riscv: Set vdata.vm field for vector load/store whole register instructions

Posted by Max Chou 1 year, 4 months ago

The vm field of the vector load/store whole register instruction's
encoding is 1.
The helper function of the vector load/store whole register instructions
may need the vdata.vm field to do some optimizations.

Signed-off-by: Max Chou <max.chou@sifive.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 3a3896ba06c..14e10568bd7 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -770,6 +770,7 @@ static bool ld_us_mask_op(DisasContext *s, arg_vlm_v *a, uint8_t eew)
     /* Mask destination register are always tail-agnostic */
     data = FIELD_DP32(data, VDATA, VTA, s->cfg_vta_all_1s);
     data = FIELD_DP32(data, VDATA, VMA, s->vma);
+    data = FIELD_DP32(data, VDATA, VM, 1);
     return ldst_us_trans(a->rd, a->rs1, data, fn, s, false);
 }
 
@@ -787,6 +788,7 @@ static bool st_us_mask_op(DisasContext *s, arg_vsm_v *a, uint8_t eew)
     /* EMUL = 1, NFIELDS = 1 */
     data = FIELD_DP32(data, VDATA, LMUL, 0);
     data = FIELD_DP32(data, VDATA, NF, 1);
+    data = FIELD_DP32(data, VDATA, VM, 1);
     return ldst_us_trans(a->rd, a->rs1, data, fn, s, true);
 }
 
@@ -1106,6 +1108,7 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, uint32_t nf,
     TCGv_i32 desc;
 
     uint32_t data = FIELD_DP32(0, VDATA, NF, nf);
+    data = FIELD_DP32(data, VDATA, VM, 1);
     dest = tcg_temp_new_ptr();
     desc = tcg_constant_i32(simd_desc(s->cfg_ptr->vlenb,
                                       s->cfg_ptr->vlenb, data));
-- 
2.34.1

Re: [PATCH v6 1/7] target/riscv: Set vdata.vm field for vector load/store whole register instructions

Posted by Daniel Henrique Barboza 1 year, 3 months ago


On 9/18/24 2:14 PM, Max Chou wrote:
> The vm field of the vector load/store whole register instruction's
> encoding is 1.
> The helper function of the vector load/store whole register instructions
> may need the vdata.vm field to do some optimizations.
> 
> Signed-off-by: Max Chou <max.chou@sifive.com>
> ---

I wonder if we should always encode 'vm' in vdata for all insns. Seems like
helpers are passing 'vm' around in the helpers ...


Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   target/riscv/insn_trans/trans_rvv.c.inc | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index 3a3896ba06c..14e10568bd7 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -770,6 +770,7 @@ static bool ld_us_mask_op(DisasContext *s, arg_vlm_v *a, uint8_t eew)
>       /* Mask destination register are always tail-agnostic */
>       data = FIELD_DP32(data, VDATA, VTA, s->cfg_vta_all_1s);
>       data = FIELD_DP32(data, VDATA, VMA, s->vma);
> +    data = FIELD_DP32(data, VDATA, VM, 1);
>       return ldst_us_trans(a->rd, a->rs1, data, fn, s, false);
>   }
>   
> @@ -787,6 +788,7 @@ static bool st_us_mask_op(DisasContext *s, arg_vsm_v *a, uint8_t eew)
>       /* EMUL = 1, NFIELDS = 1 */
>       data = FIELD_DP32(data, VDATA, LMUL, 0);
>       data = FIELD_DP32(data, VDATA, NF, 1);
> +    data = FIELD_DP32(data, VDATA, VM, 1);
>       return ldst_us_trans(a->rd, a->rs1, data, fn, s, true);
>   }
>   
> @@ -1106,6 +1108,7 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, uint32_t nf,
>       TCGv_i32 desc;
>   
>       uint32_t data = FIELD_DP32(0, VDATA, NF, nf);
> +    data = FIELD_DP32(data, VDATA, VM, 1);
>       dest = tcg_temp_new_ptr();
>       desc = tcg_constant_i32(simd_desc(s->cfg_ptr->vlenb,
>                                         s->cfg_ptr->vlenb, data));

Re: [PATCH v6 1/7] target/riscv: Set vdata.vm field for vector load/store whole register instructions

Posted by Richard Henderson 1 year, 3 months ago

On 10/29/24 18:58, Daniel Henrique Barboza wrote:
> 
> 
> On 9/18/24 2:14 PM, Max Chou wrote:
>> The vm field of the vector load/store whole register instruction's
>> encoding is 1.
>> The helper function of the vector load/store whole register instructions
>> may need the vdata.vm field to do some optimizations.
>>
>> Signed-off-by: Max Chou <max.chou@sifive.com>
>> ---
> 
> I wonder if we should always encode 'vm' in vdata for all insns. Seems like
> helpers are passing 'vm' around in the helpers ...

The intention there is that 'vm' is a constant within those helpers, so the compiler can 
fold away the code blocks.


r~