diff mbox series

[04/20] tcg/s390x: Implement vector NAND, NOR, EQV

Message ID 20211218194250.247633-5-richard.henderson@linaro.org
State New
Headers show
Series tcg: vector improvements | expand

Commit Message

Richard Henderson Dec. 18, 2021, 7:42 p.m. UTC
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/s390x/tcg-target.h     |  6 +++---
 tcg/s390x/tcg-target.c.inc | 17 +++++++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

Comments

Philippe Mathieu-Daudé Dec. 19, 2021, 12:17 a.m. UTC | #1
On 12/18/21 20:42, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/s390x/tcg-target.h     |  6 +++---
>  tcg/s390x/tcg-target.c.inc | 17 +++++++++++++++++
>  2 files changed, 20 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Alex Bennée Feb. 1, 2022, 6:29 p.m. UTC | #2
Richard Henderson <richard.henderson@linaro.org> writes:

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Alex Bennée Feb. 1, 2022, 6:31 p.m. UTC | #3
Richard Henderson <richard.henderson@linaro.org> writes:

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Philippe Mathieu-Daudé Jan. 3, 2024, 1:21 p.m. UTC | #4
Hi Richard,

(revisiting this old patch which is now commit 21eab5bfae)

On 18/12/21 20:42, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/s390x/tcg-target.h     |  6 +++---
>   tcg/s390x/tcg-target.c.inc | 17 +++++++++++++++++
>   2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/tcg/s390x/tcg-target.h b/tcg/s390x/tcg-target.h
> index ad29e62b16..fef227b0fe 100644
> --- a/tcg/s390x/tcg-target.h
> +++ b/tcg/s390x/tcg-target.h
> @@ -145,9 +145,9 @@ extern uint64_t s390_facilities[3];
>   
>   #define TCG_TARGET_HAS_andc_vec       1
>   #define TCG_TARGET_HAS_orc_vec        HAVE_FACILITY(VECTOR_ENH1)
> -#define TCG_TARGET_HAS_nand_vec       0
> -#define TCG_TARGET_HAS_nor_vec        0
> -#define TCG_TARGET_HAS_eqv_vec        0
> +#define TCG_TARGET_HAS_nand_vec       HAVE_FACILITY(VECTOR_ENH1)
> +#define TCG_TARGET_HAS_nor_vec        1
> +#define TCG_TARGET_HAS_eqv_vec        HAVE_FACILITY(VECTOR_ENH1)

Here some opcodes are conditional, ...

>   #define TCG_TARGET_HAS_not_vec        1
>   #define TCG_TARGET_HAS_neg_vec        1
>   #define TCG_TARGET_HAS_abs_vec        1
> diff --git a/tcg/s390x/tcg-target.c.inc b/tcg/s390x/tcg-target.c.inc
> index 57e803e339..5a90b892cb 100644
> --- a/tcg/s390x/tcg-target.c.inc
> +++ b/tcg/s390x/tcg-target.c.inc
> @@ -288,7 +288,9 @@ typedef enum S390Opcode {
>       VRRc_VMXL   = 0xe7fd,
>       VRRc_VN     = 0xe768,
>       VRRc_VNC    = 0xe769,
> +    VRRc_VNN    = 0xe76e,
>       VRRc_VNO    = 0xe76b,
> +    VRRc_VNX    = 0xe76c,
>       VRRc_VO     = 0xe76a,
>       VRRc_VOC    = 0xe76f,
>       VRRc_VPKS   = 0xe797,   /* we leave the m5 cs field 0 */
> @@ -2750,6 +2752,15 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
>       case INDEX_op_xor_vec:
>           tcg_out_insn(s, VRRc, VX, a0, a1, a2, 0);
>           break;
> +    case INDEX_op_nand_vec:
> +        tcg_out_insn(s, VRRc, VNN, a0, a1, a2, 0);
> +        break;
> +    case INDEX_op_nor_vec:
> +        tcg_out_insn(s, VRRc, VNO, a0, a1, a2, 0);
> +        break;
> +    case INDEX_op_eqv_vec:
> +        tcg_out_insn(s, VRRc, VNX, a0, a1, a2, 0);
> +        break;
>   
>       case INDEX_op_shli_vec:
>           tcg_out_insn(s, VRSa, VESL, a0, a2, TCG_REG_NONE, a1, vece);
> @@ -2846,7 +2857,10 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type, unsigned vece)
>       case INDEX_op_and_vec:
>       case INDEX_op_andc_vec:
>       case INDEX_op_bitsel_vec:
> +    case INDEX_op_eqv_vec:
> +    case INDEX_op_nand_vec:

... but here we unconditionally return 1 for them.

Shouldn't we return TCG_TARGET_HAS_opcode instead?

>       case INDEX_op_neg_vec:
> +    case INDEX_op_nor_vec:
>       case INDEX_op_not_vec:
>       case INDEX_op_or_vec:
>       case INDEX_op_orc_vec:

(expanding context)

             return 1;

> @@ -3191,6 +3205,9 @@ static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode op)
>       case INDEX_op_or_vec:
>       case INDEX_op_orc_vec:
>       case INDEX_op_xor_vec:
> +    case INDEX_op_nand_vec:
> +    case INDEX_op_nor_vec:
> +    case INDEX_op_eqv_vec:
>       case INDEX_op_cmp_vec:
>       case INDEX_op_mul_vec:
>       case INDEX_op_rotlv_vec:
Richard Henderson Jan. 3, 2024, 9:58 p.m. UTC | #5
On 1/4/24 00:21, Philippe Mathieu-Daudé wrote:
> Hi Richard,
> 
> (revisiting this old patch which is now commit 21eab5bfae)
> 
> On 18/12/21 20:42, Richard Henderson wrote:
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   tcg/s390x/tcg-target.h     |  6 +++---
>>   tcg/s390x/tcg-target.c.inc | 17 +++++++++++++++++
>>   2 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/tcg/s390x/tcg-target.h b/tcg/s390x/tcg-target.h
>> index ad29e62b16..fef227b0fe 100644
>> --- a/tcg/s390x/tcg-target.h
>> +++ b/tcg/s390x/tcg-target.h
>> @@ -145,9 +145,9 @@ extern uint64_t s390_facilities[3];
>>   #define TCG_TARGET_HAS_andc_vec       1
>>   #define TCG_TARGET_HAS_orc_vec        HAVE_FACILITY(VECTOR_ENH1)
>> -#define TCG_TARGET_HAS_nand_vec       0
>> -#define TCG_TARGET_HAS_nor_vec        0
>> -#define TCG_TARGET_HAS_eqv_vec        0
>> +#define TCG_TARGET_HAS_nand_vec       HAVE_FACILITY(VECTOR_ENH1)
>> +#define TCG_TARGET_HAS_nor_vec        1
>> +#define TCG_TARGET_HAS_eqv_vec        HAVE_FACILITY(VECTOR_ENH1)
> 
> Here some opcodes are conditional, ...
> 
>>   #define TCG_TARGET_HAS_not_vec        1
>>   #define TCG_TARGET_HAS_neg_vec        1
>>   #define TCG_TARGET_HAS_abs_vec        1
>> diff --git a/tcg/s390x/tcg-target.c.inc b/tcg/s390x/tcg-target.c.inc
>> index 57e803e339..5a90b892cb 100644
>> --- a/tcg/s390x/tcg-target.c.inc
>> +++ b/tcg/s390x/tcg-target.c.inc
>> @@ -288,7 +288,9 @@ typedef enum S390Opcode {
>>       VRRc_VMXL   = 0xe7fd,
>>       VRRc_VN     = 0xe768,
>>       VRRc_VNC    = 0xe769,
>> +    VRRc_VNN    = 0xe76e,
>>       VRRc_VNO    = 0xe76b,
>> +    VRRc_VNX    = 0xe76c,
>>       VRRc_VO     = 0xe76a,
>>       VRRc_VOC    = 0xe76f,
>>       VRRc_VPKS   = 0xe797,   /* we leave the m5 cs field 0 */
>> @@ -2750,6 +2752,15 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
>>       case INDEX_op_xor_vec:
>>           tcg_out_insn(s, VRRc, VX, a0, a1, a2, 0);
>>           break;
>> +    case INDEX_op_nand_vec:
>> +        tcg_out_insn(s, VRRc, VNN, a0, a1, a2, 0);
>> +        break;
>> +    case INDEX_op_nor_vec:
>> +        tcg_out_insn(s, VRRc, VNO, a0, a1, a2, 0);
>> +        break;
>> +    case INDEX_op_eqv_vec:
>> +        tcg_out_insn(s, VRRc, VNX, a0, a1, a2, 0);
>> +        break;
>>       case INDEX_op_shli_vec:
>>           tcg_out_insn(s, VRSa, VESL, a0, a2, TCG_REG_NONE, a1, vece);
>> @@ -2846,7 +2857,10 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type, unsigned vece)
>>       case INDEX_op_and_vec:
>>       case INDEX_op_andc_vec:
>>       case INDEX_op_bitsel_vec:
>> +    case INDEX_op_eqv_vec:
>> +    case INDEX_op_nand_vec:
> 
> ... but here we unconditionally return 1 for them.
> 
> Shouldn't we return TCG_TARGET_HAS_opcode instead?

Yes, you're right.  There's some logical overlap between tcg_gen_emit_vec_op and 
tcg_op_supported, and I think I confused myself a bit there.


r~
diff mbox series

Patch

diff --git a/tcg/s390x/tcg-target.h b/tcg/s390x/tcg-target.h
index ad29e62b16..fef227b0fe 100644
--- a/tcg/s390x/tcg-target.h
+++ b/tcg/s390x/tcg-target.h
@@ -145,9 +145,9 @@  extern uint64_t s390_facilities[3];
 
 #define TCG_TARGET_HAS_andc_vec       1
 #define TCG_TARGET_HAS_orc_vec        HAVE_FACILITY(VECTOR_ENH1)
-#define TCG_TARGET_HAS_nand_vec       0
-#define TCG_TARGET_HAS_nor_vec        0
-#define TCG_TARGET_HAS_eqv_vec        0
+#define TCG_TARGET_HAS_nand_vec       HAVE_FACILITY(VECTOR_ENH1)
+#define TCG_TARGET_HAS_nor_vec        1
+#define TCG_TARGET_HAS_eqv_vec        HAVE_FACILITY(VECTOR_ENH1)
 #define TCG_TARGET_HAS_not_vec        1
 #define TCG_TARGET_HAS_neg_vec        1
 #define TCG_TARGET_HAS_abs_vec        1
diff --git a/tcg/s390x/tcg-target.c.inc b/tcg/s390x/tcg-target.c.inc
index 57e803e339..5a90b892cb 100644
--- a/tcg/s390x/tcg-target.c.inc
+++ b/tcg/s390x/tcg-target.c.inc
@@ -288,7 +288,9 @@  typedef enum S390Opcode {
     VRRc_VMXL   = 0xe7fd,
     VRRc_VN     = 0xe768,
     VRRc_VNC    = 0xe769,
+    VRRc_VNN    = 0xe76e,
     VRRc_VNO    = 0xe76b,
+    VRRc_VNX    = 0xe76c,
     VRRc_VO     = 0xe76a,
     VRRc_VOC    = 0xe76f,
     VRRc_VPKS   = 0xe797,   /* we leave the m5 cs field 0 */
@@ -2750,6 +2752,15 @@  static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
     case INDEX_op_xor_vec:
         tcg_out_insn(s, VRRc, VX, a0, a1, a2, 0);
         break;
+    case INDEX_op_nand_vec:
+        tcg_out_insn(s, VRRc, VNN, a0, a1, a2, 0);
+        break;
+    case INDEX_op_nor_vec:
+        tcg_out_insn(s, VRRc, VNO, a0, a1, a2, 0);
+        break;
+    case INDEX_op_eqv_vec:
+        tcg_out_insn(s, VRRc, VNX, a0, a1, a2, 0);
+        break;
 
     case INDEX_op_shli_vec:
         tcg_out_insn(s, VRSa, VESL, a0, a2, TCG_REG_NONE, a1, vece);
@@ -2846,7 +2857,10 @@  int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type, unsigned vece)
     case INDEX_op_and_vec:
     case INDEX_op_andc_vec:
     case INDEX_op_bitsel_vec:
+    case INDEX_op_eqv_vec:
+    case INDEX_op_nand_vec:
     case INDEX_op_neg_vec:
+    case INDEX_op_nor_vec:
     case INDEX_op_not_vec:
     case INDEX_op_or_vec:
     case INDEX_op_orc_vec:
@@ -3191,6 +3205,9 @@  static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode op)
     case INDEX_op_or_vec:
     case INDEX_op_orc_vec:
     case INDEX_op_xor_vec:
+    case INDEX_op_nand_vec:
+    case INDEX_op_nor_vec:
+    case INDEX_op_eqv_vec:
     case INDEX_op_cmp_vec:
     case INDEX_op_mul_vec:
     case INDEX_op_rotlv_vec: