Re: [PATCH 1/3] tcg/s390: Change FACILITY representation

[David Hildenbrand]

On 08.05.20 17:57, Richard Henderson wrote:
> We will shortly need to be able to check facilities
> beyond the first 64.  Instead of explicitly masking
> against s390_facilities, create a FACILITY macro
> that indexes an array.
> 
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  tcg/s390/tcg-target.h     | 29 ++++++++-------
>  tcg/s390/tcg-target.inc.c | 74 +++++++++++++++++++--------------------
>  2 files changed, 52 insertions(+), 51 deletions(-)
> 
> diff --git a/tcg/s390/tcg-target.h b/tcg/s390/tcg-target.h
> index 07accabbd1..7ca48457ff 100644
> --- a/tcg/s390/tcg-target.h
> +++ b/tcg/s390/tcg-target.h
> @@ -52,16 +52,19 @@ typedef enum TCGReg {
>  /* A list of relevant facilities used by this translator.  Some of these
>     are required for proper operation, and these are checked at startup.  */
>  
> -#define FACILITY_ZARCH_ACTIVE         (1ULL << (63 - 2))
> -#define FACILITY_LONG_DISP            (1ULL << (63 - 18))
> -#define FACILITY_EXT_IMM              (1ULL << (63 - 21))
> -#define FACILITY_GEN_INST_EXT         (1ULL << (63 - 34))
> -#define FACILITY_LOAD_ON_COND         (1ULL << (63 - 45))
> +#define FACILITY_ZARCH_ACTIVE         2
> +#define FACILITY_LONG_DISP            18
> +#define FACILITY_EXT_IMM              21
> +#define FACILITY_GEN_INST_EXT         34
> +#define FACILITY_LOAD_ON_COND         45
>  #define FACILITY_FAST_BCR_SER         FACILITY_LOAD_ON_COND
>  #define FACILITY_DISTINCT_OPS         FACILITY_LOAD_ON_COND
> -#define FACILITY_LOAD_ON_COND2        (1ULL << (63 - 53))
> +#define FACILITY_LOAD_ON_COND2        53
>  
> -extern uint64_t s390_facilities;
> +extern uint64_t s390_facilities[1];
> +
> +#define FACILITY(X) \
> +    ((s390_facilities[FACILITY_##X / 64] >> (63 - FACILITY_##X % 64)) & 1)

I'd have named this "HAVE_FACILITY" or similar.

Apart from that, looks good

Reviewed-by: David Hildenbrand <address@hidden>

-- 
Thanks,

David / dhildenb