RE: [PATCH v2 3/3] target/hexagon: avoid shadowing globals

[<ltaylorsimpson@gmail.com>]

> -----Original Message-----
> From: Brian Cain <bcain@quicinc.com>
> Sent: Thursday, October 5, 2023 4:22 PM
> To: qemu-devel@nongnu.org
> Cc: bcain@quicinc.com; armbru@redhat.com; richard.henderson@linaro.org;
> philmd@linaro.org; peter.maydell@linaro.org; quic_mathbern@quicinc.com;
> stefanha@redhat.com; ale@rev.ng; anjo@rev.ng;
> quic_mliebel@quicinc.com; ltaylorsimpson@gmail.com
> Subject: [PATCH v2 3/3] target/hexagon: avoid shadowing globals
> 
> The typedef `vaddr` is shadowed by `vaddr` identifiers, so we rename the
> identifiers to avoid shadowing the type name.
> 
> The global `cpu_env` is shadowed by local `cpu_env` arguments, so we
> rename the function arguments to avoid shadowing the global.
> 
> Signed-off-by: Brian Cain <bcain@quicinc.com>
> ---
>  target/hexagon/genptr.c                 | 56 ++++++++++++-------------
>  target/hexagon/genptr.h                 | 18 ++++----
>  target/hexagon/mmvec/system_ext_mmvec.c |  4 +-
> target/hexagon/mmvec/system_ext_mmvec.h |  2 +-
>  target/hexagon/op_helper.c              |  4 +-
>  5 files changed, 42 insertions(+), 42 deletions(-)
> 
> diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c index
> 217bc7bb5a..11377ac92b 100644
> --- a/target/hexagon/genptr.c
> +++ b/target/hexagon/genptr.c
> @@ -334,28 +334,28 @@ void gen_set_byte_i64(int N, TCGv_i64 result, TCGv
> src)
>      tcg_gen_deposit_i64(result, result, src64, N * 8, 8);  }
> 
> -static inline void gen_load_locked4u(TCGv dest, TCGv vaddr, int
> mem_index)
> +static inline void gen_load_locked4u(TCGv dest, TCGv v_addr, int
> +mem_index)

I'd recommend moving both the type and the arg name to the new line, also indent the new line.
static inline void gen_load_locked4u(TCGv dest, TCGv v_addr,
                                                                  int mem_index)


> 
> -static inline void gen_load_locked8u(TCGv_i64 dest, TCGv vaddr, int
> mem_index)
> +static inline void gen_load_locked8u(TCGv_i64 dest, TCGv v_addr, int
> +mem_index)

Ditto

>  static inline void gen_store_conditional4(DisasContext *ctx,
> -                                          TCGv pred, TCGv vaddr, TCGv src)
> +                                          TCGv pred, TCGv v_addr, TCGv
> + src)

Ditto

>      zero = tcg_constant_tl(0);
> @@ -374,13 +374,13 @@ static inline void
> gen_store_conditional4(DisasContext *ctx,  }
> 
>  static inline void gen_store_conditional8(DisasContext *ctx,
> -                                          TCGv pred, TCGv vaddr, TCGv_i64 src)
> +                                          TCGv pred, TCGv v_addr,
> + TCGv_i64 src)

Indent

> -void mem_gather_store(CPUHexagonState *env, target_ulong vaddr, int
> slot)
> +void mem_gather_store(CPUHexagonState *env, target_ulong v_addr, int
> +slot)

Ditto

> -void mem_gather_store(CPUHexagonState *env, target_ulong vaddr, int
> slot);
> +void mem_gather_store(CPUHexagonState *env, target_ulong v_addr, int
> +slot);

Ditto


Otherwise,
Reviewed-by: Taylor Simpson <ltaylorsimpson@gmail.com>