Re: [Qemu-devel] [PATCH] target/ppc: fix compilation breakage on windows

[David Gibson <david@gibson.dropbear.id.au>]

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

On Sat, Dec 15, 2018 at 07:42:22PM +0100, Cédric Le Goater wrote:
> Fix the PPC_BIT definitions to use ULL instead in UL and replace
> __builtin_ffssl() by the equivalent ctz routines.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  Compile tested with --cross-prefix=x86_64-w64-mingw32-. When I have
>  some more time, I might try runtime on windows also.

Which version of the mingw compiler do you have?  As I've noted
elsewhere, I haven't been able to reproduce the problem with the mingw
in Fedora 29.

>  The PPC compile failures have been there for a while (pre 2.12) and
>  the MASK_TO_LSH macro was not used until the XIVE definitions were
>  introduced.
> 
>  I let you guys decide on how you want to proceed, but I think it is
>  safe to merge this patch as a prereq of the pull request.
> 
>  Thanks,
> 
>  C.
> 
>  target/ppc/cpu.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index ab68abe8a23c..a3d3e91eb4ce 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -70,18 +70,18 @@
>  #define PPC_ELF_MACHINE     EM_PPC
>  #endif
>  
> -#define PPC_BIT(bit)            (0x8000000000000000UL >> (bit))
> -#define PPC_BIT32(bit)          (0x80000000UL >> (bit))
> -#define PPC_BIT8(bit)           (0x80UL >> (bit))
> +#define PPC_BIT(bit)            (0x8000000000000000ULL >> (bit))
> +#define PPC_BIT32(bit)          (0x80000000ULL >> (bit))
> +#define PPC_BIT8(bit)           (0x80ULL >> (bit))

We shouldn't need the ULL for the 32-bit and (especially) 8-bit
versions.

>  #define PPC_BITMASK(bs, be)     ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs))
>  #define PPC_BITMASK32(bs, be)   ((PPC_BIT32(bs) - PPC_BIT32(be)) | \
>                                   PPC_BIT32(bs))
>  #define PPC_BITMASK8(bs, be)    ((PPC_BIT8(bs) - PPC_BIT8(be)) | PPC_BIT8(bs))
>  
>  #if HOST_LONG_BITS == 32
> -# define MASK_TO_LSH(m)          (__builtin_ffsll(m) - 1)
> +# define MASK_TO_LSH(m)          ctz32(m)
>  #elif HOST_LONG_BITS == 64
> -# define MASK_TO_LSH(m)          (__builtin_ffsl(m) - 1)
> +# define MASK_TO_LSH(m)          ctz64(m)

Having another look at this, this seems bogus: we're defining an
interface that's only correct on a *host long*.  But when we use it
we're almost certain to be working on guest values with a fixed size,
so using this wrapper just adds confusion.

I'm not really seeing any reason to use MASK_TO_LSH() rather than
using ctz32() and ctz64() explicitly.

-- 
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 --]