Re: [qemu-s390x] [Qemu-devel] [PATCH v2 01/20] tcg: Replace MO_8 with MO_UB alias

[David Hildenbrand]

On 22.07.19 17:38, address@hidden wrote:
> Preparation for splitting MO_8 out from TCGMemOp into new accelerator
> independent MemOp.
> 
> As MO_8 will be a value of MemOp, existing TCGMemOp comparisons and
> coercions will trigger -Wenum-compare and -Wenum-conversion.
> 
> Signed-off-by: Tony Nguyen <address@hidden>
> ---
>  target/arm/sve_helper.c             |  4 +-
>  target/arm/translate-a64.c          | 14 +++----
>  target/arm/translate-sve.c          |  4 +-
>  target/arm/translate.c              | 38 +++++++++----------
>  target/i386/translate.c             | 72
> +++++++++++++++++------------------
>  target/mips/translate.c             |  4 +-
>  target/ppc/translate/vmx-impl.inc.c | 28 +++++++-------
>  target/s390x/translate.c            |  2 +-
>  target/s390x/translate_vx.inc.c     |  4 +-
>  target/s390x/vec.h                  |  4 +-
>  tcg/aarch64/tcg-target.inc.c        | 16 ++++----
>  tcg/arm/tcg-target.inc.c            |  6 +--
>  tcg/i386/tcg-target.inc.c           | 54 +++++++++++++-------------
>  tcg/mips/tcg-target.inc.c           |  4 +-
>  tcg/riscv/tcg-target.inc.c          |  4 +-
>  tcg/sparc/tcg-target.inc.c          |  2 +-
>  tcg/tcg-op-gvec.c                   | 76
> ++++++++++++++++++-------------------
>  tcg/tcg-op-vec.c                    | 10 ++---
>  tcg/tcg-op.c                        |  6 +--
>  tcg/tcg.h                           |  2 +-
>  20 files changed, 177 insertions(+), 177 deletions(-)
> 
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index fc0c175..4c7e11f 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -1531,7 +1531,7 @@ void HELPER(sve_cpy_m_b)(void *vd, void *vn, void *vg,
>      uint64_t *d = vd, *n = vn;
>      uint8_t *pg = vg;
>  
> -    mm = dup_const(MO_8, mm);
> +    mm = dup_const(MO_UB, mm);

Sorry, but I don't like this. I never liked the use of
byte/word/long/quad for 8/16/32/64. It is a constant source of
confusion. Yes, we have it in the code, but at least I'd rather want to
see this getting less than more.

-- 

Thanks,

David / dhildenb