[PATCH 0/6] Add several Power ISA 3.1 32/64-bit vector instructions

[PATCH 0/6] Add several Power ISA 3.1 32/64-bit vector instructions

[Lijun Pan]

This patch series add several newly introduced 32/64-bit vector
instructions in Power ISA 3.1. The newly added instructions are
flagged as ISA300 temporarily in vmx-ops.inc.c and vmx-impl.inc.c
to make them compile and function since Power ISA 3.1, together
with next generation processor, has not been fully enabled in QEMU
yet. When Power ISA 3.1 and next generation processor are fully
supported, the flags should be changed.

Lijun Pan (6):
  target/ppc: add byte-reverse br[dwh] instructions
  target/ppc: add vmulld instruction
  targetc/ppc: add vmulh{su}w instructions
  target/ppc: add vmulh{su}d instructions
  fix the prototype of muls64/mulu64
  target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions

 include/qemu/host-utils.h           |  4 +-
 target/ppc/helper.h                 | 13 ++++++
 target/ppc/int_helper.c             | 58 +++++++++++++++++++++++++
 target/ppc/translate.c              | 65 +++++++++++++++++++++++++++++
 target/ppc/translate/vmx-impl.inc.c | 24 +++++++++++
 target/ppc/translate/vmx-ops.inc.c  | 22 ++++++++--
 6 files changed, 180 insertions(+), 6 deletions(-)

-- 
2.23.0

[PATCH 1/6] target/ppc: add byte-reverse br[dwh] instructions

[Lijun Pan]

POWER ISA 3.1 introduces following byte-reverse instructions:
brd: Byte-Reverse Doubleword X-form
brw: Byte-Reverse Word X-form
brh: Byte-Reverse Halfword X-form

Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
 target/ppc/translate.c | 62 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 4ce3d664b5..2d48fbc8db 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6971,7 +6971,69 @@ static void gen_dform3D(DisasContext *ctx)
     return gen_invalid(ctx);
 }
 
+/* brd */
+static void gen_brd(DisasContext *ctx)
+{
+	TCGv_i64 temp = tcg_temp_new_i64();
+
+	tcg_gen_bswap64_i64(temp, cpu_gpr[rS(ctx->opcode)]);
+	tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));
+
+	tcg_temp_free_i64(temp);
+}
+
+/* brw */
+static void gen_brw(DisasContext *ctx)
+{
+	TCGv_i64 temp = tcg_temp_new_i64();
+	TCGv_i64 lsb = tcg_temp_new_i64();
+	TCGv_i64 msb = tcg_temp_new_i64();
+
+	tcg_gen_movi_i64(lsb, 0x00000000ffffffffull);
+	tcg_gen_and_i64(temp, lsb, cpu_gpr[rS(ctx->opcode)]);
+	tcg_gen_bswap32_i64(lsb, temp);
+	
+	tcg_gen_shri_i64(msb, cpu_gpr[rS(ctx->opcode)], 32);
+	tcg_gen_bswap32_i64(temp, msb);
+	tcg_gen_shli_i64(msb, temp, 32);
+	
+	tcg_gen_or_i64(temp, lsb, msb);
+
+	tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));
+
+	tcg_temp_free_i64(temp);
+	tcg_temp_free_i64(lsb);
+	tcg_temp_free_i64(msb);
+}
+
+/* brh */
+static void gen_brh(DisasContext *ctx)
+{
+	TCGv_i64 temp = tcg_temp_new_i64();
+	TCGv_i64 t0 = tcg_temp_new_i64();
+	TCGv_i64 t1 = tcg_temp_new_i64();
+	TCGv_i64 t2 = tcg_temp_new_i64();
+	TCGv_i64 t3 = tcg_temp_new_i64();
+
+	tcg_gen_movi_i64(t0, 0x00ff00ff00ff00ffull);
+	tcg_gen_shri_i64(t1, cpu_gpr[rS(ctx->opcode)], 8);
+	tcg_gen_and_i64(t2, t1, t0);
+	tcg_gen_and_i64(t1, cpu_gpr[rS(ctx->opcode)], t0);
+	tcg_gen_shli_i64(t1, t1, 8);
+	tcg_gen_or_i64(temp, t1, t2);
+	tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));
+
+	tcg_temp_free_i64(temp);
+	tcg_temp_free_i64(t0);
+	tcg_temp_free_i64(t1);
+	tcg_temp_free_i64(t2);
+	tcg_temp_free_i64(t3);
+}
+
 static opcode_t opcodes[] = {
+GEN_HANDLER_E(brd, 0x1F, 0x1B, 0x05, 0x0000F801, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(brw, 0x1F, 0x1B, 0x04, 0x0000F801, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(brh, 0x1F, 0x1B, 0x06, 0x0000F801, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER(invalid, 0x00, 0x00, 0x00, 0xFFFFFFFF, PPC_NONE),
 GEN_HANDLER(cmp, 0x1F, 0x00, 0x00, 0x00400000, PPC_INTEGER),
 GEN_HANDLER(cmpi, 0x0B, 0xFF, 0xFF, 0x00400000, PPC_INTEGER),
-- 
2.23.0

[PATCH 2/6] target/ppc: add vmulld instruction

[Lijun Pan]

vmulld: Vector Multiply Low Doubleword.

Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
 target/ppc/helper.h                 | 1 +
 target/ppc/int_helper.c             | 1 +
 target/ppc/translate/vmx-impl.inc.c | 1 +
 target/ppc/translate/vmx-ops.inc.c  | 1 +
 4 files changed, 4 insertions(+)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 2dfa1c6942..c3f087ccb3 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -185,6 +185,7 @@ DEF_HELPER_3(vmuloub, void, avr, avr, avr)
 DEF_HELPER_3(vmulouh, void, avr, avr, avr)
 DEF_HELPER_3(vmulouw, void, avr, avr, avr)
 DEF_HELPER_3(vmuluwm, void, avr, avr, avr)
+DEF_HELPER_3(vmulld, void, avr, avr, avr)
 DEF_HELPER_3(vslo, void, avr, avr, avr)
 DEF_HELPER_3(vsro, void, avr, avr, avr)
 DEF_HELPER_3(vsrv, void, avr, avr, avr)
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index be53cd6f68..afbcdd05b4 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -533,6 +533,7 @@ void helper_vprtybq(ppc_avr_t *r, ppc_avr_t *b)
         }                                                               \
     }
 VARITH_DO(muluwm, *, u32)
+VARITH_DO(mulld, *, s64)
 #undef VARITH_DO
 #undef VARITH
 
diff --git a/target/ppc/translate/vmx-impl.inc.c b/target/ppc/translate/vmx-impl.inc.c
index 403ed3a01c..4ee1df48f2 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -807,6 +807,7 @@ GEN_VXFORM_DUAL(vmulouw, PPC_ALTIVEC, PPC_NONE,
 GEN_VXFORM(vmulosb, 4, 4);
 GEN_VXFORM(vmulosh, 4, 5);
 GEN_VXFORM(vmulosw, 4, 6);
+GEN_VXFORM(vmulld,  4, 7);
 GEN_VXFORM(vmuleub, 4, 8);
 GEN_VXFORM(vmuleuh, 4, 9);
 GEN_VXFORM(vmuleuw, 4, 10);
diff --git a/target/ppc/translate/vmx-ops.inc.c b/target/ppc/translate/vmx-ops.inc.c
index 84e05fb827..499bed0a44 100644
--- a/target/ppc/translate/vmx-ops.inc.c
+++ b/target/ppc/translate/vmx-ops.inc.c
@@ -104,6 +104,7 @@ GEN_VXFORM_DUAL(vmulouw, vmuluwm, 4, 2, PPC_ALTIVEC, PPC_NONE),
 GEN_VXFORM(vmulosb, 4, 4),
 GEN_VXFORM(vmulosh, 4, 5),
 GEN_VXFORM_207(vmulosw, 4, 6),
+GEN_VXFORM_300(vmulld, 4, 7),
 GEN_VXFORM(vmuleub, 4, 8),
 GEN_VXFORM(vmuleuh, 4, 9),
 GEN_VXFORM_207(vmuleuw, 4, 10),
-- 
2.23.0

[PATCH 3/6] targetc/ppc: add vmulh{su}w instructions

[Lijun Pan]

vmulhsw: Vector Multiply High Signed Word
vmulhuw: Vector Multiply High Unsigned Word

Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
 target/ppc/helper.h                 |  2 ++
 target/ppc/int_helper.c             | 14 ++++++++++++++
 target/ppc/translate/vmx-impl.inc.c |  6 ++++++
 target/ppc/translate/vmx-ops.inc.c  |  4 ++--
 4 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index c3f087ccb3..6d4a3536eb 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -186,6 +186,8 @@ DEF_HELPER_3(vmulouh, void, avr, avr, avr)
 DEF_HELPER_3(vmulouw, void, avr, avr, avr)
 DEF_HELPER_3(vmuluwm, void, avr, avr, avr)
 DEF_HELPER_3(vmulld, void, avr, avr, avr)
+DEF_HELPER_3(vmulhsw, void, avr, avr, avr)
+DEF_HELPER_3(vmulhuw, void, avr, avr, avr)
 DEF_HELPER_3(vslo, void, avr, avr, avr)
 DEF_HELPER_3(vsro, void, avr, avr, avr)
 DEF_HELPER_3(vsrv, void, avr, avr, avr)
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index afbcdd05b4..4bb3b7e928 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -523,6 +523,20 @@ void helper_vprtybq(ppc_avr_t *r, ppc_avr_t *b)
     r->VsrD(0) = 0;
 }
 
+#define VMULH_DO(name, op, element, cast_orig, cast_temp)		\
+    void helper_vmulh##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)	\
+    {									\
+	int i;								\
+									\
+	for (i = 0; i < ARRAY_SIZE(r->element); i++) {			\
+		r->element[i] = (cast_orig)(((cast_temp)a->element[i] op \
+				(cast_temp)b->element[i]) >> 32);	\
+	}								\
+    }
+VMULH_DO(sw, *, s32, int32_t, int64_t)
+VMULH_DO(uw, *, u32, uint32_t, uint64_t)
+#undef VMULH_DO
+
 #define VARITH_DO(name, op, element)                                    \
     void helper_v##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)       \
     {                                                                   \
diff --git a/target/ppc/translate/vmx-impl.inc.c b/target/ppc/translate/vmx-impl.inc.c
index 4ee1df48f2..2c35559c52 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -811,9 +811,15 @@ GEN_VXFORM(vmulld,  4, 7);
 GEN_VXFORM(vmuleub, 4, 8);
 GEN_VXFORM(vmuleuh, 4, 9);
 GEN_VXFORM(vmuleuw, 4, 10);
+GEN_VXFORM(vmulhuw, 4, 10);
+GEN_VXFORM_DUAL(vmuleuw, PPC_ALTIVEC, PPC_NONE,
+		vmulhuw, PPC_NONE, PPC2_ISA300);
 GEN_VXFORM(vmulesb, 4, 12);
 GEN_VXFORM(vmulesh, 4, 13);
 GEN_VXFORM(vmulesw, 4, 14);
+GEN_VXFORM(vmulhsw, 4, 14);
+GEN_VXFORM_DUAL(vmulesw, PPC_ALTIVEC, PPC_NONE,
+		vmulhsw, PPC_NONE, PPC2_ISA300);
 GEN_VXFORM_V(vslb, MO_8, tcg_gen_gvec_shlv, 2, 4);
 GEN_VXFORM_V(vslh, MO_16, tcg_gen_gvec_shlv, 2, 5);
 GEN_VXFORM_V(vslw, MO_32, tcg_gen_gvec_shlv, 2, 6);
diff --git a/target/ppc/translate/vmx-ops.inc.c b/target/ppc/translate/vmx-ops.inc.c
index 499bed0a44..1d8238a718 100644
--- a/target/ppc/translate/vmx-ops.inc.c
+++ b/target/ppc/translate/vmx-ops.inc.c
@@ -107,10 +107,10 @@ GEN_VXFORM_207(vmulosw, 4, 6),
 GEN_VXFORM_300(vmulld, 4, 7),
 GEN_VXFORM(vmuleub, 4, 8),
 GEN_VXFORM(vmuleuh, 4, 9),
-GEN_VXFORM_207(vmuleuw, 4, 10),
+GEN_VXFORM_DUAL(vmuleuw, vmulhuw, 4, 10, PPC_ALTIVEC, PPC_NONE),
 GEN_VXFORM(vmulesb, 4, 12),
 GEN_VXFORM(vmulesh, 4, 13),
-GEN_VXFORM_207(vmulesw, 4, 14),
+GEN_VXFORM_DUAL(vmulesw, vmulhsw, 4, 14, PPC_ALTIVEC, PPC_NONE),
 GEN_VXFORM(vslb, 2, 4),
 GEN_VXFORM(vslh, 2, 5),
 GEN_VXFORM_DUAL(vslw, vrlwnm, 2, 6, PPC_ALTIVEC, PPC_NONE),
-- 
2.23.0

[PATCH 4/6] target/ppc: add vmulh{su}d instructions

[Lijun Pan]

vmulhsd: Vector Multiply High Signed Doubleword
vmulhud: Vector Multiply High Unsigned Doubleword

Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
 target/ppc/helper.h                 |  2 ++
 target/ppc/int_helper.c             | 24 ++++++++++++++++++++++++
 target/ppc/translate/vmx-impl.inc.c |  2 ++
 target/ppc/translate/vmx-ops.inc.c  |  2 ++
 4 files changed, 30 insertions(+)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 6d4a3536eb..1aed2087cf 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -188,6 +188,8 @@ DEF_HELPER_3(vmuluwm, void, avr, avr, avr)
 DEF_HELPER_3(vmulld, void, avr, avr, avr)
 DEF_HELPER_3(vmulhsw, void, avr, avr, avr)
 DEF_HELPER_3(vmulhuw, void, avr, avr, avr)
+DEF_HELPER_3(vmulhsd, void, avr, avr, avr)
+DEF_HELPER_3(vmulhud, void, avr, avr, avr)
 DEF_HELPER_3(vslo, void, avr, avr, avr)
 DEF_HELPER_3(vsro, void, avr, avr, avr)
 DEF_HELPER_3(vsrv, void, avr, avr, avr)
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index 4bb3b7e928..6c401d41f6 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -523,6 +523,30 @@ void helper_vprtybq(ppc_avr_t *r, ppc_avr_t *b)
     r->VsrD(0) = 0;
 }
 
+void helper_vmulhsd(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
+{
+	int i;
+	uint64_t h64 = 0;
+	uint64_t l64 = 0;
+
+	for (i = 0; i < 2; i++) {
+		muls64(&l64, &h64, a->s64[i], b->s64[i]);
+		r->s64[i] = h64;
+	}
+}
+
+void helper_vmulhud(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
+{
+	int i;
+	uint64_t h64 = 0;
+	uint64_t l64 = 0;
+
+	for (i = 0; i < 2; i++) {
+		mulu64(&l64, &h64, a->s64[i], b->s64[i]);
+		r->u64[i] = h64;
+	}
+}
+
 #define VMULH_DO(name, op, element, cast_orig, cast_temp)		\
     void helper_vmulh##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)	\
     {									\
diff --git a/target/ppc/translate/vmx-impl.inc.c b/target/ppc/translate/vmx-impl.inc.c
index 2c35559c52..a9e7e7c3fe 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -812,6 +812,7 @@ GEN_VXFORM(vmuleub, 4, 8);
 GEN_VXFORM(vmuleuh, 4, 9);
 GEN_VXFORM(vmuleuw, 4, 10);
 GEN_VXFORM(vmulhuw, 4, 10);
+GEN_VXFORM(vmulhud, 4, 11);
 GEN_VXFORM_DUAL(vmuleuw, PPC_ALTIVEC, PPC_NONE,
 		vmulhuw, PPC_NONE, PPC2_ISA300);
 GEN_VXFORM(vmulesb, 4, 12);
@@ -820,6 +821,7 @@ GEN_VXFORM(vmulesw, 4, 14);
 GEN_VXFORM(vmulhsw, 4, 14);
 GEN_VXFORM_DUAL(vmulesw, PPC_ALTIVEC, PPC_NONE,
 		vmulhsw, PPC_NONE, PPC2_ISA300);
+GEN_VXFORM(vmulhsd, 4, 15);
 GEN_VXFORM_V(vslb, MO_8, tcg_gen_gvec_shlv, 2, 4);
 GEN_VXFORM_V(vslh, MO_16, tcg_gen_gvec_shlv, 2, 5);
 GEN_VXFORM_V(vslw, MO_32, tcg_gen_gvec_shlv, 2, 6);
diff --git a/target/ppc/translate/vmx-ops.inc.c b/target/ppc/translate/vmx-ops.inc.c
index 1d8238a718..719fecbaa3 100644
--- a/target/ppc/translate/vmx-ops.inc.c
+++ b/target/ppc/translate/vmx-ops.inc.c
@@ -108,9 +108,11 @@ GEN_VXFORM_300(vmulld, 4, 7),
 GEN_VXFORM(vmuleub, 4, 8),
 GEN_VXFORM(vmuleuh, 4, 9),
 GEN_VXFORM_DUAL(vmuleuw, vmulhuw, 4, 10, PPC_ALTIVEC, PPC_NONE),
+GEN_VXFORM_300(vmulhud, 4, 11),
 GEN_VXFORM(vmulesb, 4, 12),
 GEN_VXFORM(vmulesh, 4, 13),
 GEN_VXFORM_DUAL(vmulesw, vmulhsw, 4, 14, PPC_ALTIVEC, PPC_NONE),
+GEN_VXFORM_300(vmulhsd, 4, 15),
 GEN_VXFORM(vslb, 2, 4),
 GEN_VXFORM(vslh, 2, 5),
 GEN_VXFORM_DUAL(vslw, vrlwnm, 2, 6, PPC_ALTIVEC, PPC_NONE),
-- 
2.23.0

[PATCH 5/6] fix the prototype of muls64/mulu64

[Lijun Pan]

The prototypes of muls64/mulu64 in host-utils.h should match the
definitions in host-utils.c

Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
 include/qemu/host-utils.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
index 4cd170e6cd..cdca2991d8 100644
--- a/include/qemu/host-utils.h
+++ b/include/qemu/host-utils.h
@@ -77,8 +77,8 @@ static inline int divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
     }
 }
 #else
-void muls64(uint64_t *phigh, uint64_t *plow, int64_t a, int64_t b);
-void mulu64(uint64_t *phigh, uint64_t *plow, uint64_t a, uint64_t b);
+void muls64(uint64_t *plow, uint64_t *phigh, int64_t a, int64_t b);
+void mulu64(uint64_t *plow, uint64_t *phigh, uint64_t a, uint64_t b);
 int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor);
 int divs128(int64_t *plow, int64_t *phigh, int64_t divisor);
 
-- 
2.23.0

[PATCH 6/6] target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions

[Lijun Pan]

vdivsw: Vector Divide Signed Word
vdivuw: Vector Divide Unsigned Word
vdivsd: Vector Divide Signed Doubleword
vdivud: Vector Divide Unsigned Doubleword
vmodsw: Vector Modulo Signed Word
vmoduw: Vector Modulo Unsigned Word
vmodsd: Vector Modulo Signed Doubleword
vmodud: Vector Modulo Unsigned Doubleword

Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
 target/ppc/helper.h                 |  8 ++++++++
 target/ppc/int_helper.c             | 19 +++++++++++++++++++
 target/ppc/translate.c              |  3 +++
 target/ppc/translate/vmx-impl.inc.c | 15 +++++++++++++++
 target/ppc/translate/vmx-ops.inc.c  | 15 +++++++++++++--
 5 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 1aed2087cf..823999a8c2 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -190,6 +190,14 @@ DEF_HELPER_3(vmulhsw, void, avr, avr, avr)
 DEF_HELPER_3(vmulhuw, void, avr, avr, avr)
 DEF_HELPER_3(vmulhsd, void, avr, avr, avr)
 DEF_HELPER_3(vmulhud, void, avr, avr, avr)
+DEF_HELPER_3(vdivsw, void, avr, avr, avr)
+DEF_HELPER_3(vdivuw, void, avr, avr, avr)
+DEF_HELPER_3(vdivsd, void, avr, avr, avr)
+DEF_HELPER_3(vdivud, void, avr, avr, avr)
+DEF_HELPER_3(vmodsw, void, avr, avr, avr)
+DEF_HELPER_3(vmoduw, void, avr, avr, avr)
+DEF_HELPER_3(vmodsd, void, avr, avr, avr)
+DEF_HELPER_3(vmodud, void, avr, avr, avr)
 DEF_HELPER_3(vslo, void, avr, avr, avr)
 DEF_HELPER_3(vsro, void, avr, avr, avr)
 DEF_HELPER_3(vsrv, void, avr, avr, avr)
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index 6c401d41f6..585533ad53 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -575,6 +575,25 @@ VARITH_DO(mulld, *, s64)
 #undef VARITH_DO
 #undef VARITH
 
+#define VDIV_MOD_DO(name, op, element)                                  \
+    void helper_v##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)       \
+    {                                                                   \
+        int i;                                                          \
+                                                                        \
+        for (i = 0; i < ARRAY_SIZE(r->element); i++) {                  \
+            r->element[i] = a->element[i] op b->element[i];             \
+        }                                                               \
+    }
+VDIV_MOD_DO(divsw, /, s32)
+VDIV_MOD_DO(divuw, /, u32)
+VDIV_MOD_DO(divsd, /, s64)
+VDIV_MOD_DO(divud, /, u64)
+VDIV_MOD_DO(modsw, %, s32)
+VDIV_MOD_DO(moduw, %, u32)
+VDIV_MOD_DO(modsd, %, s64)
+VDIV_MOD_DO(modud, %, u64)
+#undef VDIV_MOD_DO
+
 #define VARITHFP(suffix, func)                                          \
     void helper_v##suffix(CPUPPCState *env, ppc_avr_t *r, ppc_avr_t *a, \
                           ppc_avr_t *b)                                 \
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 2d48fbc8db..59183b5c7b 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -388,6 +388,9 @@ GEN_OPCODE3(name, opc1, opc2, opc3, opc4, inval, type, type2)
 #define GEN_HANDLER2_E_2(name, onam, opc1, opc2, opc3, opc4, inval, typ, typ2) \
 GEN_OPCODE4(name, onam, opc1, opc2, opc3, opc4, inval, typ, typ2)
 
+#define GEN_HANDLER_BOTH(name, opc1, opc2, opc3, inval0, inval1, type0, type1) \
+GEN_OPCODE_DUAL(name, opc1, opc2, opc3, inval0, inval1, type0, type1)
+
 typedef struct opcode_t {
     unsigned char opc1, opc2, opc3, opc4;
 #if HOST_LONG_BITS == 64 /* Explicitly align to 64 bits */
diff --git a/target/ppc/translate/vmx-impl.inc.c b/target/ppc/translate/vmx-impl.inc.c
index a9e7e7c3fe..97ee7cf5eb 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -798,6 +798,9 @@ static void trans_vclzd(DisasContext *ctx)
     tcg_temp_free_i64(avr);
 }
 
+static void gen_vexptefp(DisasContext *ctx);
+static void gen_vlogefp(DisasContext *ctx);
+
 GEN_VXFORM(vmuloub, 4, 0);
 GEN_VXFORM(vmulouh, 4, 1);
 GEN_VXFORM(vmulouw, 4, 2);
@@ -822,6 +825,18 @@ GEN_VXFORM(vmulhsw, 4, 14);
 GEN_VXFORM_DUAL(vmulesw, PPC_ALTIVEC, PPC_NONE,
 		vmulhsw, PPC_NONE, PPC2_ISA300);
 GEN_VXFORM(vmulhsd, 4, 15);
+GEN_VXFORM(vdivuw, 5, 2);
+GEN_VXFORM(vdivud, 5, 3);
+GEN_VXFORM(vdivsw, 5, 6);
+GEN_VXFORM_DUAL_EXT(vexptefp, PPC_ALTIVEC, PPC_NONE, 0x001f0000,
+		vdivsw, PPC_NONE, PPC2_ISA300, 0x00000000);
+GEN_VXFORM(vdivsd, 5, 7);
+GEN_VXFORM_DUAL_EXT(vlogefp, PPC_ALTIVEC, PPC_NONE, 0x001f0000,
+		vdivsd, PPC_NONE, PPC2_ISA300, 0x00000000);
+GEN_VXFORM(vmoduw, 5, 26);
+GEN_VXFORM(vmodud, 5, 27);
+GEN_VXFORM(vmodsw, 5, 30);
+GEN_VXFORM(vmodsd, 5, 31);
 GEN_VXFORM_V(vslb, MO_8, tcg_gen_gvec_shlv, 2, 4);
 GEN_VXFORM_V(vslh, MO_16, tcg_gen_gvec_shlv, 2, 5);
 GEN_VXFORM_V(vslw, MO_32, tcg_gen_gvec_shlv, 2, 6);
diff --git a/target/ppc/translate/vmx-ops.inc.c b/target/ppc/translate/vmx-ops.inc.c
index 719fecbaa3..3425c5156c 100644
--- a/target/ppc/translate/vmx-ops.inc.c
+++ b/target/ppc/translate/vmx-ops.inc.c
@@ -51,6 +51,9 @@ GEN_HANDLER_E_2(name, 0x04, opc2, opc3, opc4, 0x00000000, PPC_NONE,     \
 #define GEN_VXFORM_DUAL(name0, name1, opc2, opc3, type0, type1) \
 GEN_HANDLER_E(name0##_##name1, 0x4, opc2, opc3, 0x00000000, type0, type1)
 
+#define GEN_VXFORM_DUAL_BOTH(name0, name1, opc2, opc3, inval0, inval1, type0, type1) \
+GEN_HANDLER_BOTH(name0##_##name1, 0x4, opc2, opc3, inval0, inval1, type0, type1)
+
 #define GEN_VXRFORM_DUAL(name0, name1, opc2, opc3, tp0, tp1) \
 GEN_HANDLER_E(name0##_##name1, 0x4, opc2, opc3, 0x00000000, tp0, tp1), \
 GEN_HANDLER_E(name0##_##name1, 0x4, opc2, (opc3 | 0x10), 0x00000000, tp0, tp1),
@@ -113,6 +116,16 @@ GEN_VXFORM(vmulesb, 4, 12),
 GEN_VXFORM(vmulesh, 4, 13),
 GEN_VXFORM_DUAL(vmulesw, vmulhsw, 4, 14, PPC_ALTIVEC, PPC_NONE),
 GEN_VXFORM_300(vmulhsd, 4, 15),
+GEN_VXFORM_300(vdivuw, 5, 2),
+GEN_VXFORM_300(vdivud, 5, 3),
+GEN_VXFORM_DUAL_BOTH(vexptefp, vdivsw, 5, 6, 0x001f0000, 0x00000000,
+			PPC_ALTIVEC, PPC2_ISA300),
+GEN_VXFORM_DUAL_BOTH(vlogefp, vdivsd, 5, 7, 0x001f0000, 0x00000000,
+			PPC_ALTIVEC, PPC2_ISA300),
+GEN_VXFORM_300(vmoduw, 5, 26),
+GEN_VXFORM_300(vmodud, 5, 27),
+GEN_VXFORM_300(vmodsw, 5, 30),
+GEN_VXFORM_300(vmodsd, 5, 31),
 GEN_VXFORM(vslb, 2, 4),
 GEN_VXFORM(vslh, 2, 5),
 GEN_VXFORM_DUAL(vslw, vrlwnm, 2, 6, PPC_ALTIVEC, PPC_NONE),
@@ -256,8 +269,6 @@ GEN_VXFORM_NOA(vupkhpx, 7, 13),
 GEN_VXFORM_NOA(vupklpx, 7, 15),
 GEN_VXFORM_NOA(vrefp, 5, 4),
 GEN_VXFORM_NOA(vrsqrtefp, 5, 5),
-GEN_VXFORM_NOA(vexptefp, 5, 6),
-GEN_VXFORM_NOA(vlogefp, 5, 7),
 GEN_VXFORM_NOA(vrfim, 5, 11),
 GEN_VXFORM_NOA(vrfin, 5, 8),
 GEN_VXFORM_NOA(vrfip, 5, 10),
-- 
2.23.0

Re: [PATCH 0/6] Add several Power ISA 3.1 32/64-bit vector instructions

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200613042029.22321-1-ljp@linux.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200613042029.22321-1-ljp@linux.ibm.com
Subject: [PATCH 0/6] Add several Power ISA 3.1 32/64-bit vector instructions
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
d39f30e target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions
966b641 fix the prototype of muls64/mulu64
42111c5 target/ppc: add vmulh{su}d instructions
c96e996 targetc/ppc: add vmulh{su}w instructions
c52004c target/ppc: add vmulld instruction
1061e4e target/ppc: add byte-reverse br[dwh] instructions

=== OUTPUT BEGIN ===
1/6 Checking commit 1061e4ead5bc (target/ppc: add byte-reverse br[dwh] instructions)
ERROR: code indent should never use tabs
#26: FILE: target/ppc/translate.c:6977:
+^ITCGv_i64 temp = tcg_temp_new_i64();$

ERROR: code indent should never use tabs
#28: FILE: target/ppc/translate.c:6979:
+^Itcg_gen_bswap64_i64(temp, cpu_gpr[rS(ctx->opcode)]);$

WARNING: line over 80 characters
#29: FILE: target/ppc/translate.c:6980:
+       tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));

ERROR: code indent should never use tabs
#29: FILE: target/ppc/translate.c:6980:
+^Itcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));$

ERROR: code indent should never use tabs
#31: FILE: target/ppc/translate.c:6982:
+^Itcg_temp_free_i64(temp);$

ERROR: code indent should never use tabs
#37: FILE: target/ppc/translate.c:6988:
+^ITCGv_i64 temp = tcg_temp_new_i64();$

ERROR: code indent should never use tabs
#38: FILE: target/ppc/translate.c:6989:
+^ITCGv_i64 lsb = tcg_temp_new_i64();$

ERROR: code indent should never use tabs
#39: FILE: target/ppc/translate.c:6990:
+^ITCGv_i64 msb = tcg_temp_new_i64();$

ERROR: code indent should never use tabs
#41: FILE: target/ppc/translate.c:6992:
+^Itcg_gen_movi_i64(lsb, 0x00000000ffffffffull);$

ERROR: code indent should never use tabs
#42: FILE: target/ppc/translate.c:6993:
+^Itcg_gen_and_i64(temp, lsb, cpu_gpr[rS(ctx->opcode)]);$

ERROR: code indent should never use tabs
#43: FILE: target/ppc/translate.c:6994:
+^Itcg_gen_bswap32_i64(lsb, temp);$

ERROR: trailing whitespace
#44: FILE: target/ppc/translate.c:6995:
+^I$

ERROR: code indent should never use tabs
#44: FILE: target/ppc/translate.c:6995:
+^I$

ERROR: code indent should never use tabs
#45: FILE: target/ppc/translate.c:6996:
+^Itcg_gen_shri_i64(msb, cpu_gpr[rS(ctx->opcode)], 32);$

ERROR: code indent should never use tabs
#46: FILE: target/ppc/translate.c:6997:
+^Itcg_gen_bswap32_i64(temp, msb);$

ERROR: code indent should never use tabs
#47: FILE: target/ppc/translate.c:6998:
+^Itcg_gen_shli_i64(msb, temp, 32);$

ERROR: trailing whitespace
#48: FILE: target/ppc/translate.c:6999:
+^I$

ERROR: code indent should never use tabs
#48: FILE: target/ppc/translate.c:6999:
+^I$

ERROR: code indent should never use tabs
#49: FILE: target/ppc/translate.c:7000:
+^Itcg_gen_or_i64(temp, lsb, msb);$

WARNING: line over 80 characters
#51: FILE: target/ppc/translate.c:7002:
+       tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));

ERROR: code indent should never use tabs
#51: FILE: target/ppc/translate.c:7002:
+^Itcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));$

ERROR: code indent should never use tabs
#53: FILE: target/ppc/translate.c:7004:
+^Itcg_temp_free_i64(temp);$

ERROR: code indent should never use tabs
#54: FILE: target/ppc/translate.c:7005:
+^Itcg_temp_free_i64(lsb);$

ERROR: code indent should never use tabs
#55: FILE: target/ppc/translate.c:7006:
+^Itcg_temp_free_i64(msb);$

ERROR: code indent should never use tabs
#61: FILE: target/ppc/translate.c:7012:
+^ITCGv_i64 temp = tcg_temp_new_i64();$

ERROR: code indent should never use tabs
#62: FILE: target/ppc/translate.c:7013:
+^ITCGv_i64 t0 = tcg_temp_new_i64();$

ERROR: code indent should never use tabs
#63: FILE: target/ppc/translate.c:7014:
+^ITCGv_i64 t1 = tcg_temp_new_i64();$

ERROR: code indent should never use tabs
#64: FILE: target/ppc/translate.c:7015:
+^ITCGv_i64 t2 = tcg_temp_new_i64();$

ERROR: code indent should never use tabs
#65: FILE: target/ppc/translate.c:7016:
+^ITCGv_i64 t3 = tcg_temp_new_i64();$

ERROR: code indent should never use tabs
#67: FILE: target/ppc/translate.c:7018:
+^Itcg_gen_movi_i64(t0, 0x00ff00ff00ff00ffull);$

ERROR: code indent should never use tabs
#68: FILE: target/ppc/translate.c:7019:
+^Itcg_gen_shri_i64(t1, cpu_gpr[rS(ctx->opcode)], 8);$

ERROR: code indent should never use tabs
#69: FILE: target/ppc/translate.c:7020:
+^Itcg_gen_and_i64(t2, t1, t0);$

ERROR: code indent should never use tabs
#70: FILE: target/ppc/translate.c:7021:
+^Itcg_gen_and_i64(t1, cpu_gpr[rS(ctx->opcode)], t0);$

ERROR: code indent should never use tabs
#71: FILE: target/ppc/translate.c:7022:
+^Itcg_gen_shli_i64(t1, t1, 8);$

ERROR: code indent should never use tabs
#72: FILE: target/ppc/translate.c:7023:
+^Itcg_gen_or_i64(temp, t1, t2);$

WARNING: line over 80 characters
#73: FILE: target/ppc/translate.c:7024:
+       tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));

ERROR: code indent should never use tabs
#73: FILE: target/ppc/translate.c:7024:
+^Itcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));$

ERROR: code indent should never use tabs
#75: FILE: target/ppc/translate.c:7026:
+^Itcg_temp_free_i64(temp);$

ERROR: code indent should never use tabs
#76: FILE: target/ppc/translate.c:7027:
+^Itcg_temp_free_i64(t0);$

ERROR: code indent should never use tabs
#77: FILE: target/ppc/translate.c:7028:
+^Itcg_temp_free_i64(t1);$

ERROR: code indent should never use tabs
#78: FILE: target/ppc/translate.c:7029:
+^Itcg_temp_free_i64(t2);$

ERROR: code indent should never use tabs
#79: FILE: target/ppc/translate.c:7030:
+^Itcg_temp_free_i64(t3);$

total: 39 errors, 3 warnings, 69 lines checked

Patch 1/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/6 Checking commit c52004cd8f89 (target/ppc: add vmulld instruction)
3/6 Checking commit c96e996917c6 (targetc/ppc: add vmulh{su}w instructions)
ERROR: code indent should never use tabs
#34: FILE: target/ppc/int_helper.c:526:
+#define VMULH_DO(name, op, element, cast_orig, cast_temp)^I^I\$

ERROR: code indent should never use tabs
#35: FILE: target/ppc/int_helper.c:527:
+    void helper_vmulh##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)^I\$

ERROR: code indent should never use tabs
#36: FILE: target/ppc/int_helper.c:528:
+    {^I^I^I^I^I^I^I^I^I\$

ERROR: code indent should never use tabs
#37: FILE: target/ppc/int_helper.c:529:
+^Iint i;^I^I^I^I^I^I^I^I\$

ERROR: code indent should never use tabs
#38: FILE: target/ppc/int_helper.c:530:
+^I^I^I^I^I^I^I^I^I\$

ERROR: code indent should never use tabs
#39: FILE: target/ppc/int_helper.c:531:
+^Ifor (i = 0; i < ARRAY_SIZE(r->element); i++) {^I^I^I\$

ERROR: code indent should never use tabs
#40: FILE: target/ppc/int_helper.c:532:
+^I^Ir->element[i] = (cast_orig)(((cast_temp)a->element[i] op \$

ERROR: code indent should never use tabs
#41: FILE: target/ppc/int_helper.c:533:
+^I^I^I^I(cast_temp)b->element[i]) >> 32);^I\$

ERROR: code indent should never use tabs
#42: FILE: target/ppc/int_helper.c:534:
+^I}^I^I^I^I^I^I^I^I\$

ERROR: code indent should never use tabs
#61: FILE: target/ppc/translate/vmx-impl.inc.c:816:
+^I^Ivmulhuw, PPC_NONE, PPC2_ISA300);$

ERROR: code indent should never use tabs
#67: FILE: target/ppc/translate/vmx-impl.inc.c:822:
+^I^Ivmulhsw, PPC_NONE, PPC2_ISA300);$

total: 11 errors, 0 warnings, 55 lines checked

Patch 3/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/6 Checking commit 42111c50379b (target/ppc: add vmulh{su}d instructions)
ERROR: code indent should never use tabs
#36: FILE: target/ppc/int_helper.c:528:
+^Iint i;$

ERROR: code indent should never use tabs
#37: FILE: target/ppc/int_helper.c:529:
+^Iuint64_t h64 = 0;$

ERROR: code indent should never use tabs
#38: FILE: target/ppc/int_helper.c:530:
+^Iuint64_t l64 = 0;$

ERROR: code indent should never use tabs
#40: FILE: target/ppc/int_helper.c:532:
+^Ifor (i = 0; i < 2; i++) {$

ERROR: code indent should never use tabs
#41: FILE: target/ppc/int_helper.c:533:
+^I^Imuls64(&l64, &h64, a->s64[i], b->s64[i]);$

ERROR: code indent should never use tabs
#42: FILE: target/ppc/int_helper.c:534:
+^I^Ir->s64[i] = h64;$

ERROR: code indent should never use tabs
#43: FILE: target/ppc/int_helper.c:535:
+^I}$

ERROR: code indent should never use tabs
#48: FILE: target/ppc/int_helper.c:540:
+^Iint i;$

ERROR: code indent should never use tabs
#49: FILE: target/ppc/int_helper.c:541:
+^Iuint64_t h64 = 0;$

ERROR: code indent should never use tabs
#50: FILE: target/ppc/int_helper.c:542:
+^Iuint64_t l64 = 0;$

ERROR: code indent should never use tabs
#52: FILE: target/ppc/int_helper.c:544:
+^Ifor (i = 0; i < 2; i++) {$

ERROR: code indent should never use tabs
#53: FILE: target/ppc/int_helper.c:545:
+^I^Imulu64(&l64, &h64, a->s64[i], b->s64[i]);$

ERROR: code indent should never use tabs
#54: FILE: target/ppc/int_helper.c:546:
+^I^Ir->u64[i] = h64;$

ERROR: code indent should never use tabs
#55: FILE: target/ppc/int_helper.c:547:
+^I}$

total: 14 errors, 0 warnings, 63 lines checked

Patch 4/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/6 Checking commit 966b641eba85 (fix the prototype of muls64/mulu64)
6/6 Checking commit d39f30e2a46f (target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions)
ERROR: code indent should never use tabs
#104: FILE: target/ppc/translate/vmx-impl.inc.c:832:
+^I^Ivdivsw, PPC_NONE, PPC2_ISA300, 0x00000000);$

ERROR: code indent should never use tabs
#107: FILE: target/ppc/translate/vmx-impl.inc.c:835:
+^I^Ivdivsd, PPC_NONE, PPC2_ISA300, 0x00000000);$

WARNING: line over 80 characters
#123: FILE: target/ppc/translate/vmx-ops.inc.c:54:
+#define GEN_VXFORM_DUAL_BOTH(name0, name1, opc2, opc3, inval0, inval1, type0, type1) \

ERROR: code indent should never use tabs
#136: FILE: target/ppc/translate/vmx-ops.inc.c:122:
+^I^I^IPPC_ALTIVEC, PPC2_ISA300),$

ERROR: code indent should never use tabs
#138: FILE: target/ppc/translate/vmx-ops.inc.c:124:
+^I^I^IPPC_ALTIVEC, PPC2_ISA300),$

total: 4 errors, 1 warnings, 108 lines checked

Patch 6/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200613042029.22321-1-ljp@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 0/6] Add several Power ISA 3.1 32/64-bit vector instructions

[David Gibson]

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

On Fri, Jun 12, 2020 at 09:47:58PM -0700, no-reply@patchew.org wrote:
> Patchew URL:
> https://patchew.org/QEMU/20200613042029.22321-1-ljp@linux.ibm.com/

I will need you to fix these stype errors and repost.

> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Message-id: 20200613042029.22321-1-ljp@linux.ibm.com
> Subject: [PATCH 0/6] Add several Power ISA 3.1 32/64-bit vector instructions
> Type: series
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
> Switched to a new branch 'test'
> d39f30e target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions
> 966b641 fix the prototype of muls64/mulu64
> 42111c5 target/ppc: add vmulh{su}d instructions
> c96e996 targetc/ppc: add vmulh{su}w instructions
> c52004c target/ppc: add vmulld instruction
> 1061e4e target/ppc: add byte-reverse br[dwh] instructions
> 
> === OUTPUT BEGIN ===
> 1/6 Checking commit 1061e4ead5bc (target/ppc: add byte-reverse br[dwh] instructions)
> ERROR: code indent should never use tabs
> #26: FILE: target/ppc/translate.c:6977:
> +^ITCGv_i64 temp = tcg_temp_new_i64();$
> 
> ERROR: code indent should never use tabs
> #28: FILE: target/ppc/translate.c:6979:
> +^Itcg_gen_bswap64_i64(temp, cpu_gpr[rS(ctx->opcode)]);$
> 
> WARNING: line over 80 characters
> #29: FILE: target/ppc/translate.c:6980:
> +       tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));
> 
> ERROR: code indent should never use tabs
> #29: FILE: target/ppc/translate.c:6980:
> +^Itcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));$
> 
> ERROR: code indent should never use tabs
> #31: FILE: target/ppc/translate.c:6982:
> +^Itcg_temp_free_i64(temp);$
> 
> ERROR: code indent should never use tabs
> #37: FILE: target/ppc/translate.c:6988:
> +^ITCGv_i64 temp = tcg_temp_new_i64();$
> 
> ERROR: code indent should never use tabs
> #38: FILE: target/ppc/translate.c:6989:
> +^ITCGv_i64 lsb = tcg_temp_new_i64();$
> 
> ERROR: code indent should never use tabs
> #39: FILE: target/ppc/translate.c:6990:
> +^ITCGv_i64 msb = tcg_temp_new_i64();$
> 
> ERROR: code indent should never use tabs
> #41: FILE: target/ppc/translate.c:6992:
> +^Itcg_gen_movi_i64(lsb, 0x00000000ffffffffull);$
> 
> ERROR: code indent should never use tabs
> #42: FILE: target/ppc/translate.c:6993:
> +^Itcg_gen_and_i64(temp, lsb, cpu_gpr[rS(ctx->opcode)]);$
> 
> ERROR: code indent should never use tabs
> #43: FILE: target/ppc/translate.c:6994:
> +^Itcg_gen_bswap32_i64(lsb, temp);$
> 
> ERROR: trailing whitespace
> #44: FILE: target/ppc/translate.c:6995:
> +^I$
> 
> ERROR: code indent should never use tabs
> #44: FILE: target/ppc/translate.c:6995:
> +^I$
> 
> ERROR: code indent should never use tabs
> #45: FILE: target/ppc/translate.c:6996:
> +^Itcg_gen_shri_i64(msb, cpu_gpr[rS(ctx->opcode)], 32);$
> 
> ERROR: code indent should never use tabs
> #46: FILE: target/ppc/translate.c:6997:
> +^Itcg_gen_bswap32_i64(temp, msb);$
> 
> ERROR: code indent should never use tabs
> #47: FILE: target/ppc/translate.c:6998:
> +^Itcg_gen_shli_i64(msb, temp, 32);$
> 
> ERROR: trailing whitespace
> #48: FILE: target/ppc/translate.c:6999:
> +^I$
> 
> ERROR: code indent should never use tabs
> #48: FILE: target/ppc/translate.c:6999:
> +^I$
> 
> ERROR: code indent should never use tabs
> #49: FILE: target/ppc/translate.c:7000:
> +^Itcg_gen_or_i64(temp, lsb, msb);$
> 
> WARNING: line over 80 characters
> #51: FILE: target/ppc/translate.c:7002:
> +       tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));
> 
> ERROR: code indent should never use tabs
> #51: FILE: target/ppc/translate.c:7002:
> +^Itcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));$
> 
> ERROR: code indent should never use tabs
> #53: FILE: target/ppc/translate.c:7004:
> +^Itcg_temp_free_i64(temp);$
> 
> ERROR: code indent should never use tabs
> #54: FILE: target/ppc/translate.c:7005:
> +^Itcg_temp_free_i64(lsb);$
> 
> ERROR: code indent should never use tabs
> #55: FILE: target/ppc/translate.c:7006:
> +^Itcg_temp_free_i64(msb);$
> 
> ERROR: code indent should never use tabs
> #61: FILE: target/ppc/translate.c:7012:
> +^ITCGv_i64 temp = tcg_temp_new_i64();$
> 
> ERROR: code indent should never use tabs
> #62: FILE: target/ppc/translate.c:7013:
> +^ITCGv_i64 t0 = tcg_temp_new_i64();$
> 
> ERROR: code indent should never use tabs
> #63: FILE: target/ppc/translate.c:7014:
> +^ITCGv_i64 t1 = tcg_temp_new_i64();$
> 
> ERROR: code indent should never use tabs
> #64: FILE: target/ppc/translate.c:7015:
> +^ITCGv_i64 t2 = tcg_temp_new_i64();$
> 
> ERROR: code indent should never use tabs
> #65: FILE: target/ppc/translate.c:7016:
> +^ITCGv_i64 t3 = tcg_temp_new_i64();$
> 
> ERROR: code indent should never use tabs
> #67: FILE: target/ppc/translate.c:7018:
> +^Itcg_gen_movi_i64(t0, 0x00ff00ff00ff00ffull);$
> 
> ERROR: code indent should never use tabs
> #68: FILE: target/ppc/translate.c:7019:
> +^Itcg_gen_shri_i64(t1, cpu_gpr[rS(ctx->opcode)], 8);$
> 
> ERROR: code indent should never use tabs
> #69: FILE: target/ppc/translate.c:7020:
> +^Itcg_gen_and_i64(t2, t1, t0);$
> 
> ERROR: code indent should never use tabs
> #70: FILE: target/ppc/translate.c:7021:
> +^Itcg_gen_and_i64(t1, cpu_gpr[rS(ctx->opcode)], t0);$
> 
> ERROR: code indent should never use tabs
> #71: FILE: target/ppc/translate.c:7022:
> +^Itcg_gen_shli_i64(t1, t1, 8);$
> 
> ERROR: code indent should never use tabs
> #72: FILE: target/ppc/translate.c:7023:
> +^Itcg_gen_or_i64(temp, t1, t2);$
> 
> WARNING: line over 80 characters
> #73: FILE: target/ppc/translate.c:7024:
> +       tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));
> 
> ERROR: code indent should never use tabs
> #73: FILE: target/ppc/translate.c:7024:
> +^Itcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));$
> 
> ERROR: code indent should never use tabs
> #75: FILE: target/ppc/translate.c:7026:
> +^Itcg_temp_free_i64(temp);$
> 
> ERROR: code indent should never use tabs
> #76: FILE: target/ppc/translate.c:7027:
> +^Itcg_temp_free_i64(t0);$
> 
> ERROR: code indent should never use tabs
> #77: FILE: target/ppc/translate.c:7028:
> +^Itcg_temp_free_i64(t1);$
> 
> ERROR: code indent should never use tabs
> #78: FILE: target/ppc/translate.c:7029:
> +^Itcg_temp_free_i64(t2);$
> 
> ERROR: code indent should never use tabs
> #79: FILE: target/ppc/translate.c:7030:
> +^Itcg_temp_free_i64(t3);$
> 
> total: 39 errors, 3 warnings, 69 lines checked
> 
> Patch 1/6 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 2/6 Checking commit c52004cd8f89 (target/ppc: add vmulld instruction)
> 3/6 Checking commit c96e996917c6 (targetc/ppc: add vmulh{su}w instructions)
> ERROR: code indent should never use tabs
> #34: FILE: target/ppc/int_helper.c:526:
> +#define VMULH_DO(name, op, element, cast_orig, cast_temp)^I^I\$
> 
> ERROR: code indent should never use tabs
> #35: FILE: target/ppc/int_helper.c:527:
> +    void helper_vmulh##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)^I\$
> 
> ERROR: code indent should never use tabs
> #36: FILE: target/ppc/int_helper.c:528:
> +    {^I^I^I^I^I^I^I^I^I\$
> 
> ERROR: code indent should never use tabs
> #37: FILE: target/ppc/int_helper.c:529:
> +^Iint i;^I^I^I^I^I^I^I^I\$
> 
> ERROR: code indent should never use tabs
> #38: FILE: target/ppc/int_helper.c:530:
> +^I^I^I^I^I^I^I^I^I\$
> 
> ERROR: code indent should never use tabs
> #39: FILE: target/ppc/int_helper.c:531:
> +^Ifor (i = 0; i < ARRAY_SIZE(r->element); i++) {^I^I^I\$
> 
> ERROR: code indent should never use tabs
> #40: FILE: target/ppc/int_helper.c:532:
> +^I^Ir->element[i] = (cast_orig)(((cast_temp)a->element[i] op \$
> 
> ERROR: code indent should never use tabs
> #41: FILE: target/ppc/int_helper.c:533:
> +^I^I^I^I(cast_temp)b->element[i]) >> 32);^I\$
> 
> ERROR: code indent should never use tabs
> #42: FILE: target/ppc/int_helper.c:534:
> +^I}^I^I^I^I^I^I^I^I\$
> 
> ERROR: code indent should never use tabs
> #61: FILE: target/ppc/translate/vmx-impl.inc.c:816:
> +^I^Ivmulhuw, PPC_NONE, PPC2_ISA300);$
> 
> ERROR: code indent should never use tabs
> #67: FILE: target/ppc/translate/vmx-impl.inc.c:822:
> +^I^Ivmulhsw, PPC_NONE, PPC2_ISA300);$
> 
> total: 11 errors, 0 warnings, 55 lines checked
> 
> Patch 3/6 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 4/6 Checking commit 42111c50379b (target/ppc: add vmulh{su}d instructions)
> ERROR: code indent should never use tabs
> #36: FILE: target/ppc/int_helper.c:528:
> +^Iint i;$
> 
> ERROR: code indent should never use tabs
> #37: FILE: target/ppc/int_helper.c:529:
> +^Iuint64_t h64 = 0;$
> 
> ERROR: code indent should never use tabs
> #38: FILE: target/ppc/int_helper.c:530:
> +^Iuint64_t l64 = 0;$
> 
> ERROR: code indent should never use tabs
> #40: FILE: target/ppc/int_helper.c:532:
> +^Ifor (i = 0; i < 2; i++) {$
> 
> ERROR: code indent should never use tabs
> #41: FILE: target/ppc/int_helper.c:533:
> +^I^Imuls64(&l64, &h64, a->s64[i], b->s64[i]);$
> 
> ERROR: code indent should never use tabs
> #42: FILE: target/ppc/int_helper.c:534:
> +^I^Ir->s64[i] = h64;$
> 
> ERROR: code indent should never use tabs
> #43: FILE: target/ppc/int_helper.c:535:
> +^I}$
> 
> ERROR: code indent should never use tabs
> #48: FILE: target/ppc/int_helper.c:540:
> +^Iint i;$
> 
> ERROR: code indent should never use tabs
> #49: FILE: target/ppc/int_helper.c:541:
> +^Iuint64_t h64 = 0;$
> 
> ERROR: code indent should never use tabs
> #50: FILE: target/ppc/int_helper.c:542:
> +^Iuint64_t l64 = 0;$
> 
> ERROR: code indent should never use tabs
> #52: FILE: target/ppc/int_helper.c:544:
> +^Ifor (i = 0; i < 2; i++) {$
> 
> ERROR: code indent should never use tabs
> #53: FILE: target/ppc/int_helper.c:545:
> +^I^Imulu64(&l64, &h64, a->s64[i], b->s64[i]);$
> 
> ERROR: code indent should never use tabs
> #54: FILE: target/ppc/int_helper.c:546:
> +^I^Ir->u64[i] = h64;$
> 
> ERROR: code indent should never use tabs
> #55: FILE: target/ppc/int_helper.c:547:
> +^I}$
> 
> total: 14 errors, 0 warnings, 63 lines checked
> 
> Patch 4/6 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 5/6 Checking commit 966b641eba85 (fix the prototype of muls64/mulu64)
> 6/6 Checking commit d39f30e2a46f (target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions)
> ERROR: code indent should never use tabs
> #104: FILE: target/ppc/translate/vmx-impl.inc.c:832:
> +^I^Ivdivsw, PPC_NONE, PPC2_ISA300, 0x00000000);$
> 
> ERROR: code indent should never use tabs
> #107: FILE: target/ppc/translate/vmx-impl.inc.c:835:
> +^I^Ivdivsd, PPC_NONE, PPC2_ISA300, 0x00000000);$
> 
> WARNING: line over 80 characters
> #123: FILE: target/ppc/translate/vmx-ops.inc.c:54:
> +#define GEN_VXFORM_DUAL_BOTH(name0, name1, opc2, opc3, inval0, inval1, type0, type1) \
> 
> ERROR: code indent should never use tabs
> #136: FILE: target/ppc/translate/vmx-ops.inc.c:122:
> +^I^I^IPPC_ALTIVEC, PPC2_ISA300),$
> 
> ERROR: code indent should never use tabs
> #138: FILE: target/ppc/translate/vmx-ops.inc.c:124:
> +^I^I^IPPC_ALTIVEC, PPC2_ISA300),$
> 
> total: 4 errors, 1 warnings, 108 lines checked
> 
> Patch 6/6 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> The full log is available at
> http://patchew.org/logs/20200613042029.22321-1-ljp@linux.ibm.com/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com

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

Re: [PATCH 0/6] Add several Power ISA 3.1 32/64-bit vector instructions

[Cédric Le Goater]

Hello,

On 6/13/20 6:20 AM, Lijun Pan wrote:
> This patch series add several newly introduced 32/64-bit vector
> instructions in Power ISA 3.1. The newly added instructions are
> flagged as ISA300 temporarily in vmx-ops.inc.c and vmx-impl.inc.c
> to make them compile and function since Power ISA 3.1, together
> with next generation processor, has not been fully enabled in QEMU
> yet. When Power ISA 3.1 and next generation processor are fully
> supported, the flags should be changed.

What do you mean ? 

ISA 3.1 and POWER10 are merged in Linux and in the QEMU pseries 
and PowerNV (OPAL) machines.

It's very much empty but it's there.

C. 


> 
> Lijun Pan (6):
>   target/ppc: add byte-reverse br[dwh] instructions
>   target/ppc: add vmulld instruction
>   targetc/ppc: add vmulh{su}w instructions
>   target/ppc: add vmulh{su}d instructions
>   fix the prototype of muls64/mulu64
>   target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions
> 
>  include/qemu/host-utils.h           |  4 +-
>  target/ppc/helper.h                 | 13 ++++++
>  target/ppc/int_helper.c             | 58 +++++++++++++++++++++++++
>  target/ppc/translate.c              | 65 +++++++++++++++++++++++++++++
>  target/ppc/translate/vmx-impl.inc.c | 24 +++++++++++
>  target/ppc/translate/vmx-ops.inc.c  | 22 ++++++++--
>  6 files changed, 180 insertions(+), 6 deletions(-)
> 

Re: [PATCH 0/6] Add several Power ISA 3.1 32/64-bit vector instructions

[Lijun Pan]

> On Jun 15, 2020, at 12:36 PM, Cédric Le Goater <clg@kaod.org> wrote:
> 
> Hello,
> 
> On 6/13/20 6:20 AM, Lijun Pan wrote:
>> This patch series add several newly introduced 32/64-bit vector
>> instructions in Power ISA 3.1. The newly added instructions are
>> flagged as ISA300 temporarily in vmx-ops.inc.c and vmx-impl.inc.c
>> to make them compile and function since Power ISA 3.1, together
>> with next generation processor, has not been fully enabled in QEMU
>> yet. When Power ISA 3.1 and next generation processor are fully
>> supported, the flags should be changed.
> 
> What do you mean ? 
> 
> ISA 3.1 and POWER10 are merged in Linux and in the QEMU pseries 
> and PowerNV (OPAL) machines.
> 
> It's very much empty but it's there.

I mean it does not work if you boots the guest in TCG mode, not KVM mode.

Lijun

> 
> C. 
> 
> 
>> 
>> Lijun Pan (6):
>>  target/ppc: add byte-reverse br[dwh] instructions
>>  target/ppc: add vmulld instruction
>>  targetc/ppc: add vmulh{su}w instructions
>>  target/ppc: add vmulh{su}d instructions
>>  fix the prototype of muls64/mulu64
>>  target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions
>> 
>> include/qemu/host-utils.h           |  4 +-
>> target/ppc/helper.h                 | 13 ++++++
>> target/ppc/int_helper.c             | 58 +++++++++++++++++++++++++
>> target/ppc/translate.c              | 65 +++++++++++++++++++++++++++++
>> target/ppc/translate/vmx-impl.inc.c | 24 +++++++++++
>> target/ppc/translate/vmx-ops.inc.c  | 22 ++++++++--
>> 6 files changed, 180 insertions(+), 6 deletions(-)
>> 
> 

Re: [PATCH 0/6] Add several Power ISA 3.1 32/64-bit vector instructions

[Cédric Le Goater]

On 6/15/20 10:54 PM, Lijun Pan wrote:
> 
> 
>> On Jun 15, 2020, at 12:36 PM, Cédric Le Goater <clg@kaod.org> wrote:
>>
>> Hello,
>>
>> On 6/13/20 6:20 AM, Lijun Pan wrote:
>>> This patch series add several newly introduced 32/64-bit vector
>>> instructions in Power ISA 3.1. The newly added instructions are
>>> flagged as ISA300 temporarily in vmx-ops.inc.c and vmx-impl.inc.c
>>> to make them compile and function since Power ISA 3.1, together
>>> with next generation processor, has not been fully enabled in QEMU
>>> yet. When Power ISA 3.1 and next generation processor are fully
>>> supported, the flags should be changed.
>>
>> What do you mean ? 
>>
>> ISA 3.1 and POWER10 are merged in Linux and in the QEMU pseries 
>> and PowerNV (OPAL) machines.
>>
>> It's very much empty but it's there.
> 
> I mean it does not work if you boots the guest in TCG mode, not KVM mode.

Something is wrong in your environment. With the latest QEMU and
Linux 5.8-rc1, here is a POWER10 pseries machine : 
    
    BusyBox v1.30.1 (Ubuntu 1:1.30.1-4ubuntu4) built-in shell (ash)
    Enter 'help' for a list of built-in commands.
    
    (initramfs) 
    (initramfs) cat /proc/cpuinfo 
    processor	: 0
    cpu		: POWER10 (architected), altivec supported
    clock	: 1000.000000MHz
    revision	: 1.0 (pvr 0080 0100)
    
    processor	: 1
    cpu		: POWER10 (architected), altivec supported
    clock	: 1000.000000MHz
    revision	: 1.0 (pvr 0080 0100)
    
    processor	: 2
    cpu		: POWER10 (architected), altivec supported
    clock	: 1000.000000MHz
    revision	: 1.0 (pvr 0080 0100)
    
    processor	: 3
    cpu		: POWER10 (architected), altivec supported
    clock	: 1000.000000MHz
    revision	: 1.0 (pvr 0080 0100)
    
    timebase	: 512000000
    platform	: pSeries
    model	: IBM pSeries (emulated by qemu)
    machine	: CHRP IBM pSeries (emulated by qemu)
    MMU		: Radix
    (initramfs) uname -a 
    Linux (none) 5.8.0-rc1+ #199 SMP Tue Jun 16 07:54:06 CEST 2020 ppc64le GNU/Linux



KVM works also but you will need to run the guest under a QEMU
PowerNV machine using my powernv-5.1 branch. All support for
POWER10 baremetal is not merged yet.

Thanks,

C.

Re: [PATCH 1/6] target/ppc: add byte-reverse br[dwh] instructions

[Richard Henderson]

On 6/12/20 9:20 PM, Lijun Pan wrote:
> POWER ISA 3.1 introduces following byte-reverse instructions:
> brd: Byte-Reverse Doubleword X-form
> brw: Byte-Reverse Word X-form
> brh: Byte-Reverse Halfword X-form
> 
> Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
> ---
>  target/ppc/translate.c | 62 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 4ce3d664b5..2d48fbc8db 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6971,7 +6971,69 @@ static void gen_dform3D(DisasContext *ctx)
>      return gen_invalid(ctx);
>  }
>  
> +/* brd */
> +static void gen_brd(DisasContext *ctx)
> +{
> +	TCGv_i64 temp = tcg_temp_new_i64();
> +
> +	tcg_gen_bswap64_i64(temp, cpu_gpr[rS(ctx->opcode)]);
> +	tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));


The store is wrong.  You cannot modify storage behind a tcg global variable
like that.  This should just be

    tcg_gen_bswap64_i64(cpu_gpr[rA(ctx->opcode)],
                        cpu_gpr[rS(ctx->opcode)]);

Is this code is within an ifdef for TARGET_PPC64?
If not, then this will break the 32-bit qemu-system-ppc build.
Are you sure you have built and tested all configurations?


> +/* brw */
> +static void gen_brw(DisasContext *ctx)
> +{
> +	TCGv_i64 temp = tcg_temp_new_i64();
> +	TCGv_i64 lsb = tcg_temp_new_i64();
> +	TCGv_i64 msb = tcg_temp_new_i64();
> +
> +	tcg_gen_movi_i64(lsb, 0x00000000ffffffffull);
> +	tcg_gen_and_i64(temp, lsb, cpu_gpr[rS(ctx->opcode)]);
> +	tcg_gen_bswap32_i64(lsb, temp);
> +	
> +	tcg_gen_shri_i64(msb, cpu_gpr[rS(ctx->opcode)], 32);
> +	tcg_gen_bswap32_i64(temp, msb);
> +	tcg_gen_shli_i64(msb, temp, 32);
> +	
> +	tcg_gen_or_i64(temp, lsb, msb);
> +
> +	tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));

Again, the store is wrong.

In addition, this can be computed as

    tcg_gen_bswap64_i64(dest, source);
    tcg_gen_rotli_i64(dest, dest, 32);

> +static void gen_brh(DisasContext *ctx)
> +{
> +	TCGv_i64 temp = tcg_temp_new_i64();
> +	TCGv_i64 t0 = tcg_temp_new_i64();
> +	TCGv_i64 t1 = tcg_temp_new_i64();
> +	TCGv_i64 t2 = tcg_temp_new_i64();
> +	TCGv_i64 t3 = tcg_temp_new_i64();
> +
> +	tcg_gen_movi_i64(t0, 0x00ff00ff00ff00ffull);
> +	tcg_gen_shri_i64(t1, cpu_gpr[rS(ctx->opcode)], 8);
> +	tcg_gen_and_i64(t2, t1, t0);
> +	tcg_gen_and_i64(t1, cpu_gpr[rS(ctx->opcode)], t0);
> +	tcg_gen_shli_i64(t1, t1, 8);
> +	tcg_gen_or_i64(temp, t1, t2);
> +	tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));

Again, the store is wrong.


r~

Re: [PATCH 2/6] target/ppc: add vmulld instruction

[Richard Henderson]

On 6/12/20 9:20 PM, Lijun Pan wrote:
> vmulld: Vector Multiply Low Doubleword.
> 
> Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
> ---
>  target/ppc/helper.h                 | 1 +
>  target/ppc/int_helper.c             | 1 +
>  target/ppc/translate/vmx-impl.inc.c | 1 +
>  target/ppc/translate/vmx-ops.inc.c  | 1 +
>  4 files changed, 4 insertions(+)
> 
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 2dfa1c6942..c3f087ccb3 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -185,6 +185,7 @@ DEF_HELPER_3(vmuloub, void, avr, avr, avr)
>  DEF_HELPER_3(vmulouh, void, avr, avr, avr)
>  DEF_HELPER_3(vmulouw, void, avr, avr, avr)
>  DEF_HELPER_3(vmuluwm, void, avr, avr, avr)
> +DEF_HELPER_3(vmulld, void, avr, avr, avr)
>  DEF_HELPER_3(vslo, void, avr, avr, avr)
>  DEF_HELPER_3(vsro, void, avr, avr, avr)
>  DEF_HELPER_3(vsrv, void, avr, avr, avr)
> diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
> index be53cd6f68..afbcdd05b4 100644
> --- a/target/ppc/int_helper.c
> +++ b/target/ppc/int_helper.c
> @@ -533,6 +533,7 @@ void helper_vprtybq(ppc_avr_t *r, ppc_avr_t *b)
>          }                                                               \
>      }
>  VARITH_DO(muluwm, *, u32)
> +VARITH_DO(mulld, *, s64)

>From this implementation, I would say that both vmuluwm and vmulld can be
implemented with tcg_gen_gvec_mul().

I guess vmuluwm was missed when many of the other vmx operations were converted
to gvec.

Please first convert vmuluwm to tcg_gen_gvec_mul, then implement vmulld in the
same manner.


r~

Re: [PATCH 3/6] targetc/ppc: add vmulh{su}w instructions

[Richard Henderson]

On 6/12/20 9:20 PM, Lijun Pan wrote:
> +#define VMULH_DO(name, op, element, cast_orig, cast_temp)		\
> +    void helper_vmulh##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)	\
> +    {									\
> +	int i;								\
> +									\
> +	for (i = 0; i < ARRAY_SIZE(r->element); i++) {			\
> +		r->element[i] = (cast_orig)(((cast_temp)a->element[i] op \
> +				(cast_temp)b->element[i]) >> 32);	\
> +	}								\
> +    }
> +VMULH_DO(sw, *, s32, int32_t, int64_t)
> +VMULH_DO(uw, *, u32, uint32_t, uint64_t)
> +#undef VMULH_DO

There's no point in calling the macro "VMUL" and then passing in "op" as a
parameter.  Just inline the multiply directly.

Also, fix your indentation.


r~

Re: [PATCH 4/6] target/ppc: add vmulh{su}d instructions

[Richard Henderson]

On 6/12/20 9:20 PM, Lijun Pan wrote:
> +void helper_vmulhsd(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
> +{
> +	int i;
> +	uint64_t h64 = 0;
> +	uint64_t l64 = 0;
> +
> +	for (i = 0; i < 2; i++) {
> +		muls64(&l64, &h64, a->s64[i], b->s64[i]);
> +		r->s64[i] = h64;
> +	}
> +}

Indentation is off.

This can just as easily be written as

    uint64_t discard;

    muls64(&discard, &r->u64[0], a->s64[0], b->s64[0]);
    muls64(&discard, &r->u64[1], a->s64[1], b->s64[1]);

and similarly for helper_vmulhud.


r~

Re: [PATCH 5/6] fix the prototype of muls64/mulu64

[Richard Henderson]

On 6/12/20 9:20 PM, Lijun Pan wrote:
> The prototypes of muls64/mulu64 in host-utils.h should match the
> definitions in host-utils.c
> 
> Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
> ---
>  include/qemu/host-utils.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
CC: qemu-trivial.


r~

Re: [PATCH 6/6] target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions

[Richard Henderson]

On 6/12/20 9:20 PM, Lijun Pan wrote:
> +#define VDIV_MOD_DO(name, op, element)                                  \
> +    void helper_v##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)       \
> +    {                                                                   \
> +        int i;                                                          \
> +                                                                        \
> +        for (i = 0; i < ARRAY_SIZE(r->element); i++) {                  \
> +            r->element[i] = a->element[i] op b->element[i];             \
> +        }                                                               \
> +    }

You're missing all of the divide-by-zero handling.


r~

Re: [PATCH 0/6] Add several Power ISA 3.1 32/64-bit vector instructions

[Richard Henderson]

On 6/12/20 9:20 PM, Lijun Pan wrote:
> This patch series add several newly introduced 32/64-bit vector
> instructions in Power ISA 3.1. The newly added instructions are
> flagged as ISA300 temporarily in vmx-ops.inc.c and vmx-impl.inc.c
> to make them compile and function since Power ISA 3.1, together
> with next generation processor, has not been fully enabled in QEMU
> yet. When Power ISA 3.1 and next generation processor are fully
> supported, the flags should be changed.

This is not the correct procedure.

Step 1 is to add a new define for ISA301, which is not enabled for any processor.

Step 2 is to add all of the new instructions, using ISA301.  In this way there
is no intermediate point in which a 3.01 instruction is enabled for 3.00.  In
addition, we do not have extra churn simply to change the ISA.

Step 3 is to add a new processor for which ISA301 is set.  It is often
reasonable to have a fake processor named "max" that contains all of the
available features.  For ppc, I see that "max" is currently aliased to "7400_v2.9".


r~

Re: [PATCH 1/6] target/ppc: add byte-reverse br[dwh] instructions

[Lijun Pan]

> On Jun 18, 2020, at 6:19 PM, Richard Henderson <richard.henderson@linaro.org> wrote:
> 
> On 6/12/20 9:20 PM, Lijun Pan wrote:
>> POWER ISA 3.1 introduces following byte-reverse instructions:
>> brd: Byte-Reverse Doubleword X-form
>> brw: Byte-Reverse Word X-form
>> brh: Byte-Reverse Halfword X-form
>> 
>> Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
>> ---
>> target/ppc/translate.c | 62 ++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 62 insertions(+)
>> 
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index 4ce3d664b5..2d48fbc8db 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -6971,7 +6971,69 @@ static void gen_dform3D(DisasContext *ctx)
>>     return gen_invalid(ctx);
>> }
>> 
>> +/* brd */
>> +static void gen_brd(DisasContext *ctx)
>> +{
>> +	TCGv_i64 temp = tcg_temp_new_i64();
>> +
>> +	tcg_gen_bswap64_i64(temp, cpu_gpr[rS(ctx->opcode)]);
>> +	tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));
> 
> 
> The store is wrong.  You cannot modify storage behind a tcg global variable
> like that.  This should just be
> 
>    tcg_gen_bswap64_i64(cpu_gpr[rA(ctx->opcode)],
>                        cpu_gpr[rS(ctx->opcode)]);

Why can’t I retrieve the offset via “offsetof(CPUPPCState, gpr[rA(ctx->opcode)])”?
I would like to learn more.

> Is this code is within an ifdef for TARGET_PPC64?

I will change it to 
+#if defined(TARGET_PPC64)
+GEN_HANDLER_E(brd, 0x1F, 0x1B, 0x05, 0x0000F801, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(brw, 0x1F, 0x1B, 0x04, 0x0000F801, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(brh, 0x1F, 0x1B, 0x06, 0x0000F801, PPC_NONE, PPC2_ISA300),
+#endif

> If not, then this will break the 32-bit qemu-system-ppc build.
> Are you sure you have built and tested all configurations?
> 
> 
>> +/* brw */
>> +static void gen_brw(DisasContext *ctx)
>> +{
>> +	TCGv_i64 temp = tcg_temp_new_i64();
>> +	TCGv_i64 lsb = tcg_temp_new_i64();
>> +	TCGv_i64 msb = tcg_temp_new_i64();
>> +
>> +	tcg_gen_movi_i64(lsb, 0x00000000ffffffffull);
>> +	tcg_gen_and_i64(temp, lsb, cpu_gpr[rS(ctx->opcode)]);
>> +	tcg_gen_bswap32_i64(lsb, temp);
>> +	
>> +	tcg_gen_shri_i64(msb, cpu_gpr[rS(ctx->opcode)], 32);
>> +	tcg_gen_bswap32_i64(temp, msb);
>> +	tcg_gen_shli_i64(msb, temp, 32);
>> +	
>> +	tcg_gen_or_i64(temp, lsb, msb);
>> +
>> +	tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));
> 
> Again, the store is wrong.
> 
> In addition, this can be computed as
> 
>    tcg_gen_bswap64_i64(dest, source);
>    tcg_gen_rotli_i64(dest, dest, 32);
> 
>> +static void gen_brh(DisasContext *ctx)
>> +{
>> +	TCGv_i64 temp = tcg_temp_new_i64();
>> +	TCGv_i64 t0 = tcg_temp_new_i64();
>> +	TCGv_i64 t1 = tcg_temp_new_i64();
>> +	TCGv_i64 t2 = tcg_temp_new_i64();
>> +	TCGv_i64 t3 = tcg_temp_new_i64();
>> +
>> +	tcg_gen_movi_i64(t0, 0x00ff00ff00ff00ffull);
>> +	tcg_gen_shri_i64(t1, cpu_gpr[rS(ctx->opcode)], 8);
>> +	tcg_gen_and_i64(t2, t1, t0);
>> +	tcg_gen_and_i64(t1, cpu_gpr[rS(ctx->opcode)], t0);
>> +	tcg_gen_shli_i64(t1, t1, 8);
>> +	tcg_gen_or_i64(temp, t1, t2);
>> +	tcg_gen_st_i64(temp, cpu_env, offsetof(CPUPPCState, gpr[rA(ctx->opcode)]));
> 
> Again, the store is wrong.
> 
> 
> r~
> 

Re: [PATCH 2/6] target/ppc: add vmulld instruction

[Lijun Pan]

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



> On Jun 18, 2020, at 6:27 PM, Richard Henderson <richard.henderson@linaro.org> wrote:
> 
> On 6/12/20 9:20 PM, Lijun Pan wrote:
>> vmulld: Vector Multiply Low Doubleword.
>> 
>> Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
>> ---
>> target/ppc/helper.h                 | 1 +
>> target/ppc/int_helper.c             | 1 +
>> target/ppc/translate/vmx-impl.inc.c | 1 +
>> target/ppc/translate/vmx-ops.inc.c  | 1 +
>> 4 files changed, 4 insertions(+)
>> 
>> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
>> index 2dfa1c6942..c3f087ccb3 100644
>> --- a/target/ppc/helper.h
>> +++ b/target/ppc/helper.h
>> @@ -185,6 +185,7 @@ DEF_HELPER_3(vmuloub, void, avr, avr, avr)
>> DEF_HELPER_3(vmulouh, void, avr, avr, avr)
>> DEF_HELPER_3(vmulouw, void, avr, avr, avr)
>> DEF_HELPER_3(vmuluwm, void, avr, avr, avr)
>> +DEF_HELPER_3(vmulld, void, avr, avr, avr)
>> DEF_HELPER_3(vslo, void, avr, avr, avr)
>> DEF_HELPER_3(vsro, void, avr, avr, avr)
>> DEF_HELPER_3(vsrv, void, avr, avr, avr)
>> diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
>> index be53cd6f68..afbcdd05b4 100644
>> --- a/target/ppc/int_helper.c
>> +++ b/target/ppc/int_helper.c
>> @@ -533,6 +533,7 @@ void helper_vprtybq(ppc_avr_t *r, ppc_avr_t *b)
>>         }                                                               \
>>     }
>> VARITH_DO(muluwm, *, u32)
>> +VARITH_DO(mulld, *, s64)
> 
>> From this implementation, I would say that both vmuluwm and vmulld can be
> implemented with tcg_gen_gvec_mul().
> 
> I guess vmuluwm was missed when many of the other vmx operations were converted
> to gvec.
> 
> Please first convert vmuluwm to tcg_gen_gvec_mul, then implement vmulld in the
> same manner.

I did a grep in git repo, and found out only arm use this tcg_gen_gvec_mul.
The original implementation is very straightforward, and being adopted at many places
all over target/ppc/int_helper.c. Why do we need to convert
to tcg_gen_gvec_mul, which seems to me very convoluted?

Thanks,
Lijun

[-- Attachment #2: Type: text/html, Size: 9470 bytes --]

Re: [PATCH 3/6] targetc/ppc: add vmulh{su}w instructions

[Lijun Pan]

> On Jun 18, 2020, at 6:29 PM, Richard Henderson <richard.henderson@linaro.org> wrote:
> 
> On 6/12/20 9:20 PM, Lijun Pan wrote:
>> +#define VMULH_DO(name, op, element, cast_orig, cast_temp)		\
>> +    void helper_vmulh##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)	\
>> +    {									\
>> +	int i;								\
>> +									\
>> +	for (i = 0; i < ARRAY_SIZE(r->element); i++) {			\
>> +		r->element[i] = (cast_orig)(((cast_temp)a->element[i] op \
>> +				(cast_temp)b->element[i]) >> 32);	\
>> +	}								\
>> +    }
>> +VMULH_DO(sw, *, s32, int32_t, int64_t)
>> +VMULH_DO(uw, *, u32, uint32_t, uint64_t)
>> +#undef VMULH_DO
> 
> There's no point in calling the macro "VMUL" and then passing in "op" as a
> parameter.  Just inline the multiply directly.

Do you mean writing two functions directly, 

void helper_vmulhsw(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
{
    int i;

    for (i = 0; i < 4; i++) {
        r->s32[i] = (int32_t)((int64_t)a->s32[i] * (int64_t)b->s32[i]) >> 32);
    }
}

void helper_vmulhuw(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
{
    int i;

    for (i = 0; i < 4; i++) {
        r->u32[i] = (uint32_t)((uint64_t)a->u32[i] * (uint64_t)b->u32[i]) >> 32);
    }
}

Thanks,
Lijun

Re: [PATCH 1/6] target/ppc: add byte-reverse br[dwh] instructions

[Richard Henderson]

On 6/18/20 10:24 PM, Lijun Pan wrote:
> Why can’t I retrieve the offset via “offsetof(CPUPPCState,gpr[rA(ctx->opcode)])”?
> I would like to learn more.

The TCG compiler makes some simplifying assumptions in order to make it faster.
 One of them is that global temporaries cannot be modified via direct loads and
stores, so we do not have to check for that overlap during compilation.

I thought that was documented in tcg/README, but I can't find it.


r~

Re: [PATCH 2/6] target/ppc: add vmulld instruction

[Richard Henderson]

On 6/18/20 10:30 PM, Lijun Pan wrote:
> Why do we need to convert
> to tcg_gen_gvec_mul, which seems to me very convoluted?

Because that way we can generate a single host vector multiply instruction in
the compiled translation block.


r~

Re: [PATCH 3/6] targetc/ppc: add vmulh{su}w instructions

[Richard Henderson]

On 6/18/20 10:37 PM, Lijun Pan wrote:
> Do you mean writing two functions directly, 
> 
> void helper_vmulhsw(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
> {
>     int i;
> 
>     for (i = 0; i < 4; i++) {
>         r->s32[i] = (int32_t)((int64_t)a->s32[i] * (int64_t)b->s32[i]) >> 32);
>     }
> }
> 
> void helper_vmulhuw(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
> {
>     int i;
> 
>     for (i = 0; i < 4; i++) {
>         r->u32[i] = (uint32_t)((uint64_t)a->u32[i] * (uint64_t)b->u32[i]) >> 32);
>     }
> }

That works for me.


r~