Re: [PATCH v4 09/16] tcg/s390x: Implement andc, orc, abs, neg, not vector operations

[David Hildenbrand]

On 26.06.21 07:03, Richard Henderson wrote:
> These logical and arithmetic operations are optional but trivial.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/s390x/tcg-target-con-set.h |  1 +
>   tcg/s390x/tcg-target.h         | 10 +++++-----
>   tcg/s390x/tcg-target.c.inc     | 34 +++++++++++++++++++++++++++++++++-
>   3 files changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/tcg/s390x/tcg-target-con-set.h b/tcg/s390x/tcg-target-con-set.h
> index ce9432cfe3..cb953896d5 100644
> --- a/tcg/s390x/tcg-target-con-set.h
> +++ b/tcg/s390x/tcg-target-con-set.h
> @@ -17,6 +17,7 @@ C_O0_I2(v, r)
>   C_O1_I1(r, L)
>   C_O1_I1(r, r)
>   C_O1_I1(v, r)
> +C_O1_I1(v, v)
>   C_O1_I1(v, vr)
>   C_O1_I2(r, 0, ri)
>   C_O1_I2(r, 0, rI)
> diff --git a/tcg/s390x/tcg-target.h b/tcg/s390x/tcg-target.h
> index db54266da0..a3d4b5111f 100644
> --- a/tcg/s390x/tcg-target.h
> +++ b/tcg/s390x/tcg-target.h
> @@ -143,11 +143,11 @@ extern uint64_t s390_facilities[3];
>   #define TCG_TARGET_HAS_v128           HAVE_FACILITY(VECTOR)
>   #define TCG_TARGET_HAS_v256           0
>   
> -#define TCG_TARGET_HAS_andc_vec       0
> -#define TCG_TARGET_HAS_orc_vec        0
> -#define TCG_TARGET_HAS_not_vec        0
> -#define TCG_TARGET_HAS_neg_vec        0
> -#define TCG_TARGET_HAS_abs_vec        0
> +#define TCG_TARGET_HAS_andc_vec       1
> +#define TCG_TARGET_HAS_orc_vec        1
> +#define TCG_TARGET_HAS_not_vec        1
> +#define TCG_TARGET_HAS_neg_vec        1
> +#define TCG_TARGET_HAS_abs_vec        1
>   #define TCG_TARGET_HAS_roti_vec       0
>   #define TCG_TARGET_HAS_rots_vec       0
>   #define TCG_TARGET_HAS_rotv_vec       0
> diff --git a/tcg/s390x/tcg-target.c.inc b/tcg/s390x/tcg-target.c.inc
> index c0622daaa0..040690abe2 100644
> --- a/tcg/s390x/tcg-target.c.inc
> +++ b/tcg/s390x/tcg-target.c.inc
> @@ -270,13 +270,18 @@ typedef enum S390Opcode {
>       VRIb_VGM    = 0xe746,
>       VRIc_VREP   = 0xe74d,
>   
> +    VRRa_VLC    = 0xe7de,
> +    VRRa_VLP    = 0xe7df,
>       VRRa_VLR    = 0xe756,
>       VRRc_VA     = 0xe7f3,
>       VRRc_VCEQ   = 0xe7f8,   /* we leave the m5 cs field 0 */
>       VRRc_VCH    = 0xe7fb,   /* " */
>       VRRc_VCHL   = 0xe7f9,   /* " */
>       VRRc_VN     = 0xe768,
> +    VRRc_VNC    = 0xe769,
> +    VRRc_VNO    = 0xe76b,
>       VRRc_VO     = 0xe76a,
> +    VRRc_VOC    = 0xe76f,

VOC requires the vector-enhancements facility 1.

>       VRRc_VS     = 0xe7f7,
>       VRRc_VX     = 0xe76d,
>       VRRf_VLVGP  = 0xe762,
> @@ -2637,6 +2642,16 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
>           tcg_out_dupm_vec(s, type, vece, a0, a1, a2);
>           break;
>   
> +    case INDEX_op_abs_vec:
> +        tcg_out_insn(s, VRRa, VLP, a0, a1, vece);
> +        break;
> +    case INDEX_op_neg_vec:
> +        tcg_out_insn(s, VRRa, VLC, a0, a1, vece);
> +        break;
> +    case INDEX_op_not_vec:
> +        tcg_out_insn(s, VRRc, VNO, a0, a1, a1, 0);
> +        break;
> +
>       case INDEX_op_add_vec:
>           tcg_out_insn(s, VRRc, VA, a0, a1, a2, vece);
>           break;
> @@ -2646,9 +2661,15 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
>       case INDEX_op_and_vec:
>           tcg_out_insn(s, VRRc, VN, a0, a1, a2, 0);
>           break;
> +    case INDEX_op_andc_vec:
> +        tcg_out_insn(s, VRRc, VNC, a0, a1, a2, 0);
> +        break;
>       case INDEX_op_or_vec:
>           tcg_out_insn(s, VRRc, VO, a0, a1, a2, 0);
>           break;
> +    case INDEX_op_orc_vec:
> +        tcg_out_insn(s, VRRc, VOC, a0, a1, a2, 0);
> +        break;
>       case INDEX_op_xor_vec:
>           tcg_out_insn(s, VRRc, VX, a0, a1, a2, 0);
>           break;
> @@ -2679,10 +2700,15 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
>   int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type, unsigned vece)
>   {
>       switch (opc) {
> +    case INDEX_op_abs_vec:
>       case INDEX_op_add_vec:
> -    case INDEX_op_sub_vec:

Seems like an unrelated change that should have been performed in the 
introducing patch.


Apart from that:

Reviewed-by: David Hildenbrand <david@redhat.com>

--
Thanks,

David / dhildenb