Re: [PATCH v1 2/6] target/loongarch: Add set_vec_extctx to set LSX/LASX instructions extctx_flags

[Richard Henderson <richard.henderson@linaro.org>]

On 10/29/23 20:28, gaosong wrote:
> 在 2023/10/29 上午5:40, Richard Henderson 写道:
>> On 10/9/23 20:36, Song Gao wrote:
>>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>>> ---
>>>   target/loongarch/insn_trans/trans_vec.c.inc | 12 ++++++++++++
>>>   target/loongarch/internals.h                |  2 ++
>>>   2 files changed, 14 insertions(+)
>>>
>>> diff --git a/target/loongarch/insn_trans/trans_vec.c.inc 
>>> b/target/loongarch/insn_trans/trans_vec.c.inc
>>> index 98f856bb29..aef16ef44a 100644
>>> --- a/target/loongarch/insn_trans/trans_vec.c.inc
>>> +++ b/target/loongarch/insn_trans/trans_vec.c.inc
>>> @@ -23,8 +23,20 @@ static bool check_vec(DisasContext *ctx, uint32_t oprsz)
>>>     #else
>>>   +static void set_vec_extctx(DisasContext *ctx, uint32_t oprsz)
>>> +{
>>> +    if (oprsz == 16) {
>>> +        ctx->extctx_flags |= EXTCTX_FLAGS_LSX;
>>> +    }
>>> +
>>> +    if (oprsz == 32) {
>>> +        ctx->extctx_flags |= EXTCTX_FLAGS_LASX;
>>> +    }
>>> +}
>>> +
>>>   static bool check_vec(DisasContext *ctx, uint32_t oprsz)
>>>   {
>>> +    set_vec_extctx(ctx, oprsz);
>>>       return true;
>>>   }
>>
>> This doesn't do anything.  Nothing copies the changed value back to env.
>> Anyway, I think this is the wrong way to go about it.
>>
> Oh, It is on patch1.
> 
> @@ -294,6 +296,7 @@ static void loongarch_tr_translate_insn(DisasContextBase *dcbase, 
> CPUState *cs)
>           generate_exception(ctx, EXCCODE_INE);
>       }
> 
> +    env->extctx_flags |= ctx->extctx_flags;

Ah, well, this is also incorrect.

This copy only happens at translation time, not at execution time.

Anyway, I think my previous suggestion is better:

>> If you want to track what the program is using, you should do it exactly like the real 
>> kernel: disable the execution unit, have the program trap, and the enable the execution 
>> unit when the trap occurs. At this point, CSR_EUEN enable bits contain exactly which 
>> units have been used by the program.


r~