Re: [PATCH v7 30/42] target/arm: Use mte_check1 for sve LD1R

[Richard Henderson]

On 6/19/20 7:03 AM, Peter Maydell wrote:
>>  static bool trans_LD1R_zpri(DisasContext *s, arg_rpri_load *a)
>>  {
>> -if (!sve_access_check(s)) {
>> -return true;
>> -}
>> -
>>  unsigned vsz = vec_full_reg_size(s);
>>  unsigned psz = pred_full_reg_size(s);
>>  unsigned esz = dtype_esz[a->dtype];
>>  unsigned msz = dtype_msz(a->dtype);
>>  TCGLabel *over = gen_new_label();
>> -TCGv_i64 temp;
>> +TCGv_i64 temp, clean_addr;
>> +
>> +if (!sve_access_check(s)) {
>> +return true;
>> +}
> 
> This cleanup should really be its own patch.
> 
> We should probably not call gen_new_label() until after
> the sve_access_check() ?


Done.


r~

Re: [PATCH v7 30/42] target/arm: Use mte_check1 for sve LD1R

[Peter Maydell]

On Wed, 3 Jun 2020 at 02:13, Richard Henderson
 wrote:
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate-sve.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
> index e515646db2..4b3b879815 100644
> --- a/target/arm/translate-sve.c
> +++ b/target/arm/translate-sve.c
> @@ -4830,16 +4830,16 @@ static bool trans_LD1RQ_zpri(DisasContext *s, 
> arg_rpri_load *a)
>  /* Load and broadcast element.  */
>  static bool trans_LD1R_zpri(DisasContext *s, arg_rpri_load *a)
>  {
> -if (!sve_access_check(s)) {
> -return true;
> -}
> -
>  unsigned vsz = vec_full_reg_size(s);
>  unsigned psz = pred_full_reg_size(s);
>  unsigned esz = dtype_esz[a->dtype];
>  unsigned msz = dtype_msz(a->dtype);
>  TCGLabel *over = gen_new_label();
> -TCGv_i64 temp;
> +TCGv_i64 temp, clean_addr;
> +
> +if (!sve_access_check(s)) {
> +return true;
> +}

This cleanup should really be its own patch.

We should probably not call gen_new_label() until after
the sve_access_check() ?

>
>  /* If the guarding predicate has no bits set, no load occurs.  */
>  if (psz <= 8) {
> @@ -4862,7 +4862,9 @@ static bool trans_LD1R_zpri(DisasContext *s, 
> arg_rpri_load *a)
>  /* Load the data.  */
>  temp = tcg_temp_new_i64();
>  tcg_gen_addi_i64(temp, cpu_reg_sp(s, a->rn), a->imm << msz);
> -tcg_gen_qemu_ld_i64(temp, temp, get_mem_index(s),
> +clean_addr = gen_mte_check1(s, temp, false, true, msz);
> +
> +tcg_gen_qemu_ld_i64(temp, clean_addr, get_mem_index(s),
>  s->be_data | dtype_mop[a->dtype]);
>
>  /* Broadcast to *all* elements.  */
> --

This patch of the change looks good.

thanks
-- PMM