[PATCH 04/11] target/arm: Split out gen_wrap2_i32 helper

Richard Henderson posted 11 patches 3 months, 2 weeks ago
Maintainers: Laurent Vivier <laurent@vivier.eu>, Peter Maydell <peter.maydell@linaro.org>
[PATCH 04/11] target/arm: Split out gen_wrap2_i32 helper
Posted by Richard Henderson 3 months, 2 weeks ago
Wrapper to extract the low 32 bits, perform an operation,
and zero-extend back to 64 bits.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/tcg/translate-a64.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index bb92bdc296..64a845d5fb 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -8231,13 +8231,18 @@ static bool gen_rr(DisasContext *s, int rd, int rn, ArithOneOp fn)
     return true;
 }
 
+static void gen_wrap2_i32(TCGv_i64 d, TCGv_i64 n, NeonGenOneOpFn fn)
+{
+    TCGv_i32 t = tcg_temp_new_i32();
+
+    tcg_gen_extrl_i64_i32(t, n);
+    fn(t, t);
+    tcg_gen_extu_i32_i64(d, t);
+}
+
 static void gen_rbit32(TCGv_i64 tcg_rd, TCGv_i64 tcg_rn)
 {
-    TCGv_i32 t32 = tcg_temp_new_i32();
-
-    tcg_gen_extrl_i64_i32(t32, tcg_rn);
-    gen_helper_rbit(t32, t32);
-    tcg_gen_extu_i32_i64(tcg_rd, t32);
+    gen_wrap2_i32(tcg_rn, tcg_rn, gen_helper_rbit);
 }
 
 static void gen_rev16_xx(TCGv_i64 tcg_rd, TCGv_i64 tcg_rn, TCGv_i64 mask)
@@ -8293,11 +8298,7 @@ static void gen_clz64(TCGv_i64 tcg_rd, TCGv_i64 tcg_rn)
 
 static void gen_cls32(TCGv_i64 tcg_rd, TCGv_i64 tcg_rn)
 {
-    TCGv_i32 t32 = tcg_temp_new_i32();
-
-    tcg_gen_extrl_i64_i32(t32, tcg_rn);
-    tcg_gen_clrsb_i32(t32, t32);
-    tcg_gen_extu_i32_i64(tcg_rd, t32);
+    gen_wrap2_i32(tcg_rn, tcg_rn, tcg_gen_clrsb_i32);
 }
 
 TRANS(CLZ, gen_rr, a->rd, a->rn, a->sf ? gen_clz64 : gen_clz32)
-- 
2.43.0
Re: [PATCH 04/11] target/arm: Split out gen_wrap2_i32 helper
Posted by Peter Maydell 3 months ago
On Sun, 3 Aug 2025 at 02:42, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Wrapper to extract the low 32 bits, perform an operation,
> and zero-extend back to 64 bits.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/tcg/translate-a64.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
> index bb92bdc296..64a845d5fb 100644
> --- a/target/arm/tcg/translate-a64.c
> +++ b/target/arm/tcg/translate-a64.c
> @@ -8231,13 +8231,18 @@ static bool gen_rr(DisasContext *s, int rd, int rn, ArithOneOp fn)
>      return true;
>  }
>
> +static void gen_wrap2_i32(TCGv_i64 d, TCGv_i64 n, NeonGenOneOpFn fn)
> +{
> +    TCGv_i32 t = tcg_temp_new_i32();
> +
> +    tcg_gen_extrl_i64_i32(t, n);
> +    fn(t, t);
> +    tcg_gen_extu_i32_i64(d, t);
> +}
> +
>  static void gen_rbit32(TCGv_i64 tcg_rd, TCGv_i64 tcg_rn)
>  {
> -    TCGv_i32 t32 = tcg_temp_new_i32();
> -
> -    tcg_gen_extrl_i64_i32(t32, tcg_rn);
> -    gen_helper_rbit(t32, t32);
> -    tcg_gen_extu_i32_i64(tcg_rd, t32);
> +    gen_wrap2_i32(tcg_rn, tcg_rn, gen_helper_rbit);

...should be (tcg_rd, tcg_rn, gen_helper_rbit);


>  }
>
>  static void gen_rev16_xx(TCGv_i64 tcg_rd, TCGv_i64 tcg_rn, TCGv_i64 mask)
> @@ -8293,11 +8298,7 @@ static void gen_clz64(TCGv_i64 tcg_rd, TCGv_i64 tcg_rn)
>
>  static void gen_cls32(TCGv_i64 tcg_rd, TCGv_i64 tcg_rn)
>  {
> -    TCGv_i32 t32 = tcg_temp_new_i32();
> -
> -    tcg_gen_extrl_i64_i32(t32, tcg_rn);
> -    tcg_gen_clrsb_i32(t32, t32);
> -    tcg_gen_extu_i32_i64(tcg_rd, t32);
> +    gen_wrap2_i32(tcg_rn, tcg_rn, tcg_gen_clrsb_i32);

Ditto.

This caused the 'check-functional' tests to fail.

-- PMM
Re: [PATCH 04/11] target/arm: Split out gen_wrap2_i32 helper
Posted by Peter Maydell 3 months ago
On Sun, 3 Aug 2025 at 02:42, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Wrapper to extract the low 32 bits, perform an operation,
> and zero-extend back to 64 bits.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/tcg/translate-a64.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
> index bb92bdc296..64a845d5fb 100644
> --- a/target/arm/tcg/translate-a64.c
> +++ b/target/arm/tcg/translate-a64.c
> @@ -8231,13 +8231,18 @@ static bool gen_rr(DisasContext *s, int rd, int rn, ArithOneOp fn)
>      return true;
>  }
>

A brief comment here would help:

  /*
   * Perform 32-bit operation fn on the low half of n;
   * the high half of the output is zeroed.
   */
> +static void gen_wrap2_i32(TCGv_i64 d, TCGv_i64 n, NeonGenOneOpFn fn)
> +{
> +    TCGv_i32 t = tcg_temp_new_i32();
> +
> +    tcg_gen_extrl_i64_i32(t, n);
> +    fn(t, t);
> +    tcg_gen_extu_i32_i64(d, t);
> +}
> +

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM