[PATCH v2 0/1] target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9

[PATCH v2 0/1] target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9

[Daniele Buono]

This patch fixes a compilation error with Clang v9 and higher in
target/ppc/translate.c, on a comparison that is always true in PPC32
because of type sizes.

More information about the issue are in the first version of the patch.

v2, changed to avoid the nested ifdef/conditional solution of v1, and
keep a code structure more similar to the original.

Daniele Buono (1):
  target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9

 target/ppc/translate.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

-- 
2.26.2

[PATCH v2 1/1] target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9

[Daniele Buono]

Starting with Clang v9, -Wtype-limits is implemented and triggers a
few "result of comparison is always true" errors when compiling PPC32
targets.

The comparisons seem to be necessary only on PPC64, since the
else branch in PPC32 only has a "g_assert_not_reached();" in all cases.

This patch restructures the code so that the actual if/else is done on a
local flag variable, that is set accordingly for PPC64, and always
true for PPC32.

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 807d14faaa..338529879f 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1882,6 +1882,7 @@ static void gen_rlwimi(DisasContext *ctx)
         tcg_gen_deposit_tl(t_ra, t_ra, t_rs, sh, me - mb + 1);
     } else {
         target_ulong mask;
+        bool mask_in_32b = true;
         TCGv t1;
 
 #if defined(TARGET_PPC64)
@@ -1890,8 +1891,13 @@ static void gen_rlwimi(DisasContext *ctx)
 #endif
         mask = MASK(mb, me);
 
+#if defined(TARGET_PPC64)
+        if (mask > 0xffffffffu) {
+            mask_in_32b = false;
+        }
+#endif
         t1 = tcg_temp_new();
-        if (mask <= 0xffffffffu) {
+        if (mask_in_32b) {
             TCGv_i32 t0 = tcg_temp_new_i32();
             tcg_gen_trunc_tl_i32(t0, t_rs);
             tcg_gen_rotli_i32(t0, t0, sh);
@@ -1933,12 +1939,18 @@ static void gen_rlwinm(DisasContext *ctx)
         tcg_gen_extract_tl(t_ra, t_rs, rsh, len);
     } else {
         target_ulong mask;
+        bool mask_in_32b = true;
 #if defined(TARGET_PPC64)
         mb += 32;
         me += 32;
 #endif
         mask = MASK(mb, me);
-        if (mask <= 0xffffffffu) {
+#if defined(TARGET_PPC64)
+        if (mask > 0xffffffffu) {
+            mask_in_32b = false;
+        }
+#endif
+        if (mask_in_32b) {
             if (sh == 0) {
                 tcg_gen_andi_tl(t_ra, t_rs, mask);
             } else {
@@ -1973,6 +1985,7 @@ static void gen_rlwnm(DisasContext *ctx)
     uint32_t mb = MB(ctx->opcode);
     uint32_t me = ME(ctx->opcode);
     target_ulong mask;
+    bool mask_in_32b = true;
 
 #if defined(TARGET_PPC64)
     mb += 32;
@@ -1980,7 +1993,12 @@ static void gen_rlwnm(DisasContext *ctx)
 #endif
     mask = MASK(mb, me);
 
-    if (mask <= 0xffffffffu) {
+#if defined(TARGET_PPC64)
+    if (mask > 0xffffffffu) {
+        mask_in_32b = false;
+    }
+#endif
+    if (mask_in_32b) {
         TCGv_i32 t0 = tcg_temp_new_i32();
         TCGv_i32 t1 = tcg_temp_new_i32();
         tcg_gen_trunc_tl_i32(t0, t_rb);
-- 
2.26.2

Re: [PATCH v2 1/1] target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3264 bytes --]

On Tue, May 05, 2020 at 02:38:17PM -0400, Daniele Buono wrote:
> Starting with Clang v9, -Wtype-limits is implemented and triggers a
> few "result of comparison is always true" errors when compiling PPC32
> targets.
> 
> The comparisons seem to be necessary only on PPC64, since the
> else branch in PPC32 only has a "g_assert_not_reached();" in all cases.
> 
> This patch restructures the code so that the actual if/else is done on a
> local flag variable, that is set accordingly for PPC64, and always
> true for PPC32.
> 
> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>

That's a bit nicer, thanks.  Applied to ppc-for-5.1 in place of the
earlier version.

> ---
>  target/ppc/translate.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 807d14faaa..338529879f 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1882,6 +1882,7 @@ static void gen_rlwimi(DisasContext *ctx)
>          tcg_gen_deposit_tl(t_ra, t_ra, t_rs, sh, me - mb + 1);
>      } else {
>          target_ulong mask;
> +        bool mask_in_32b = true;
>          TCGv t1;
>  
>  #if defined(TARGET_PPC64)
> @@ -1890,8 +1891,13 @@ static void gen_rlwimi(DisasContext *ctx)
>  #endif
>          mask = MASK(mb, me);
>  
> +#if defined(TARGET_PPC64)
> +        if (mask > 0xffffffffu) {
> +            mask_in_32b = false;
> +        }
> +#endif
>          t1 = tcg_temp_new();
> -        if (mask <= 0xffffffffu) {
> +        if (mask_in_32b) {
>              TCGv_i32 t0 = tcg_temp_new_i32();
>              tcg_gen_trunc_tl_i32(t0, t_rs);
>              tcg_gen_rotli_i32(t0, t0, sh);
> @@ -1933,12 +1939,18 @@ static void gen_rlwinm(DisasContext *ctx)
>          tcg_gen_extract_tl(t_ra, t_rs, rsh, len);
>      } else {
>          target_ulong mask;
> +        bool mask_in_32b = true;
>  #if defined(TARGET_PPC64)
>          mb += 32;
>          me += 32;
>  #endif
>          mask = MASK(mb, me);
> -        if (mask <= 0xffffffffu) {
> +#if defined(TARGET_PPC64)
> +        if (mask > 0xffffffffu) {
> +            mask_in_32b = false;
> +        }
> +#endif
> +        if (mask_in_32b) {
>              if (sh == 0) {
>                  tcg_gen_andi_tl(t_ra, t_rs, mask);
>              } else {
> @@ -1973,6 +1985,7 @@ static void gen_rlwnm(DisasContext *ctx)
>      uint32_t mb = MB(ctx->opcode);
>      uint32_t me = ME(ctx->opcode);
>      target_ulong mask;
> +    bool mask_in_32b = true;
>  
>  #if defined(TARGET_PPC64)
>      mb += 32;
> @@ -1980,7 +1993,12 @@ static void gen_rlwnm(DisasContext *ctx)
>  #endif
>      mask = MASK(mb, me);
>  
> -    if (mask <= 0xffffffffu) {
> +#if defined(TARGET_PPC64)
> +    if (mask > 0xffffffffu) {
> +        mask_in_32b = false;
> +    }
> +#endif
> +    if (mask_in_32b) {
>          TCGv_i32 t0 = tcg_temp_new_i32();
>          TCGv_i32 t1 = tcg_temp_new_i32();
>          tcg_gen_trunc_tl_i32(t0, t_rb);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]