RE: [PATCH v6 05/12] target/hexagon: introduce new helper functions

[Taylor Simpson]

> -----Original Message-----
> From: Alessandro Di Federico <ale.qemu@rev.ng>
> Sent: Tuesday, July 20, 2021 7:30 AM
> To: qemu-devel@nongnu.org
> Cc: Taylor Simpson <tsimpson@quicinc.com>; Brian Cain
> <bcain@quicinc.com>; babush@rev.ng; nizzo@rev.ng;
> richard.henderson@linaro.org; Alessandro Di Federico <ale@rev.ng>
> Subject: [PATCH v6 05/12] target/hexagon: introduce new helper functions
> 
> From: Niccolò Izzo <nizzo@rev.ng>
> 
> These helpers will be employed by the idef-parser generated code.
> 
> Signed-off-by: Alessandro Di Federico <ale@rev.ng>
> Signed-off-by: Niccolò Izzo <nizzo@rev.ng>
> diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c index
> 5e7f446657..b7eca94de7 100644
> --- a/target/hexagon/genptr.c
> +++ b/target/hexagon/genptr.c
> -void gen_store32(TCGv vaddr, TCGv src, int width, uint32_t slot)
> +void gen_store32(DisasContext *ctx, TCGv vaddr, TCGv src, tcg_target_long
> width,
> +                 uint32_t slot)
>  {
>      tcg_gen_mov_tl(hex_store_addr[slot], vaddr);
>      tcg_gen_movi_tl(hex_store_width[slot], width);
>      tcg_gen_mov_tl(hex_store_val32[slot], src);
> +    ctx->store_width[slot] = width;

This isn't strictly a new helper function.  You are moving the update to the 
DisasContext here instead of doing it in the functions that call this one.

>  }
> 
>  void gen_store1(TCGv_env cpu_env, TCGv vaddr, TCGv src, DisasContext
> *ctx,
>                  uint32_t slot)
>  {
> -    gen_store32(vaddr, src, 1, slot);
> -    ctx->store_width[slot] = 1;

Here is where you the code above was previously.  I'm not sure if this is 
better or worse, but it's not a new function.


Otherwise
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>