[PATCH 33/43] target/ppc: Remove single use static inline function

BALATON Zoltan posted 43 patches 6 months ago
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>
[PATCH 33/43] target/ppc: Remove single use static inline function
Posted by BALATON Zoltan 6 months ago
The ger_pack_masks() function is only used once and the inverse of
this operation is already inlined so it can be inlined too in the only
caller and removed from the header.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 target/ppc/internal.h               | 9 ---------
 target/ppc/translate/vsx-impl.c.inc | 6 ++++--
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index 20fb2ec593..8e5a241f74 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -293,13 +293,4 @@ FIELD(GER_MSK, XMSK, 0, 4)
 FIELD(GER_MSK, YMSK, 4, 4)
 FIELD(GER_MSK, PMSK, 8, 8)
 
-static inline int ger_pack_masks(int pmsk, int ymsk, int xmsk)
-{
-    int msk = 0;
-    msk = FIELD_DP32(msk, GER_MSK, XMSK, xmsk);
-    msk = FIELD_DP32(msk, GER_MSK, YMSK, ymsk);
-    msk = FIELD_DP32(msk, GER_MSK, PMSK, pmsk);
-    return msk;
-}
-
 #endif /* PPC_INTERNAL_H */
diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
index 0266f09119..62950d348a 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -2819,7 +2819,7 @@ static bool trans_XXSETACCZ(DisasContext *ctx, arg_X_a *a)
 static bool do_ger(DisasContext *ctx, arg_MMIRR_XX3 *a,
     void (*helper)(TCGv_env, TCGv_ptr, TCGv_ptr, TCGv_ptr, TCGv_i32))
 {
-    uint32_t mask;
+    uint32_t mask = 0;
     TCGv_ptr xt, xa, xb;
     REQUIRE_INSNS_FLAGS2(ctx, ISA310);
     REQUIRE_VSX(ctx);
@@ -2832,7 +2832,9 @@ static bool do_ger(DisasContext *ctx, arg_MMIRR_XX3 *a,
     xa = gen_vsr_ptr(a->xa);
     xb = gen_vsr_ptr(a->xb);
 
-    mask = ger_pack_masks(a->pmsk, a->ymsk, a->xmsk);
+    mask = FIELD_DP32(mask, GER_MSK, XMSK, a->xmsk);
+    mask = FIELD_DP32(mask, GER_MSK, YMSK, a->ymsk);
+    mask = FIELD_DP32(mask, GER_MSK, PMSK, a->pmsk);
     helper(tcg_env, xa, xb, xt, tcg_constant_i32(mask));
     return true;
 }
-- 
2.30.9
Re: [PATCH 33/43] target/ppc: Remove single use static inline function
Posted by Nicholas Piggin 4 months, 3 weeks ago
On Mon May 27, 2024 at 9:13 AM AEST, BALATON Zoltan wrote:
> The ger_pack_masks() function is only used once and the inverse of
> this operation is already inlined so it can be inlined too in the only
> caller and removed from the header.

Is this needed for later patches? I might prefer to keep it, even
move it into vsx-impl.c.inc and pull its inverse out into its own
function too even.

Thanks,
Nick

>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  target/ppc/internal.h               | 9 ---------
>  target/ppc/translate/vsx-impl.c.inc | 6 ++++--
>  2 files changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
> index 20fb2ec593..8e5a241f74 100644
> --- a/target/ppc/internal.h
> +++ b/target/ppc/internal.h
> @@ -293,13 +293,4 @@ FIELD(GER_MSK, XMSK, 0, 4)
>  FIELD(GER_MSK, YMSK, 4, 4)
>  FIELD(GER_MSK, PMSK, 8, 8)
>  
> -static inline int ger_pack_masks(int pmsk, int ymsk, int xmsk)
> -{
> -    int msk = 0;
> -    msk = FIELD_DP32(msk, GER_MSK, XMSK, xmsk);
> -    msk = FIELD_DP32(msk, GER_MSK, YMSK, ymsk);
> -    msk = FIELD_DP32(msk, GER_MSK, PMSK, pmsk);
> -    return msk;
> -}
> -
>  #endif /* PPC_INTERNAL_H */
> diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
> index 0266f09119..62950d348a 100644
> --- a/target/ppc/translate/vsx-impl.c.inc
> +++ b/target/ppc/translate/vsx-impl.c.inc
> @@ -2819,7 +2819,7 @@ static bool trans_XXSETACCZ(DisasContext *ctx, arg_X_a *a)
>  static bool do_ger(DisasContext *ctx, arg_MMIRR_XX3 *a,
>      void (*helper)(TCGv_env, TCGv_ptr, TCGv_ptr, TCGv_ptr, TCGv_i32))
>  {
> -    uint32_t mask;
> +    uint32_t mask = 0;
>      TCGv_ptr xt, xa, xb;
>      REQUIRE_INSNS_FLAGS2(ctx, ISA310);
>      REQUIRE_VSX(ctx);
> @@ -2832,7 +2832,9 @@ static bool do_ger(DisasContext *ctx, arg_MMIRR_XX3 *a,
>      xa = gen_vsr_ptr(a->xa);
>      xb = gen_vsr_ptr(a->xb);
>  
> -    mask = ger_pack_masks(a->pmsk, a->ymsk, a->xmsk);
> +    mask = FIELD_DP32(mask, GER_MSK, XMSK, a->xmsk);
> +    mask = FIELD_DP32(mask, GER_MSK, YMSK, a->ymsk);
> +    mask = FIELD_DP32(mask, GER_MSK, PMSK, a->pmsk);
>      helper(tcg_env, xa, xb, xt, tcg_constant_i32(mask));
>      return true;
>  }
Re: [PATCH 33/43] target/ppc: Remove single use static inline function
Posted by BALATON Zoltan 4 months, 3 weeks ago
On Thu, 4 Jul 2024, Nicholas Piggin wrote:
> On Mon May 27, 2024 at 9:13 AM AEST, BALATON Zoltan wrote:
>> The ger_pack_masks() function is only used once and the inverse of
>> this operation is already inlined so it can be inlined too in the only
>> caller and removed from the header.
>
> Is this needed for later patches? I might prefer to keep it, even

No, I think this patch can just be dropped then. I don't plan to make 
another version of it so just leave it out if you don't want to take it.

Regards,
BALATON Zoltan

> move it into vsx-impl.c.inc and pull its inverse out into its own
> function too even.
>
> Thanks,
> Nick
>
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  target/ppc/internal.h               | 9 ---------
>>  target/ppc/translate/vsx-impl.c.inc | 6 ++++--
>>  2 files changed, 4 insertions(+), 11 deletions(-)
>>
>> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
>> index 20fb2ec593..8e5a241f74 100644
>> --- a/target/ppc/internal.h
>> +++ b/target/ppc/internal.h
>> @@ -293,13 +293,4 @@ FIELD(GER_MSK, XMSK, 0, 4)
>>  FIELD(GER_MSK, YMSK, 4, 4)
>>  FIELD(GER_MSK, PMSK, 8, 8)
>>
>> -static inline int ger_pack_masks(int pmsk, int ymsk, int xmsk)
>> -{
>> -    int msk = 0;
>> -    msk = FIELD_DP32(msk, GER_MSK, XMSK, xmsk);
>> -    msk = FIELD_DP32(msk, GER_MSK, YMSK, ymsk);
>> -    msk = FIELD_DP32(msk, GER_MSK, PMSK, pmsk);
>> -    return msk;
>> -}
>> -
>>  #endif /* PPC_INTERNAL_H */
>> diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
>> index 0266f09119..62950d348a 100644
>> --- a/target/ppc/translate/vsx-impl.c.inc
>> +++ b/target/ppc/translate/vsx-impl.c.inc
>> @@ -2819,7 +2819,7 @@ static bool trans_XXSETACCZ(DisasContext *ctx, arg_X_a *a)
>>  static bool do_ger(DisasContext *ctx, arg_MMIRR_XX3 *a,
>>      void (*helper)(TCGv_env, TCGv_ptr, TCGv_ptr, TCGv_ptr, TCGv_i32))
>>  {
>> -    uint32_t mask;
>> +    uint32_t mask = 0;
>>      TCGv_ptr xt, xa, xb;
>>      REQUIRE_INSNS_FLAGS2(ctx, ISA310);
>>      REQUIRE_VSX(ctx);
>> @@ -2832,7 +2832,9 @@ static bool do_ger(DisasContext *ctx, arg_MMIRR_XX3 *a,
>>      xa = gen_vsr_ptr(a->xa);
>>      xb = gen_vsr_ptr(a->xb);
>>
>> -    mask = ger_pack_masks(a->pmsk, a->ymsk, a->xmsk);
>> +    mask = FIELD_DP32(mask, GER_MSK, XMSK, a->xmsk);
>> +    mask = FIELD_DP32(mask, GER_MSK, YMSK, a->ymsk);
>> +    mask = FIELD_DP32(mask, GER_MSK, PMSK, a->pmsk);
>>      helper(tcg_env, xa, xb, xt, tcg_constant_i32(mask));
>>      return true;
>>  }
>
>