* [PATCH v2 0/1] target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9 @ 2020-05-05 18:38 Daniele Buono 2020-05-05 18:38 ` [PATCH v2 1/1] " Daniele Buono 0 siblings, 1 reply; 3+ messages in thread From: Daniele Buono @ 2020-05-05 18:38 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-ppc, Daniele Buono, David Gibson 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2 1/1] target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9 2020-05-05 18:38 [PATCH v2 0/1] target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9 Daniele Buono @ 2020-05-05 18:38 ` Daniele Buono 2020-05-06 1:36 ` David Gibson 0 siblings, 1 reply; 3+ messages in thread From: Daniele Buono @ 2020-05-05 18:38 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-ppc, Daniele Buono, David Gibson 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 1/1] target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9 2020-05-05 18:38 ` [PATCH v2 1/1] " Daniele Buono @ 2020-05-06 1:36 ` David Gibson 0 siblings, 0 replies; 3+ messages in thread From: David Gibson @ 2020-05-06 1:36 UTC (permalink / raw) To: Daniele Buono; +Cc: qemu-ppc, qemu-devel [-- 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 --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-06 1:54 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-05-05 18:38 [PATCH v2 0/1] target-ppc: fix rlwimi, rlwinm, rlwnm for Clang-9 Daniele Buono 2020-05-05 18:38 ` [PATCH v2 1/1] " Daniele Buono 2020-05-06 1:36 ` David Gibson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).