Re: [Qemu-devel] [PATCH v9 2/7] target/ppc: replace __builtin_ffssl() by the equivalent ctz routines

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

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

On Mon, Dec 17, 2018 at 11:34:40PM +0100, Cédric Le Goater wrote:
> And remove the intermediate MASK_TO_LSH macro which does not add any value.
> 
> This fixes a compile breakage on windows.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

It's an improvement over what's there, but it still leaves macros
whose primary use would be for guest registers, but are typed
according to host values, which doesn't make much sense.

I think instead we should redefine your BE64 / BE32 variants in terms
of the existing extract*() and deposit*() primitives, and get rid of
the GETFIELD / SETFIELD macros.

> ---
>  target/ppc/cpu.h | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 527181c0f09f..f4ef4f214564 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -78,18 +78,21 @@
>                                   PPC_BIT32(bs))
>  #define PPC_BITMASK8(bs, be)    ((PPC_BIT8(bs) - PPC_BIT8(be)) | PPC_BIT8(bs))
>  
> +/*
> + * OPAL PPC bitmask field manipulation, used in XIVE, PHB3 and PHB4
> + */
>  #if HOST_LONG_BITS == 32
> -# define MASK_TO_LSH(m)          (__builtin_ffsll(m) - 1)
> +#  define GETFIELD(m, v)        (((v) & (m)) >> ctz32(m))
> +#  define SETFIELD(m, v, val)                                   \
> +    (((v) & ~(m)) | ((((typeof(v))(val)) << ctz32(m)) & (m)))
>  #elif HOST_LONG_BITS == 64
> -# define MASK_TO_LSH(m)          (__builtin_ffsl(m) - 1)
> +#  define GETFIELD(m, v)        (((v) & (m)) >> ctz64(m))
> +#  define SETFIELD(m, v, val)                                   \
> +    (((v) & ~(m)) | ((((typeof(v))(val)) << ctz64(m)) & (m)))
>  #else
>  # error Unknown sizeof long
>  #endif
>  
> -#define GETFIELD(m, v)          (((v) & (m)) >> MASK_TO_LSH(m))
> -#define SETFIELD(m, v, val)                             \
> -        (((v) & ~(m)) | ((((typeof(v))(val)) << MASK_TO_LSH(m)) & (m)))
> -
>  /*****************************************************************************/
>  /* Exception vectors definitions                                             */
>  enum {

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