* [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I
@ 2016-10-26 13:18 Jose Ricardo Ziviani
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction Jose Ricardo Ziviani
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Jose Ricardo Ziviani @ 2016-10-26 13:18 UTC (permalink / raw)
To: qemu-ppc; +Cc: qemu-devel, david, nikunj, bharata
This serie contains 4 new instructions for POWER9 ISA3.0
bcdcfn.: Decimal Convert From National
bcdctn.: Decimal Convert To National
bcdcfz.: Decimal Convert From Zoned
bcdctz.: Decimal Convert to Zoned
Jose Ricardo Ziviani (4):
target-ppc: Implement bcdcfn. instruction
target-ppc: Implement bcdctn. instruction
target-ppc: Implement bcdcfz. instruction
target-ppc: Implement bcdctz. instruction
target-ppc/helper.h | 4 +
target-ppc/int_helper.c | 209 ++++++++++++++++++++++++++++++++++++
target-ppc/translate/vmx-impl.inc.c | 103 ++++++++++++++++++
target-ppc/translate/vmx-ops.inc.c | 4 +-
4 files changed, 318 insertions(+), 2 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction
2016-10-26 13:18 [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I Jose Ricardo Ziviani
@ 2016-10-26 13:18 ` Jose Ricardo Ziviani
2016-10-27 1:05 ` David Gibson
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction Jose Ricardo Ziviani
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Jose Ricardo Ziviani @ 2016-10-26 13:18 UTC (permalink / raw)
To: qemu-ppc; +Cc: qemu-devel, david, nikunj, bharata
bcdcfn. converts from National numeric format to BCD. National format
uses a byte to represent a digit where the most significant nibble is
always 0x3 and the least sign. nibbles is the digit itself.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
---
target-ppc/helper.h | 1 +
target-ppc/int_helper.c | 54 ++++++++++++++++++++++++++
target-ppc/translate/vmx-impl.inc.c | 75 +++++++++++++++++++++++++++++++++++++
target-ppc/translate/vmx-ops.inc.c | 4 +-
4 files changed, 132 insertions(+), 2 deletions(-)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 04c6421..d30ec60 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -369,6 +369,7 @@ DEF_HELPER_4(vpermxor, void, avr, avr, avr, avr)
DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32)
DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
+DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
DEF_HELPER_2(xsadddp, void, env, i32)
DEF_HELPER_2(xssubdp, void, env, i32)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 5aee0a8..494c74e 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -2417,6 +2417,8 @@ void helper_vsubecuq(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)
#define BCD_NEG_PREF 0xD
#define BCD_NEG_ALT 0xB
#define BCD_PLUS_ALT_2 0xE
+#define NATIONAL_PLUS 0x2B
+#define NATIONAL_NEG 0x2D
#if defined(HOST_WORDS_BIGENDIAN)
#define BCD_DIG_BYTE(n) (15 - (n/2))
@@ -2483,6 +2485,15 @@ static void bcd_put_digit(ppc_avr_t *bcd, uint8_t digit, int n)
}
}
+static uint8_t get_national_digit(ppc_avr_t *reg, int n)
+{
+#if defined(HOST_WORDS_BIGENDIAN)
+ return reg->u16[8 - n] & 0xFF;
+#else
+ return reg->u16[n] & 0xFF;
+#endif
+}
+
static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b)
{
int i;
@@ -2613,6 +2624,49 @@ uint32_t helper_bcdsub(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, uint32_t ps)
return helper_bcdadd(r, a, &bcopy, ps);
}
+uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
+{
+ int i;
+ int is_zero = 0;
+ int cr = 0;
+ int national = 0;
+ ppc_avr_t ret = { .u64 = { 0, 0 } };
+ uint16_t sgnb = get_national_digit(b, 0);
+ int invalid = (sgnb != NATIONAL_PLUS && sgnb != NATIONAL_NEG);
+
+ for (i = 1; i < 8; i++) {
+ national = get_national_digit(b, i);
+
+ is_zero += (national == 0x30);
+ if (unlikely(national < 0x30 || national > 0x39)) {
+ invalid = 1;
+ }
+
+ bcd_put_digit(&ret, national & 0xf, i);
+ }
+
+ if (sgnb == NATIONAL_PLUS ||
+ (b->u64[0] == 0 && b->u64[1] == 0)) {
+ bcd_put_digit(&ret, (ps == 0) ? BCD_PLUS_PREF_1 : BCD_PLUS_PREF_2, 0);
+ } else {
+ bcd_put_digit(&ret, BCD_NEG_PREF, 0);
+ }
+
+ if (!is_zero) {
+ cr = (sgnb == NATIONAL_PLUS) ? 1 << CRF_GT : 1 << CRF_LT;
+ } else {
+ cr = 1 << CRF_EQ;
+ }
+
+ if (unlikely(invalid)) {
+ cr = 1 << CRF_SO;
+ }
+
+ *r = ret;
+
+ return cr;
+}
+
void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
{
int i;
diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
index c8998f3..2abdcac 100644
--- a/target-ppc/translate/vmx-impl.inc.c
+++ b/target-ppc/translate/vmx-impl.inc.c
@@ -871,8 +871,81 @@ static void gen_##op(DisasContext *ctx) \
tcg_temp_free_i32(ps); \
}
+#define GEN_BCD2(op) \
+static void gen_##op(DisasContext *ctx) \
+{ \
+ TCGv_ptr rd, rb; \
+ TCGv_i32 ps; \
+ \
+ if (unlikely(!ctx->altivec_enabled)) { \
+ gen_exception(ctx, POWERPC_EXCP_VPU); \
+ return; \
+ } \
+ \
+ rb = gen_avr_ptr(rB(ctx->opcode)); \
+ rd = gen_avr_ptr(rD(ctx->opcode)); \
+ \
+ ps = tcg_const_i32((ctx->opcode & 0x200) != 0); \
+ \
+ gen_helper_##op(cpu_crf[6], rd, rb, ps); \
+ \
+ tcg_temp_free_ptr(rb); \
+ tcg_temp_free_ptr(rd); \
+ tcg_temp_free_i32(ps); \
+}
+
GEN_BCD(bcdadd)
GEN_BCD(bcdsub)
+GEN_BCD2(bcdcfn)
+
+static void gen_xpnd04_1(DisasContext *ctx)
+{
+ switch (opc4(ctx->opcode)) {
+ case 0:
+ break; /* bcdctsq. */
+ case 2:
+ break; /* bcdcfsq. */
+ case 4:
+ break; /* bcdctz. */
+ case 5:
+ break; /* bcdctn. */
+ case 6:
+ break; /* bcdcfz. */
+ case 7:
+ gen_bcdcfn(ctx);
+ break;
+ case 31:
+ break; /* bcdsetsgn. */
+ default:
+ break;
+ }
+}
+
+static void gen_xpnd04_2(DisasContext *ctx)
+{
+ switch (opc4(ctx->opcode)) {
+ case 0:
+ break; /* bcdctsq. */
+ case 2:
+ break; /* bcdcfsq. */
+ case 4:
+ break; /* bcdctz. */
+ case 6:
+ break; /* bcdcfz. */
+ case 7:
+ gen_bcdcfn(ctx);
+ break;
+ case 31:
+ break; /* bcdsetsgn. */
+ default:
+ break;
+ }
+}
+
+GEN_VXFORM_DUAL(vsubcuw, PPC_ALTIVEC, PPC_NONE, \
+ xpnd04_1, PPC_NONE, PPC2_ISA300)
+GEN_VXFORM_DUAL(vsubsws, PPC_ALTIVEC, PPC_NONE, \
+ xpnd04_2, PPC_NONE, PPC2_ISA300)
GEN_VXFORM_DUAL(vsububm, PPC_ALTIVEC, PPC_NONE, \
bcdadd, PPC_NONE, PPC2_ALTIVEC_207)
@@ -949,3 +1022,5 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE,
#undef GEN_VXFORM_NOA
#undef GEN_VXFORM_UIMM
#undef GEN_VAFORM_PAIRED
+
+#undef GEN_BCD2
diff --git a/target-ppc/translate/vmx-ops.inc.c b/target-ppc/translate/vmx-ops.inc.c
index 68cba3e..7a5fec6 100644
--- a/target-ppc/translate/vmx-ops.inc.c
+++ b/target-ppc/translate/vmx-ops.inc.c
@@ -122,7 +122,7 @@ GEN_VXFORM_300(vslv, 2, 29),
GEN_VXFORM(vslo, 6, 16),
GEN_VXFORM(vsro, 6, 17),
GEN_VXFORM(vaddcuw, 0, 6),
-GEN_VXFORM(vsubcuw, 0, 22),
+GEN_VXFORM_DUAL(vsubcuw, xpnd04_1, 0, 22, PPC_ALTIVEC, PPC_NONE),
GEN_VXFORM(vaddubs, 0, 8),
GEN_VXFORM(vadduhs, 0, 9),
GEN_VXFORM(vadduws, 0, 10),
@@ -134,7 +134,7 @@ GEN_VXFORM_DUAL(vsubuhs, bcdsub, 0, 25, PPC_ALTIVEC, PPC_NONE),
GEN_VXFORM(vsubuws, 0, 26),
GEN_VXFORM(vsubsbs, 0, 28),
GEN_VXFORM(vsubshs, 0, 29),
-GEN_VXFORM(vsubsws, 0, 30),
+GEN_VXFORM_DUAL(vsubsws, xpnd04_2, 0, 30, PPC_ALTIVEC, PPC_NONE),
GEN_VXFORM_207(vadduqm, 0, 4),
GEN_VXFORM_207(vaddcuq, 0, 5),
GEN_VXFORM_DUAL(vaddeuqm, vaddecuq, 30, 0xFF, PPC_NONE, PPC2_ALTIVEC_207),
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction
2016-10-26 13:18 [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I Jose Ricardo Ziviani
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction Jose Ricardo Ziviani
@ 2016-10-26 13:18 ` Jose Ricardo Ziviani
2016-10-27 1:20 ` David Gibson
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction Jose Ricardo Ziviani
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction Jose Ricardo Ziviani
3 siblings, 1 reply; 16+ messages in thread
From: Jose Ricardo Ziviani @ 2016-10-26 13:18 UTC (permalink / raw)
To: qemu-ppc; +Cc: qemu-devel, david, nikunj, bharata
bcdctn. converts from BCD to National numeric format. National format
uses a byte to represent a digit where the most significant nibble is
always 0x3 and the least sign. nibbles is the digit itself.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
---
target-ppc/helper.h | 1 +
target-ppc/int_helper.c | 46 +++++++++++++++++++++++++++++++++++++
target-ppc/translate/vmx-impl.inc.c | 24 ++++++++++++++++++-
3 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index d30ec60..92eaaf0 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -370,6 +370,7 @@ DEF_HELPER_4(vpermxor, void, avr, avr, avr, avr)
DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32)
DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
+DEF_HELPER_2(bcdctn, i32, avr, avr)
DEF_HELPER_2(xsadddp, void, env, i32)
DEF_HELPER_2(xssubdp, void, env, i32)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 494c74e..cffe82c 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -2494,6 +2494,17 @@ static uint8_t get_national_digit(ppc_avr_t *reg, int n)
#endif
}
+static void set_national_digit(ppc_avr_t *reg, uint8_t val, int n)
+{
+#if defined(HOST_WORDS_BIGENDIAN)
+ reg->u16[8 - n] &= 0;
+ reg->u16[8 - n] |= val;
+#else
+ reg->u16[n] &= 0;
+ reg->u16[n] |= val;
+#endif
+}
+
static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b)
{
int i;
@@ -2667,6 +2678,41 @@ uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
return cr;
}
+uint32_t helper_bcdctn(ppc_avr_t *r, ppc_avr_t *b)
+{
+ int i;
+ int cr = 0;
+ int invalid = 0;
+ int sgnb = bcd_get_sgn(b);
+ ppc_avr_t ret = { .u64 = { 0, 0 } };
+
+ int eq_flag = (b->u64[HI_IDX] == 0) && ((b->u64[LO_IDX] >> 4) == 0);
+ int ox_flag = (b->u64[HI_IDX] != 0) || ((b->u64[LO_IDX] >> 8) != 0);
+
+ for (i = 1; i < 8; i++) {
+ set_national_digit(&ret, 0x30 + bcd_get_digit(b, i, &invalid), i);
+ }
+ set_national_digit(&ret, (sgnb == -1) ? NATIONAL_NEG : NATIONAL_PLUS, 0);
+
+ if (!eq_flag) {
+ cr = (sgnb == -1) ? 1 << CRF_LT : 1 << CRF_GT;
+ } else {
+ cr = 1 << CRF_EQ;
+ }
+
+ if (ox_flag) {
+ cr |= 1 << CRF_SO;
+ }
+
+ if (unlikely(invalid)) {
+ cr = 1 << CRF_SO;
+ }
+
+ *r = ret;
+
+ return cr;
+}
+
void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
{
int i;
diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
index 2abdcac..4364881 100644
--- a/target-ppc/translate/vmx-impl.inc.c
+++ b/target-ppc/translate/vmx-impl.inc.c
@@ -894,9 +894,29 @@ static void gen_##op(DisasContext *ctx) \
tcg_temp_free_i32(ps); \
}
+#define GEN_BCD3(op) \
+static void gen_##op(DisasContext *ctx) \
+{ \
+ TCGv_ptr rb, rd; \
+ \
+ if (unlikely(!ctx->altivec_enabled)) { \
+ gen_exception(ctx, POWERPC_EXCP_VPU); \
+ return; \
+ } \
+ \
+ rb = gen_avr_ptr(rB(ctx->opcode)); \
+ rd = gen_avr_ptr(rD(ctx->opcode)); \
+ \
+ gen_helper_##op(cpu_crf[6], rd, rb); \
+ \
+ tcg_temp_free_ptr(rb); \
+ tcg_temp_free_ptr(rd); \
+}
+
GEN_BCD(bcdadd)
GEN_BCD(bcdsub)
GEN_BCD2(bcdcfn)
+GEN_BCD3(bcdctn)
static void gen_xpnd04_1(DisasContext *ctx)
{
@@ -908,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx)
case 4:
break; /* bcdctz. */
case 5:
- break; /* bcdctn. */
+ gen_bcdctn(ctx);
+ break;
case 6:
break; /* bcdcfz. */
case 7:
@@ -1024,3 +1045,4 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE,
#undef GEN_VAFORM_PAIRED
#undef GEN_BCD2
+#undef GEN_BCD3
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction
2016-10-26 13:18 [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I Jose Ricardo Ziviani
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction Jose Ricardo Ziviani
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction Jose Ricardo Ziviani
@ 2016-10-26 13:18 ` Jose Ricardo Ziviani
2016-10-27 1:35 ` David Gibson
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction Jose Ricardo Ziviani
3 siblings, 1 reply; 16+ messages in thread
From: Jose Ricardo Ziviani @ 2016-10-26 13:18 UTC (permalink / raw)
To: qemu-ppc; +Cc: qemu-devel, david, nikunj, bharata
bcdcfz. converts from Zoned numeric format to BCD. Zoned format uses
a byte to represent a digit where the most significant nibble is 0x3
or 0xf, depending on the preferred signal.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
---
target-ppc/helper.h | 1 +
target-ppc/int_helper.c | 55 +++++++++++++++++++++++++++++++++++++
target-ppc/translate/vmx-impl.inc.c | 7 +++--
3 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 92eaaf0..f460635 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -371,6 +371,7 @@ DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32)
DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
DEF_HELPER_2(bcdctn, i32, avr, avr)
+DEF_HELPER_3(bcdcfz, i32, avr, avr, i32)
DEF_HELPER_2(xsadddp, void, env, i32)
DEF_HELPER_2(xssubdp, void, env, i32)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index cffe82c..8cbbdfc 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -2459,6 +2459,17 @@ static int bcd_preferred_sgn(int sgn, int ps)
}
}
+static uint8_t get_nibble(ppc_avr_t *bcd, int n)
+{
+ uint8_t result;
+ if (!(n & 1)) {
+ result = bcd->u8[BCD_DIG_BYTE(n)] & 0xF;
+ } else {
+ result = bcd->u8[BCD_DIG_BYTE(n)] >> 4;
+ }
+ return result;
+}
+
static uint8_t bcd_get_digit(ppc_avr_t *bcd, int n, int *invalid)
{
uint8_t result;
@@ -2713,6 +2724,50 @@ uint32_t helper_bcdctn(ppc_avr_t *r, ppc_avr_t *b)
return cr;
}
+uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
+{
+ int i;
+ int j = 0;
+ int cr = 0;
+ int invalid = 0;
+ int eq_flag = 0;
+ int zone_digit = 0;
+ ppc_avr_t ret = { .u64 = { 0, 0 } };
+ int sgnb = get_nibble(b, 1);
+
+ if (unlikely(((sgnb < 0xA) && ps) ||
+ ((get_nibble(b, 0) > 0x9) && !ps))) {
+ invalid = 1;
+ }
+
+ for (i = 31, j = 16; i > 1; i -= 2, j--) {
+ zone_digit = get_nibble(b, i);
+ if (unlikely((ps && zone_digit != 0xF) ||
+ (!ps && zone_digit != 0x3))) {
+ invalid = 1;
+ }
+
+ eq_flag += zone_digit;
+ bcd_put_digit(&ret, get_nibble(b, i - 1), j);
+ }
+ bcd_put_digit(&ret, get_nibble(b, 0), 1);
+
+ if ((ps && (sgnb == 0xB || sgnb == 0xD)) ||
+ (!ps && (sgnb & 0x4))) {
+ bcd_put_digit(&ret, BCD_NEG_PREF, 0);
+ cr = (!eq_flag) ? 1 << CRF_LT : 1 << CRF_EQ;
+ } else {
+ bcd_put_digit(&ret, BCD_PLUS_PREF_1, 0);
+ cr = (!eq_flag) ? 1 << CRF_GT : 1 << CRF_EQ;
+ }
+
+ if (unlikely(invalid)) {
+ cr = 1 << CRF_SO;
+ }
+
+ *r = ret;
+ return cr;
+}
void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
{
int i;
diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
index 4364881..9192f8f 100644
--- a/target-ppc/translate/vmx-impl.inc.c
+++ b/target-ppc/translate/vmx-impl.inc.c
@@ -917,6 +917,7 @@ GEN_BCD(bcdadd)
GEN_BCD(bcdsub)
GEN_BCD2(bcdcfn)
GEN_BCD3(bcdctn)
+GEN_BCD2(bcdcfz)
static void gen_xpnd04_1(DisasContext *ctx)
{
@@ -931,7 +932,8 @@ static void gen_xpnd04_1(DisasContext *ctx)
gen_bcdctn(ctx);
break;
case 6:
- break; /* bcdcfz. */
+ gen_bcdcfz(ctx);
+ break;
case 7:
gen_bcdcfn(ctx);
break;
@@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx)
case 4:
break; /* bcdctz. */
case 6:
- break; /* bcdcfz. */
+ gen_bcdcfz(ctx);
+ break;
case 7:
gen_bcdcfn(ctx);
break;
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction
2016-10-26 13:18 [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I Jose Ricardo Ziviani
` (2 preceding siblings ...)
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction Jose Ricardo Ziviani
@ 2016-10-26 13:18 ` Jose Ricardo Ziviani
2016-10-27 1:47 ` David Gibson
3 siblings, 1 reply; 16+ messages in thread
From: Jose Ricardo Ziviani @ 2016-10-26 13:18 UTC (permalink / raw)
To: qemu-ppc; +Cc: qemu-devel, david, nikunj, bharata
bcdctz. converts from BCD to Zoned numeric format. Zoned format uses
a byte to represent a digit where the most significant nibble is 0x3
or 0xf, depending on the preferred signal.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
---
target-ppc/helper.h | 1 +
target-ppc/int_helper.c | 54 +++++++++++++++++++++++++++++++++++++
target-ppc/translate/vmx-impl.inc.c | 7 +++--
3 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index f460635..3b928b8 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -372,6 +372,7 @@ DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
DEF_HELPER_2(bcdctn, i32, avr, avr)
DEF_HELPER_3(bcdcfz, i32, avr, avr, i32)
+DEF_HELPER_3(bcdctz, i32, avr, avr, i32)
DEF_HELPER_2(xsadddp, void, env, i32)
DEF_HELPER_2(xssubdp, void, env, i32)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 8cbbdfc..e0a84bb 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -2768,6 +2768,60 @@ uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
*r = ret;
return cr;
}
+
+uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
+{
+ int i;
+ int j;
+ int cr = 0;
+ int invalid = 0;
+ uint8_t digit = 0;
+ int sgnb = bcd_get_sgn(b);
+ int zone_lead = (ps) ? 0xF0 : 0x30;
+ ppc_avr_t ret = { .u64 = { 0, 0 } };
+
+ int eq_flag = (b->u64[HI_IDX] == 0) && ((b->u64[LO_IDX] >> 4) == 0);
+ int ox_flag = (b->u64[HI_IDX] != 0);
+
+ for (i = 31, j = 16; i > 1; i -= 2, j--) {
+ digit = get_nibble(b, j);
+ ret.u8[BCD_DIG_BYTE(i)] = zone_lead + digit;
+
+ if (unlikely(digit > 9)) {
+ invalid = 1;
+ }
+ }
+
+ if (unlikely(!sgnb)) {
+ sgnb = (!get_nibble(b, 0)) ? 1 : -1;
+ }
+
+ if (ps) {
+ bcd_put_digit(&ret, (sgnb == 1) ? 0xC : 0xD, 1);
+ } else {
+ bcd_put_digit(&ret, (sgnb == 1) ? 0x3 : 0x7, 1);
+ }
+ bcd_put_digit(&ret, get_nibble(b, 1), 0);
+
+ if (!eq_flag) {
+ cr = (sgnb == 1) ? 1 << CRF_GT : 1 << CRF_LT;
+ } else {
+ cr = 1 << CRF_EQ;
+ }
+
+ if (ox_flag) {
+ cr |= 1 << CRF_SO;
+ }
+
+ if (unlikely(invalid)) {
+ cr = 1 << CRF_SO;
+ }
+
+ *r = ret;
+
+ return cr;
+}
+
void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
{
int i;
diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
index 9192f8f..62c44f0 100644
--- a/target-ppc/translate/vmx-impl.inc.c
+++ b/target-ppc/translate/vmx-impl.inc.c
@@ -918,6 +918,7 @@ GEN_BCD(bcdsub)
GEN_BCD2(bcdcfn)
GEN_BCD3(bcdctn)
GEN_BCD2(bcdcfz)
+GEN_BCD2(bcdctz)
static void gen_xpnd04_1(DisasContext *ctx)
{
@@ -927,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx)
case 2:
break; /* bcdcfsq. */
case 4:
- break; /* bcdctz. */
+ gen_bcdctz(ctx);
+ break;
case 5:
gen_bcdctn(ctx);
break;
@@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx)
case 2:
break; /* bcdcfsq. */
case 4:
- break; /* bcdctz. */
+ gen_bcdctz(ctx);
+ break;
case 6:
gen_bcdcfz(ctx);
break;
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction Jose Ricardo Ziviani
@ 2016-10-27 1:05 ` David Gibson
2016-10-27 15:29 ` joserz
0 siblings, 1 reply; 16+ messages in thread
From: David Gibson @ 2016-10-27 1:05 UTC (permalink / raw)
To: Jose Ricardo Ziviani; +Cc: qemu-ppc, qemu-devel, nikunj, bharata
[-- Attachment #1: Type: text/plain, Size: 8474 bytes --]
On Wed, Oct 26, 2016 at 11:18:55AM -0200, Jose Ricardo Ziviani wrote:
> bcdcfn. converts from National numeric format to BCD. National format
> uses a byte to represent a digit where the most significant nibble is
> always 0x3 and the least sign. nibbles is the digit itself.
>
> Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> ---
> target-ppc/helper.h | 1 +
> target-ppc/int_helper.c | 54 ++++++++++++++++++++++++++
> target-ppc/translate/vmx-impl.inc.c | 75 +++++++++++++++++++++++++++++++++++++
> target-ppc/translate/vmx-ops.inc.c | 4 +-
> 4 files changed, 132 insertions(+), 2 deletions(-)
>
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index 04c6421..d30ec60 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -369,6 +369,7 @@ DEF_HELPER_4(vpermxor, void, avr, avr, avr, avr)
>
> DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32)
> DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
> +DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
>
> DEF_HELPER_2(xsadddp, void, env, i32)
> DEF_HELPER_2(xssubdp, void, env, i32)
> diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> index 5aee0a8..494c74e 100644
> --- a/target-ppc/int_helper.c
> +++ b/target-ppc/int_helper.c
> @@ -2417,6 +2417,8 @@ void helper_vsubecuq(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)
> #define BCD_NEG_PREF 0xD
> #define BCD_NEG_ALT 0xB
> #define BCD_PLUS_ALT_2 0xE
> +#define NATIONAL_PLUS 0x2B
> +#define NATIONAL_NEG 0x2D
>
> #if defined(HOST_WORDS_BIGENDIAN)
> #define BCD_DIG_BYTE(n) (15 - (n/2))
> @@ -2483,6 +2485,15 @@ static void bcd_put_digit(ppc_avr_t *bcd, uint8_t digit, int n)
> }
> }
>
> +static uint8_t get_national_digit(ppc_avr_t *reg, int n)
> +{
> +#if defined(HOST_WORDS_BIGENDIAN)
> + return reg->u16[8 - n] & 0xFF;
> +#else
> + return reg->u16[n] & 0xFF;
> +#endif
You're discarding the high byte of each digit here, which means you
won't detect badly encoded values that have correct low bytes.
> +}
> +
> static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b)
> {
> int i;
> @@ -2613,6 +2624,49 @@ uint32_t helper_bcdsub(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, uint32_t ps)
> return helper_bcdadd(r, a, &bcopy, ps);
> }
>
> +uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> +{
> + int i;
> + int is_zero = 0;
> + int cr = 0;
> + int national = 0;
> + ppc_avr_t ret = { .u64 = { 0, 0 } };
> + uint16_t sgnb = get_national_digit(b, 0);
> + int invalid = (sgnb != NATIONAL_PLUS && sgnb != NATIONAL_NEG);
> +
> + for (i = 1; i < 8; i++) {
> + national = get_national_digit(b, i);
> +
> + is_zero += (national == 0x30);
> + if (unlikely(national < 0x30 || national > 0x39)) {
> + invalid = 1;
> + }
> +
> + bcd_put_digit(&ret, national & 0xf, i);
> + }
> +
> + if (sgnb == NATIONAL_PLUS ||
> + (b->u64[0] == 0 && b->u64[1] == 0)) {
The second part of this test doesn't seem to make much sense. I think
you're trying to always put a positive sign on a zero result. But
you're testing the national encoded input, not the BCD encoded output,
and zero will *not* be all zero bits in national encoding.
> + bcd_put_digit(&ret, (ps == 0) ? BCD_PLUS_PREF_1 : BCD_PLUS_PREF_2, 0);
> + } else {
> + bcd_put_digit(&ret, BCD_NEG_PREF, 0);
> + }
> +
> + if (!is_zero) {
From the logic above, 'is_zero' is currently a count of how many 0
digits the value has, not whether the value as a whole is zero. That
means you will get the wrong result here.
> + cr = (sgnb == NATIONAL_PLUS) ? 1 << CRF_GT : 1 << CRF_LT;
> + } else {
> + cr = 1 << CRF_EQ;
> + }
> +
> + if (unlikely(invalid)) {
> + cr = 1 << CRF_SO;
> + }
> +
> + *r = ret;
> +
> + return cr;
> +}
> +
> void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
> {
> int i;
> diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
> index c8998f3..2abdcac 100644
> --- a/target-ppc/translate/vmx-impl.inc.c
> +++ b/target-ppc/translate/vmx-impl.inc.c
> @@ -871,8 +871,81 @@ static void gen_##op(DisasContext *ctx) \
> tcg_temp_free_i32(ps); \
> }
>
> +#define GEN_BCD2(op) \
> +static void gen_##op(DisasContext *ctx) \
> +{ \
> + TCGv_ptr rd, rb; \
> + TCGv_i32 ps; \
> + \
> + if (unlikely(!ctx->altivec_enabled)) { \
> + gen_exception(ctx, POWERPC_EXCP_VPU); \
> + return; \
> + } \
> + \
> + rb = gen_avr_ptr(rB(ctx->opcode)); \
> + rd = gen_avr_ptr(rD(ctx->opcode)); \
> + \
> + ps = tcg_const_i32((ctx->opcode & 0x200) != 0); \
> + \
> + gen_helper_##op(cpu_crf[6], rd, rb, ps); \
> + \
> + tcg_temp_free_ptr(rb); \
> + tcg_temp_free_ptr(rd); \
> + tcg_temp_free_i32(ps); \
> +}
> +
> GEN_BCD(bcdadd)
> GEN_BCD(bcdsub)
> +GEN_BCD2(bcdcfn)
> +
> +static void gen_xpnd04_1(DisasContext *ctx)
> +{
> + switch (opc4(ctx->opcode)) {
> + case 0:
> + break; /* bcdctsq. */
> + case 2:
> + break; /* bcdcfsq. */
> + case 4:
> + break; /* bcdctz. */
> + case 5:
> + break; /* bcdctn. */
> + case 6:
> + break; /* bcdcfz. */
> + case 7:
> + gen_bcdcfn(ctx);
> + break;
> + case 31:
> + break; /* bcdsetsgn. */
> + default:
> + break;
> + }
> +}
> +
> +static void gen_xpnd04_2(DisasContext *ctx)
> +{
> + switch (opc4(ctx->opcode)) {
> + case 0:
> + break; /* bcdctsq. */
> + case 2:
> + break; /* bcdcfsq. */
> + case 4:
> + break; /* bcdctz. */
> + case 6:
> + break; /* bcdcfz. */
> + case 7:
> + gen_bcdcfn(ctx);
> + break;
> + case 31:
> + break; /* bcdsetsgn. */
> + default:
> + break;
> + }
> +}
I thougt the main opcode table now had support for opc4, so you
shouldn't need these special expanders.
> +GEN_VXFORM_DUAL(vsubcuw, PPC_ALTIVEC, PPC_NONE, \
> + xpnd04_1, PPC_NONE, PPC2_ISA300)
> +GEN_VXFORM_DUAL(vsubsws, PPC_ALTIVEC, PPC_NONE, \
> + xpnd04_2, PPC_NONE, PPC2_ISA300)
>
> GEN_VXFORM_DUAL(vsububm, PPC_ALTIVEC, PPC_NONE, \
> bcdadd, PPC_NONE, PPC2_ALTIVEC_207)
> @@ -949,3 +1022,5 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE,
> #undef GEN_VXFORM_NOA
> #undef GEN_VXFORM_UIMM
> #undef GEN_VAFORM_PAIRED
> +
> +#undef GEN_BCD2
> diff --git a/target-ppc/translate/vmx-ops.inc.c b/target-ppc/translate/vmx-ops.inc.c
> index 68cba3e..7a5fec6 100644
> --- a/target-ppc/translate/vmx-ops.inc.c
> +++ b/target-ppc/translate/vmx-ops.inc.c
> @@ -122,7 +122,7 @@ GEN_VXFORM_300(vslv, 2, 29),
> GEN_VXFORM(vslo, 6, 16),
> GEN_VXFORM(vsro, 6, 17),
> GEN_VXFORM(vaddcuw, 0, 6),
> -GEN_VXFORM(vsubcuw, 0, 22),
> +GEN_VXFORM_DUAL(vsubcuw, xpnd04_1, 0, 22, PPC_ALTIVEC, PPC_NONE),
> GEN_VXFORM(vaddubs, 0, 8),
> GEN_VXFORM(vadduhs, 0, 9),
> GEN_VXFORM(vadduws, 0, 10),
> @@ -134,7 +134,7 @@ GEN_VXFORM_DUAL(vsubuhs, bcdsub, 0, 25, PPC_ALTIVEC, PPC_NONE),
> GEN_VXFORM(vsubuws, 0, 26),
> GEN_VXFORM(vsubsbs, 0, 28),
> GEN_VXFORM(vsubshs, 0, 29),
> -GEN_VXFORM(vsubsws, 0, 30),
> +GEN_VXFORM_DUAL(vsubsws, xpnd04_2, 0, 30, PPC_ALTIVEC, PPC_NONE),
> GEN_VXFORM_207(vadduqm, 0, 4),
> GEN_VXFORM_207(vaddcuq, 0, 5),
> GEN_VXFORM_DUAL(vaddeuqm, vaddecuq, 30, 0xFF, PPC_NONE, PPC2_ALTIVEC_207),
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction Jose Ricardo Ziviani
@ 2016-10-27 1:20 ` David Gibson
2016-10-27 16:13 ` joserz
0 siblings, 1 reply; 16+ messages in thread
From: David Gibson @ 2016-10-27 1:20 UTC (permalink / raw)
To: Jose Ricardo Ziviani; +Cc: qemu-ppc, qemu-devel, nikunj, bharata
[-- Attachment #1: Type: text/plain, Size: 5393 bytes --]
On Wed, Oct 26, 2016 at 11:18:56AM -0200, Jose Ricardo Ziviani wrote:
> bcdctn. converts from BCD to National numeric format. National format
> uses a byte to represent a digit where the most significant nibble is
> always 0x3 and the least sign. nibbles is the digit itself.
>
> Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> ---
> target-ppc/helper.h | 1 +
> target-ppc/int_helper.c | 46 +++++++++++++++++++++++++++++++++++++
> target-ppc/translate/vmx-impl.inc.c | 24 ++++++++++++++++++-
> 3 files changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index d30ec60..92eaaf0 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -370,6 +370,7 @@ DEF_HELPER_4(vpermxor, void, avr, avr, avr, avr)
> DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32)
> DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
> DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
> +DEF_HELPER_2(bcdctn, i32, avr, avr)
>
> DEF_HELPER_2(xsadddp, void, env, i32)
> DEF_HELPER_2(xssubdp, void, env, i32)
> diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> index 494c74e..cffe82c 100644
> --- a/target-ppc/int_helper.c
> +++ b/target-ppc/int_helper.c
> @@ -2494,6 +2494,17 @@ static uint8_t get_national_digit(ppc_avr_t *reg, int n)
> #endif
> }
>
> +static void set_national_digit(ppc_avr_t *reg, uint8_t val, int n)
> +{
> +#if defined(HOST_WORDS_BIGENDIAN)
> + reg->u16[8 - n] &= 0;
> + reg->u16[8 - n] |= val;
The &= always sets the value to 0, so you might as well just use a
plain assignment in place of the &=, |=.
> +#else
> + reg->u16[n] &= 0;
> + reg->u16[n] |= val;
> +#endif
> +}
> +
> static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b)
> {
> int i;
> @@ -2667,6 +2678,41 @@ uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> return cr;
> }
>
> +uint32_t helper_bcdctn(ppc_avr_t *r, ppc_avr_t *b)
> +{
> + int i;
> + int cr = 0;
> + int invalid = 0;
> + int sgnb = bcd_get_sgn(b);
> + ppc_avr_t ret = { .u64 = { 0, 0 } };
> +
> + int eq_flag = (b->u64[HI_IDX] == 0) && ((b->u64[LO_IDX] >> 4) == 0);
> + int ox_flag = (b->u64[HI_IDX] != 0) || ((b->u64[LO_IDX] >> 8) != 0);
This looks wrong. You're shifing the low half right 8 bits == 2
nybbles == 1 digit + sign. So this will set the overflow flag if your
input is a number of >1 digit. I think you want >>32, so it only sets
overflow if the input exceeds 7 decimal digits + sign.
> + for (i = 1; i < 8; i++) {
> + set_national_digit(&ret, 0x30 + bcd_get_digit(b, i, &invalid), i);
> + }
> + set_national_digit(&ret, (sgnb == -1) ? NATIONAL_NEG : NATIONAL_PLUS, 0);
> +
> + if (!eq_flag) {
> + cr = (sgnb == -1) ? 1 << CRF_LT : 1 << CRF_GT;
> + } else {
> + cr = 1 << CRF_EQ;
> + }
> +
> + if (ox_flag) {
> + cr |= 1 << CRF_SO;
> + }
> +
> + if (unlikely(invalid)) {
> + cr = 1 << CRF_SO;
> + }
> +
> + *r = ret;
> +
> + return cr;
> +}
> +
> void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
> {
> int i;
> diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
> index 2abdcac..4364881 100644
> --- a/target-ppc/translate/vmx-impl.inc.c
> +++ b/target-ppc/translate/vmx-impl.inc.c
> @@ -894,9 +894,29 @@ static void gen_##op(DisasContext *ctx) \
> tcg_temp_free_i32(ps); \
> }
>
> +#define GEN_BCD3(op) \
> +static void gen_##op(DisasContext *ctx) \
> +{ \
> + TCGv_ptr rb, rd; \
> + \
> + if (unlikely(!ctx->altivec_enabled)) { \
> + gen_exception(ctx, POWERPC_EXCP_VPU); \
> + return; \
> + } \
> + \
> + rb = gen_avr_ptr(rB(ctx->opcode)); \
> + rd = gen_avr_ptr(rD(ctx->opcode)); \
> + \
> + gen_helper_##op(cpu_crf[6], rd, rb); \
> + \
> + tcg_temp_free_ptr(rb); \
> + tcg_temp_free_ptr(rd); \
> +}
> GEN_BCD(bcdadd)
> GEN_BCD(bcdsub)
> GEN_BCD2(bcdcfn)
> +GEN_BCD3(bcdctn)
>
> static void gen_xpnd04_1(DisasContext *ctx)
> {
> @@ -908,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx)
> case 4:
> break; /* bcdctz. */
> case 5:
> - break; /* bcdctn. */
> + gen_bcdctn(ctx);
> + break;
> case 6:
> break; /* bcdcfz. */
> case 7:
Uh.. doesn't adding bcdctn to this make it identical to gen_xpnd04_2?
> @@ -1024,3 +1045,4 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE,
> #undef GEN_VAFORM_PAIRED
>
> #undef GEN_BCD2
> +#undef GEN_BCD3
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction Jose Ricardo Ziviani
@ 2016-10-27 1:35 ` David Gibson
2016-10-27 18:02 ` joserz
0 siblings, 1 reply; 16+ messages in thread
From: David Gibson @ 2016-10-27 1:35 UTC (permalink / raw)
To: Jose Ricardo Ziviani; +Cc: qemu-ppc, qemu-devel, nikunj, bharata
[-- Attachment #1: Type: text/plain, Size: 4996 bytes --]
On Wed, Oct 26, 2016 at 11:18:57AM -0200, Jose Ricardo Ziviani wrote:
> bcdcfz. converts from Zoned numeric format to BCD. Zoned format uses
> a byte to represent a digit where the most significant nibble is 0x3
> or 0xf, depending on the preferred signal.
>
> Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> ---
> target-ppc/helper.h | 1 +
> target-ppc/int_helper.c | 55 +++++++++++++++++++++++++++++++++++++
> target-ppc/translate/vmx-impl.inc.c | 7 +++--
> 3 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index 92eaaf0..f460635 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -371,6 +371,7 @@ DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32)
> DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
> DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
> DEF_HELPER_2(bcdctn, i32, avr, avr)
> +DEF_HELPER_3(bcdcfz, i32, avr, avr, i32)
>
> DEF_HELPER_2(xsadddp, void, env, i32)
> DEF_HELPER_2(xssubdp, void, env, i32)
> diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> index cffe82c..8cbbdfc 100644
> --- a/target-ppc/int_helper.c
> +++ b/target-ppc/int_helper.c
> @@ -2459,6 +2459,17 @@ static int bcd_preferred_sgn(int sgn, int ps)
> }
> }
>
> +static uint8_t get_nibble(ppc_avr_t *bcd, int n)
> +{
> + uint8_t result;
> + if (!(n & 1)) {
> + result = bcd->u8[BCD_DIG_BYTE(n)] & 0xF;
> + } else {
> + result = bcd->u8[BCD_DIG_BYTE(n)] >> 4;
> + }
> + return result;
> +}
This can be merged with bcd_get_digit() without too much effort.
> static uint8_t bcd_get_digit(ppc_avr_t *bcd, int n, int *invalid)
> {
> uint8_t result;
> @@ -2713,6 +2724,50 @@ uint32_t helper_bcdctn(ppc_avr_t *r, ppc_avr_t *b)
> return cr;
> }
>
> +uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> +{
> + int i;
> + int j = 0;
> + int cr = 0;
> + int invalid = 0;
> + int eq_flag = 0;
> + int zone_digit = 0;
> + ppc_avr_t ret = { .u64 = { 0, 0 } };
> + int sgnb = get_nibble(b, 1);
> +
> + if (unlikely(((sgnb < 0xA) && ps) ||
> + ((get_nibble(b, 0) > 0x9) && !ps))) {
The second half of this condition doesn't belong here. It's a
requirement for both ps values, and would make more sense applied
along with the same test on the other digits.
> + invalid = 1;
> + }
> +
> + for (i = 31, j = 16; i > 1; i -= 2, j--) {
I don't see why you're doing this in reverse order.
> + zone_digit = get_nibble(b, i);
> + if (unlikely((ps && zone_digit != 0xF) ||
> + (!ps && zone_digit != 0x3))) {
> + invalid = 1;
In this and the other instructions it would make sense to break out of
the function as soon as you detect an invalid encoding.
> + }
You never check for invalid encoding in the low input nibble
(i.e. value > 0x9).
> + eq_flag += zone_digit;
What?? How on earth does adding up the zone digits help you. For a
valid encoding the answer should always be either 0x2d (PS=0) or 0xe1
(PS=1).
> + bcd_put_digit(&ret, get_nibble(b, i - 1), j);
> + }
> + bcd_put_digit(&ret, get_nibble(b, 0), 1);
> +
> + if ((ps && (sgnb == 0xB || sgnb == 0xD)) ||
> + (!ps && (sgnb & 0x4))) {
> + bcd_put_digit(&ret, BCD_NEG_PREF, 0);
> + cr = (!eq_flag) ? 1 << CRF_LT : 1 << CRF_EQ;
> + } else {
> + bcd_put_digit(&ret, BCD_PLUS_PREF_1, 0);
> + cr = (!eq_flag) ? 1 << CRF_GT : 1 << CRF_EQ;
> + }
> +
> + if (unlikely(invalid)) {
> + cr = 1 << CRF_SO;
> + }
> +
> + *r = ret;
> + return cr;
> +}
> void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
> {
> int i;
> diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
> index 4364881..9192f8f 100644
> --- a/target-ppc/translate/vmx-impl.inc.c
> +++ b/target-ppc/translate/vmx-impl.inc.c
> @@ -917,6 +917,7 @@ GEN_BCD(bcdadd)
> GEN_BCD(bcdsub)
> GEN_BCD2(bcdcfn)
> GEN_BCD3(bcdctn)
> +GEN_BCD2(bcdcfz)
>
> static void gen_xpnd04_1(DisasContext *ctx)
> {
> @@ -931,7 +932,8 @@ static void gen_xpnd04_1(DisasContext *ctx)
> gen_bcdctn(ctx);
> break;
> case 6:
> - break; /* bcdcfz. */
> + gen_bcdcfz(ctx);
> + break;
> case 7:
> gen_bcdcfn(ctx);
> break;
> @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx)
> case 4:
> break; /* bcdctz. */
> case 6:
> - break; /* bcdcfz. */
> + gen_bcdcfz(ctx);
> + break;
> case 7:
> gen_bcdcfn(ctx);
> break;
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction Jose Ricardo Ziviani
@ 2016-10-27 1:47 ` David Gibson
2016-10-27 18:29 ` joserz
2016-10-27 20:08 ` [Qemu-devel] [Qemu-ppc] " joserz
0 siblings, 2 replies; 16+ messages in thread
From: David Gibson @ 2016-10-27 1:47 UTC (permalink / raw)
To: Jose Ricardo Ziviani; +Cc: qemu-ppc, qemu-devel, nikunj, bharata
[-- Attachment #1: Type: text/plain, Size: 4409 bytes --]
On Wed, Oct 26, 2016 at 11:18:58AM -0200, Jose Ricardo Ziviani wrote:
> bcdctz. converts from BCD to Zoned numeric format. Zoned format uses
> a byte to represent a digit where the most significant nibble is 0x3
> or 0xf, depending on the preferred signal.
>
> Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> ---
> target-ppc/helper.h | 1 +
> target-ppc/int_helper.c | 54 +++++++++++++++++++++++++++++++++++++
> target-ppc/translate/vmx-impl.inc.c | 7 +++--
> 3 files changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index f460635..3b928b8 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -372,6 +372,7 @@ DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
> DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
> DEF_HELPER_2(bcdctn, i32, avr, avr)
> DEF_HELPER_3(bcdcfz, i32, avr, avr, i32)
> +DEF_HELPER_3(bcdctz, i32, avr, avr, i32)
>
> DEF_HELPER_2(xsadddp, void, env, i32)
> DEF_HELPER_2(xssubdp, void, env, i32)
> diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> index 8cbbdfc..e0a84bb 100644
> --- a/target-ppc/int_helper.c
> +++ b/target-ppc/int_helper.c
> @@ -2768,6 +2768,60 @@ uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> *r = ret;
> return cr;
> }
> +
> +uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> +{
> + int i;
> + int j;
> + int cr = 0;
> + int invalid = 0;
> + uint8_t digit = 0;
> + int sgnb = bcd_get_sgn(b);
> + int zone_lead = (ps) ? 0xF0 : 0x30;
> + ppc_avr_t ret = { .u64 = { 0, 0 } };
> +
> + int eq_flag = (b->u64[HI_IDX] == 0) && ((b->u64[LO_IDX] >> 4) == 0);
> + int ox_flag = (b->u64[HI_IDX] != 0);
This doesn't look right. The zoned format can accomodate 16 digits.
The low word of the packed format can accomodate 15 digits + sign. So
it should be possible to convert a packed for with a value in the low
nibble of the upper word.
> + for (i = 31, j = 16; i > 1; i -= 2, j--) {
Again not seeing any point for the reverse order.
> + digit = get_nibble(b, j);
> + ret.u8[BCD_DIG_BYTE(i)] = zone_lead + digit;
> +
> + if (unlikely(digit > 9)) {
> + invalid = 1;
> + }
> + }
> +
> + if (unlikely(!sgnb)) {
> + sgnb = (!get_nibble(b, 0)) ? 1 : -1;
> + }
Doesn't bcd_get_sgn() returning 0 indicate a badly encoded sign
point. In which case you should set invalid, rather than making up a
sign according to this apparently arbitrary scheme.
> +
> + if (ps) {
> + bcd_put_digit(&ret, (sgnb == 1) ? 0xC : 0xD, 1);
> + } else {
> + bcd_put_digit(&ret, (sgnb == 1) ? 0x3 : 0x7, 1);
> + }
> + bcd_put_digit(&ret, get_nibble(b, 1), 0);
> +
> + if (!eq_flag) {
> + cr = (sgnb == 1) ? 1 << CRF_GT : 1 << CRF_LT;
> + } else {
> + cr = 1 << CRF_EQ;
> + }
> +
> + if (ox_flag) {
> + cr |= 1 << CRF_SO;
> + }
> +
> + if (unlikely(invalid)) {
> + cr = 1 << CRF_SO;
> + }
> +
> + *r = ret;
> +
> + return cr;
> +}
> +
> void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
> {
> int i;
> diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
> index 9192f8f..62c44f0 100644
> --- a/target-ppc/translate/vmx-impl.inc.c
> +++ b/target-ppc/translate/vmx-impl.inc.c
> @@ -918,6 +918,7 @@ GEN_BCD(bcdsub)
> GEN_BCD2(bcdcfn)
> GEN_BCD3(bcdctn)
> GEN_BCD2(bcdcfz)
> +GEN_BCD2(bcdctz)
>
> static void gen_xpnd04_1(DisasContext *ctx)
> {
> @@ -927,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx)
> case 2:
> break; /* bcdcfsq. */
> case 4:
> - break; /* bcdctz. */
> + gen_bcdctz(ctx);
> + break;
> case 5:
> gen_bcdctn(ctx);
> break;
> @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx)
> case 2:
> break; /* bcdcfsq. */
> case 4:
> - break; /* bcdctz. */
> + gen_bcdctz(ctx);
> + break;
> case 6:
> gen_bcdcfz(ctx);
> break;
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction
2016-10-27 1:05 ` David Gibson
@ 2016-10-27 15:29 ` joserz
0 siblings, 0 replies; 16+ messages in thread
From: joserz @ 2016-10-27 15:29 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, nikunj, bharata
Hello David,
Thank you very much for your review. As you might have noticed this is my first patch so I hope you don't mind about my newbie questions/explanations below.
On Thu, Oct 27, 2016 at 12:05:30PM +1100, David Gibson wrote:
> On Wed, Oct 26, 2016 at 11:18:55AM -0200, Jose Ricardo Ziviani wrote:
> > bcdcfn. converts from National numeric format to BCD. National format
> > uses a byte to represent a digit where the most significant nibble is
> > always 0x3 and the least sign. nibbles is the digit itself.
> >
> > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> > ---
> > target-ppc/helper.h | 1 +
> > target-ppc/int_helper.c | 54 ++++++++++++++++++++++++++
> > target-ppc/translate/vmx-impl.inc.c | 75 +++++++++++++++++++++++++++++++++++++
> > target-ppc/translate/vmx-ops.inc.c | 4 +-
> > 4 files changed, 132 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> > index 04c6421..d30ec60 100644
> > --- a/target-ppc/helper.h
> > +++ b/target-ppc/helper.h
> > @@ -369,6 +369,7 @@ DEF_HELPER_4(vpermxor, void, avr, avr, avr, avr)
> >
> > DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32)
> > DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
> > +DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
> >
> > DEF_HELPER_2(xsadddp, void, env, i32)
> > DEF_HELPER_2(xssubdp, void, env, i32)
> > diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> > index 5aee0a8..494c74e 100644
> > --- a/target-ppc/int_helper.c
> > +++ b/target-ppc/int_helper.c
> > @@ -2417,6 +2417,8 @@ void helper_vsubecuq(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)
> > #define BCD_NEG_PREF 0xD
> > #define BCD_NEG_ALT 0xB
> > #define BCD_PLUS_ALT_2 0xE
> > +#define NATIONAL_PLUS 0x2B
> > +#define NATIONAL_NEG 0x2D
> >
> > #if defined(HOST_WORDS_BIGENDIAN)
> > #define BCD_DIG_BYTE(n) (15 - (n/2))
> > @@ -2483,6 +2485,15 @@ static void bcd_put_digit(ppc_avr_t *bcd, uint8_t digit, int n)
> > }
> > }
> >
> > +static uint8_t get_national_digit(ppc_avr_t *reg, int n)
> > +{
> > +#if defined(HOST_WORDS_BIGENDIAN)
> > + return reg->u16[8 - n] & 0xFF;
> > +#else
> > + return reg->u16[n] & 0xFF;
> > +#endif
>
> You're discarding the high byte of each digit here, which means you
> won't detect badly encoded values that have correct low bytes.
OK! The high byte is expected to be 0, I'll check if it's != 0 and set a
flag like:
*invalid = (reg->u16[n] >> 8) != 0;
makes sense?
>
> > +}
> > +
> > static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b)
> > {
> > int i;
> > @@ -2613,6 +2624,49 @@ uint32_t helper_bcdsub(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, uint32_t ps)
> > return helper_bcdadd(r, a, &bcopy, ps);
> > }
> >
> > +uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> > +{
> > + int i;
> > + int is_zero = 0;
> > + int cr = 0;
> > + int national = 0;
> > + ppc_avr_t ret = { .u64 = { 0, 0 } };
> > + uint16_t sgnb = get_national_digit(b, 0);
> > + int invalid = (sgnb != NATIONAL_PLUS && sgnb != NATIONAL_NEG);
> > +
> > + for (i = 1; i < 8; i++) {
> > + national = get_national_digit(b, i);
> > +
> > + is_zero += (national == 0x30);
> > + if (unlikely(national < 0x30 || national > 0x39)) {
> > + invalid = 1;
> > + }
> > +
> > + bcd_put_digit(&ret, national & 0xf, i);
> > + }
> > +
> > + if (sgnb == NATIONAL_PLUS ||
> > + (b->u64[0] == 0 && b->u64[1] == 0)) {
>
> The second part of this test doesn't seem to make much sense. I think
> you're trying to always put a positive sign on a zero result. But
> you're testing the national encoded input, not the BCD encoded output,
> and zero will *not* be all zero bits in national encoding.
OK! ISA defines invalid encoding as undefined behavior. I forced a
positive sign to vrt when vrb = 0 (invalid enc.) but it is not necessary.
I'll remove it in v2.
>
> > + bcd_put_digit(&ret, (ps == 0) ? BCD_PLUS_PREF_1 : BCD_PLUS_PREF_2, 0);
> > + } else {
> > + bcd_put_digit(&ret, BCD_NEG_PREF, 0);
> > + }
> > +
> > + if (!is_zero) {
>
> From the logic above, 'is_zero' is currently a count of how many 0
> digits the value has, not whether the value as a whole is zero. That
> means you will get the wrong result here.
>
OK! I'll fix that logic.
> > + cr = (sgnb == NATIONAL_PLUS) ? 1 << CRF_GT : 1 << CRF_LT;
> > + } else {
> > + cr = 1 << CRF_EQ;
> > + }
> > +
> > + if (unlikely(invalid)) {
> > + cr = 1 << CRF_SO;
> > + }
> > +
> > + *r = ret;
> > +
> > + return cr;
> > +}
> > +
> > void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
> > {
> > int i;
> > diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
> > index c8998f3..2abdcac 100644
> > --- a/target-ppc/translate/vmx-impl.inc.c
> > +++ b/target-ppc/translate/vmx-impl.inc.c
> > @@ -871,8 +871,81 @@ static void gen_##op(DisasContext *ctx) \
> > tcg_temp_free_i32(ps); \
> > }
> >
> > +#define GEN_BCD2(op) \
> > +static void gen_##op(DisasContext *ctx) \
> > +{ \
> > + TCGv_ptr rd, rb; \
> > + TCGv_i32 ps; \
> > + \
> > + if (unlikely(!ctx->altivec_enabled)) { \
> > + gen_exception(ctx, POWERPC_EXCP_VPU); \
> > + return; \
> > + } \
> > + \
> > + rb = gen_avr_ptr(rB(ctx->opcode)); \
> > + rd = gen_avr_ptr(rD(ctx->opcode)); \
> > + \
> > + ps = tcg_const_i32((ctx->opcode & 0x200) != 0); \
> > + \
> > + gen_helper_##op(cpu_crf[6], rd, rb, ps); \
> > + \
> > + tcg_temp_free_ptr(rb); \
> > + tcg_temp_free_ptr(rd); \
> > + tcg_temp_free_i32(ps); \
> > +}
> > +
> > GEN_BCD(bcdadd)
> > GEN_BCD(bcdsub)
> > +GEN_BCD2(bcdcfn)
> > +
> > +static void gen_xpnd04_1(DisasContext *ctx)
> > +{
> > + switch (opc4(ctx->opcode)) {
> > + case 0:
> > + break; /* bcdctsq. */
> > + case 2:
> > + break; /* bcdcfsq. */
> > + case 4:
> > + break; /* bcdctz. */
> > + case 5:
> > + break; /* bcdctn. */
> > + case 6:
> > + break; /* bcdcfz. */
> > + case 7:
> > + gen_bcdcfn(ctx);
> > + break;
> > + case 31:
> > + break; /* bcdsetsgn. */
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +static void gen_xpnd04_2(DisasContext *ctx)
> > +{
> > + switch (opc4(ctx->opcode)) {
> > + case 0:
> > + break; /* bcdctsq. */
> > + case 2:
> > + break; /* bcdcfsq. */
> > + case 4:
> > + break; /* bcdctz. */
> > + case 6:
> > + break; /* bcdcfz. */
> > + case 7:
> > + gen_bcdcfn(ctx);
> > + break;
> > + case 31:
> > + break; /* bcdsetsgn. */
> > + default:
> > + break;
> > + }
> > +}
>
> I thougt the main opcode table now had support for opc4, so you
> shouldn't need these special expanders.
This is the hardest one. :)
For all the functions above we have opcode collisions today. Let's take the bcdcfn case, its opcodes are:
000100 vrt:5 00111 vrb:5 11110 00000 1, when ps=1
000100 vrt:5 00111 vrb:5 10110 00000 1, when ps=0
Registering the opcode in the opcode table at translate_init.c, the following sequence is assumed:
0x4 -> 0x0 -> 0x1e (ps=1) or 0x16 (ps=0) -> 0x7
However,
0x4, 0x0, 0x1e already maps to a direct opcode:
vsubsws 000100 vrt:5 vra:5 vrb:5 11110 00000 0
as well as 0x4, 0x0, 0x16, that maps to:
vsubcuw 000100 vrt:5 vra:5 vrb:5 10110 00000 0
I this case, gen_xpnd04_1 handles ps=0 collisions and gen_xpnd04_2 does the same when ps=1. So Nikunj gave me this great idea to handle these exceptional cases.
>
> > +GEN_VXFORM_DUAL(vsubcuw, PPC_ALTIVEC, PPC_NONE, \
> > + xpnd04_1, PPC_NONE, PPC2_ISA300)
> > +GEN_VXFORM_DUAL(vsubsws, PPC_ALTIVEC, PPC_NONE, \
> > + xpnd04_2, PPC_NONE, PPC2_ISA300)
> >
> > GEN_VXFORM_DUAL(vsububm, PPC_ALTIVEC, PPC_NONE, \
> > bcdadd, PPC_NONE, PPC2_ALTIVEC_207)
> > @@ -949,3 +1022,5 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE,
> > #undef GEN_VXFORM_NOA
> > #undef GEN_VXFORM_UIMM
> > #undef GEN_VAFORM_PAIRED
> > +
> > +#undef GEN_BCD2
> > diff --git a/target-ppc/translate/vmx-ops.inc.c b/target-ppc/translate/vmx-ops.inc.c
> > index 68cba3e..7a5fec6 100644
> > --- a/target-ppc/translate/vmx-ops.inc.c
> > +++ b/target-ppc/translate/vmx-ops.inc.c
> > @@ -122,7 +122,7 @@ GEN_VXFORM_300(vslv, 2, 29),
> > GEN_VXFORM(vslo, 6, 16),
> > GEN_VXFORM(vsro, 6, 17),
> > GEN_VXFORM(vaddcuw, 0, 6),
> > -GEN_VXFORM(vsubcuw, 0, 22),
> > +GEN_VXFORM_DUAL(vsubcuw, xpnd04_1, 0, 22, PPC_ALTIVEC, PPC_NONE),
> > GEN_VXFORM(vaddubs, 0, 8),
> > GEN_VXFORM(vadduhs, 0, 9),
> > GEN_VXFORM(vadduws, 0, 10),
> > @@ -134,7 +134,7 @@ GEN_VXFORM_DUAL(vsubuhs, bcdsub, 0, 25, PPC_ALTIVEC, PPC_NONE),
> > GEN_VXFORM(vsubuws, 0, 26),
> > GEN_VXFORM(vsubsbs, 0, 28),
> > GEN_VXFORM(vsubshs, 0, 29),
> > -GEN_VXFORM(vsubsws, 0, 30),
> > +GEN_VXFORM_DUAL(vsubsws, xpnd04_2, 0, 30, PPC_ALTIVEC, PPC_NONE),
> > GEN_VXFORM_207(vadduqm, 0, 4),
> > GEN_VXFORM_207(vaddcuq, 0, 5),
> > GEN_VXFORM_DUAL(vaddeuqm, vaddecuq, 30, 0xFF, PPC_NONE, PPC2_ALTIVEC_207),
>
> --
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction
2016-10-27 1:20 ` David Gibson
@ 2016-10-27 16:13 ` joserz
2016-10-31 0:16 ` David Gibson
0 siblings, 1 reply; 16+ messages in thread
From: joserz @ 2016-10-27 16:13 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, nikunj, bharata
On Thu, Oct 27, 2016 at 12:20:17PM +1100, David Gibson wrote:
> On Wed, Oct 26, 2016 at 11:18:56AM -0200, Jose Ricardo Ziviani wrote:
> > bcdctn. converts from BCD to National numeric format. National format
> > uses a byte to represent a digit where the most significant nibble is
> > always 0x3 and the least sign. nibbles is the digit itself.
> >
> > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> > ---
> > target-ppc/helper.h | 1 +
> > target-ppc/int_helper.c | 46 +++++++++++++++++++++++++++++++++++++
> > target-ppc/translate/vmx-impl.inc.c | 24 ++++++++++++++++++-
> > 3 files changed, 70 insertions(+), 1 deletion(-)
> >
> > diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> > index d30ec60..92eaaf0 100644
> > --- a/target-ppc/helper.h
> > +++ b/target-ppc/helper.h
> > @@ -370,6 +370,7 @@ DEF_HELPER_4(vpermxor, void, avr, avr, avr, avr)
> > DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32)
> > DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
> > DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
> > +DEF_HELPER_2(bcdctn, i32, avr, avr)
> >
> > DEF_HELPER_2(xsadddp, void, env, i32)
> > DEF_HELPER_2(xssubdp, void, env, i32)
> > diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> > index 494c74e..cffe82c 100644
> > --- a/target-ppc/int_helper.c
> > +++ b/target-ppc/int_helper.c
> > @@ -2494,6 +2494,17 @@ static uint8_t get_national_digit(ppc_avr_t *reg, int n)
> > #endif
> > }
> >
> > +static void set_national_digit(ppc_avr_t *reg, uint8_t val, int n)
> > +{
> > +#if defined(HOST_WORDS_BIGENDIAN)
> > + reg->u16[8 - n] &= 0;
> > + reg->u16[8 - n] |= val;
>
> The &= always sets the value to 0, so you might as well just use a
> plain assignment in place of the &=, |=.
OK! I'll fix it in v2.
>
> > +#else
> > + reg->u16[n] &= 0;
> > + reg->u16[n] |= val;
> > +#endif
> > +}
> > +
> > static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b)
> > {
> > int i;
> > @@ -2667,6 +2678,41 @@ uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> > return cr;
> > }
> >
> > +uint32_t helper_bcdctn(ppc_avr_t *r, ppc_avr_t *b)
> > +{
> > + int i;
> > + int cr = 0;
> > + int invalid = 0;
> > + int sgnb = bcd_get_sgn(b);
> > + ppc_avr_t ret = { .u64 = { 0, 0 } };
> > +
> > + int eq_flag = (b->u64[HI_IDX] == 0) && ((b->u64[LO_IDX] >> 4) == 0);
> > + int ox_flag = (b->u64[HI_IDX] != 0) || ((b->u64[LO_IDX] >> 8) != 0);
>
> This looks wrong. You're shifing the low half right 8 bits == 2
> nybbles == 1 digit + sign. So this will set the overflow flag if your
> input is a number of >1 digit. I think you want >>32, so it only sets
> overflow if the input exceeds 7 decimal digits + sign.
OK! I'll fix it in v2.
>
> > + for (i = 1; i < 8; i++) {
> > + set_national_digit(&ret, 0x30 + bcd_get_digit(b, i, &invalid), i);
> > + }
> > + set_national_digit(&ret, (sgnb == -1) ? NATIONAL_NEG : NATIONAL_PLUS, 0);
> > +
> > + if (!eq_flag) {
> > + cr = (sgnb == -1) ? 1 << CRF_LT : 1 << CRF_GT;
> > + } else {
> > + cr = 1 << CRF_EQ;
> > + }
> > +
> > + if (ox_flag) {
> > + cr |= 1 << CRF_SO;
> > + }
> > +
> > + if (unlikely(invalid)) {
> > + cr = 1 << CRF_SO;
> > + }
> > +
> > + *r = ret;
> > +
> > + return cr;
> > +}
> > +
> > void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
> > {
> > int i;
> > diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
> > index 2abdcac..4364881 100644
> > --- a/target-ppc/translate/vmx-impl.inc.c
> > +++ b/target-ppc/translate/vmx-impl.inc.c
> > @@ -894,9 +894,29 @@ static void gen_##op(DisasContext *ctx) \
> > tcg_temp_free_i32(ps); \
> > }
> >
> > +#define GEN_BCD3(op) \
> > +static void gen_##op(DisasContext *ctx) \
> > +{ \
> > + TCGv_ptr rb, rd; \
> > + \
> > + if (unlikely(!ctx->altivec_enabled)) { \
> > + gen_exception(ctx, POWERPC_EXCP_VPU); \
> > + return; \
> > + } \
> > + \
> > + rb = gen_avr_ptr(rB(ctx->opcode)); \
> > + rd = gen_avr_ptr(rD(ctx->opcode)); \
> > + \
> > + gen_helper_##op(cpu_crf[6], rd, rb); \
> > + \
> > + tcg_temp_free_ptr(rb); \
> > + tcg_temp_free_ptr(rd); \
> > +}
> > GEN_BCD(bcdadd)
> > GEN_BCD(bcdsub)
> > GEN_BCD2(bcdcfn)
> > +GEN_BCD3(bcdctn)
> >
> > static void gen_xpnd04_1(DisasContext *ctx)
> > {
> > @@ -908,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx)
> > case 4:
> > break; /* bcdctz. */
> > case 5:
> > - break; /* bcdctn. */
> > + gen_bcdctn(ctx);
> > + break;
> > case 6:
> > break; /* bcdcfz. */
> > case 7:
>
> Uh.. doesn't adding bcdctn to this make it identical to gen_xpnd04_2?
Actually this opcode is invalid for bcdctn as per ISA3.0.
Do you think that I should throw something like a SIGILL for this particular case?
>
> > @@ -1024,3 +1045,4 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE,
> > #undef GEN_VAFORM_PAIRED
> >
> > #undef GEN_BCD2
> > +#undef GEN_BCD3
>
> --
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction
2016-10-27 1:35 ` David Gibson
@ 2016-10-27 18:02 ` joserz
0 siblings, 0 replies; 16+ messages in thread
From: joserz @ 2016-10-27 18:02 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, nikunj, bharata
I'll send an improved version in v2.
Thank you for reviewing it.
On Thu, Oct 27, 2016 at 12:35:19PM +1100, David Gibson wrote:
> On Wed, Oct 26, 2016 at 11:18:57AM -0200, Jose Ricardo Ziviani wrote:
> > bcdcfz. converts from Zoned numeric format to BCD. Zoned format uses
> > a byte to represent a digit where the most significant nibble is 0x3
> > or 0xf, depending on the preferred signal.
> >
> > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> > ---
> > target-ppc/helper.h | 1 +
> > target-ppc/int_helper.c | 55 +++++++++++++++++++++++++++++++++++++
> > target-ppc/translate/vmx-impl.inc.c | 7 +++--
> > 3 files changed, 61 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> > index 92eaaf0..f460635 100644
> > --- a/target-ppc/helper.h
> > +++ b/target-ppc/helper.h
> > @@ -371,6 +371,7 @@ DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32)
> > DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
> > DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
> > DEF_HELPER_2(bcdctn, i32, avr, avr)
> > +DEF_HELPER_3(bcdcfz, i32, avr, avr, i32)
> >
> > DEF_HELPER_2(xsadddp, void, env, i32)
> > DEF_HELPER_2(xssubdp, void, env, i32)
> > diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> > index cffe82c..8cbbdfc 100644
> > --- a/target-ppc/int_helper.c
> > +++ b/target-ppc/int_helper.c
> > @@ -2459,6 +2459,17 @@ static int bcd_preferred_sgn(int sgn, int ps)
> > }
> > }
> >
> > +static uint8_t get_nibble(ppc_avr_t *bcd, int n)
> > +{
> > + uint8_t result;
> > + if (!(n & 1)) {
> > + result = bcd->u8[BCD_DIG_BYTE(n)] & 0xF;
> > + } else {
> > + result = bcd->u8[BCD_DIG_BYTE(n)] >> 4;
> > + }
> > + return result;
> > +}
>
> This can be merged with bcd_get_digit() without too much effort.
>
> > static uint8_t bcd_get_digit(ppc_avr_t *bcd, int n, int *invalid)
> > {
> > uint8_t result;
> > @@ -2713,6 +2724,50 @@ uint32_t helper_bcdctn(ppc_avr_t *r, ppc_avr_t *b)
> > return cr;
> > }
> >
> > +uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> > +{
> > + int i;
> > + int j = 0;
> > + int cr = 0;
> > + int invalid = 0;
> > + int eq_flag = 0;
> > + int zone_digit = 0;
> > + ppc_avr_t ret = { .u64 = { 0, 0 } };
> > + int sgnb = get_nibble(b, 1);
> > +
> > + if (unlikely(((sgnb < 0xA) && ps) ||
> > + ((get_nibble(b, 0) > 0x9) && !ps))) {
>
> The second half of this condition doesn't belong here. It's a
> requirement for both ps values, and would make more sense applied
> along with the same test on the other digits.
>
> > + invalid = 1;
> > + }
> > +
> > + for (i = 31, j = 16; i > 1; i -= 2, j--) {
>
> I don't see why you're doing this in reverse order.
>
> > + zone_digit = get_nibble(b, i);
> > + if (unlikely((ps && zone_digit != 0xF) ||
> > + (!ps && zone_digit != 0x3))) {
> > + invalid = 1;
>
> In this and the other instructions it would make sense to break out of
> the function as soon as you detect an invalid encoding.
>
> > + }
>
> You never check for invalid encoding in the low input nibble
> (i.e. value > 0x9).
>
> > + eq_flag += zone_digit;
>
> What?? How on earth does adding up the zone digits help you. For a
> valid encoding the answer should always be either 0x2d (PS=0) or 0xe1
> (PS=1).
>
> > + bcd_put_digit(&ret, get_nibble(b, i - 1), j);
> > + }
> > + bcd_put_digit(&ret, get_nibble(b, 0), 1);
> > +
> > + if ((ps && (sgnb == 0xB || sgnb == 0xD)) ||
> > + (!ps && (sgnb & 0x4))) {
> > + bcd_put_digit(&ret, BCD_NEG_PREF, 0);
> > + cr = (!eq_flag) ? 1 << CRF_LT : 1 << CRF_EQ;
> > + } else {
> > + bcd_put_digit(&ret, BCD_PLUS_PREF_1, 0);
> > + cr = (!eq_flag) ? 1 << CRF_GT : 1 << CRF_EQ;
> > + }
> > +
> > + if (unlikely(invalid)) {
> > + cr = 1 << CRF_SO;
> > + }
> > +
> > + *r = ret;
> > + return cr;
> > +}
> > void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
> > {
> > int i;
> > diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
> > index 4364881..9192f8f 100644
> > --- a/target-ppc/translate/vmx-impl.inc.c
> > +++ b/target-ppc/translate/vmx-impl.inc.c
> > @@ -917,6 +917,7 @@ GEN_BCD(bcdadd)
> > GEN_BCD(bcdsub)
> > GEN_BCD2(bcdcfn)
> > GEN_BCD3(bcdctn)
> > +GEN_BCD2(bcdcfz)
> >
> > static void gen_xpnd04_1(DisasContext *ctx)
> > {
> > @@ -931,7 +932,8 @@ static void gen_xpnd04_1(DisasContext *ctx)
> > gen_bcdctn(ctx);
> > break;
> > case 6:
> > - break; /* bcdcfz. */
> > + gen_bcdcfz(ctx);
> > + break;
> > case 7:
> > gen_bcdcfn(ctx);
> > break;
> > @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx)
> > case 4:
> > break; /* bcdctz. */
> > case 6:
> > - break; /* bcdcfz. */
> > + gen_bcdcfz(ctx);
> > + break;
> > case 7:
> > gen_bcdcfn(ctx);
> > break;
>
> --
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction
2016-10-27 1:47 ` David Gibson
@ 2016-10-27 18:29 ` joserz
2016-10-27 20:08 ` [Qemu-devel] [Qemu-ppc] " joserz
1 sibling, 0 replies; 16+ messages in thread
From: joserz @ 2016-10-27 18:29 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, nikunj, bharata
Everything will be fixed in v2.
Thanks for reviewing it!
On Thu, Oct 27, 2016 at 12:47:34PM +1100, David Gibson wrote:
> On Wed, Oct 26, 2016 at 11:18:58AM -0200, Jose Ricardo Ziviani wrote:
> > bcdctz. converts from BCD to Zoned numeric format. Zoned format uses
> > a byte to represent a digit where the most significant nibble is 0x3
> > or 0xf, depending on the preferred signal.
> >
> > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> > ---
> > target-ppc/helper.h | 1 +
> > target-ppc/int_helper.c | 54 +++++++++++++++++++++++++++++++++++++
> > target-ppc/translate/vmx-impl.inc.c | 7 +++--
> > 3 files changed, 60 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> > index f460635..3b928b8 100644
> > --- a/target-ppc/helper.h
> > +++ b/target-ppc/helper.h
> > @@ -372,6 +372,7 @@ DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
> > DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
> > DEF_HELPER_2(bcdctn, i32, avr, avr)
> > DEF_HELPER_3(bcdcfz, i32, avr, avr, i32)
> > +DEF_HELPER_3(bcdctz, i32, avr, avr, i32)
> >
> > DEF_HELPER_2(xsadddp, void, env, i32)
> > DEF_HELPER_2(xssubdp, void, env, i32)
> > diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> > index 8cbbdfc..e0a84bb 100644
> > --- a/target-ppc/int_helper.c
> > +++ b/target-ppc/int_helper.c
> > @@ -2768,6 +2768,60 @@ uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> > *r = ret;
> > return cr;
> > }
> > +
> > +uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> > +{
> > + int i;
> > + int j;
> > + int cr = 0;
> > + int invalid = 0;
> > + uint8_t digit = 0;
> > + int sgnb = bcd_get_sgn(b);
> > + int zone_lead = (ps) ? 0xF0 : 0x30;
> > + ppc_avr_t ret = { .u64 = { 0, 0 } };
> > +
> > + int eq_flag = (b->u64[HI_IDX] == 0) && ((b->u64[LO_IDX] >> 4) == 0);
> > + int ox_flag = (b->u64[HI_IDX] != 0);
>
> This doesn't look right. The zoned format can accomodate 16 digits.
> The low word of the packed format can accomodate 15 digits + sign. So
> it should be possible to convert a packed for with a value in the low
> nibble of the upper word.
>
> > + for (i = 31, j = 16; i > 1; i -= 2, j--) {
>
> Again not seeing any point for the reverse order.
>
> > + digit = get_nibble(b, j);
> > + ret.u8[BCD_DIG_BYTE(i)] = zone_lead + digit;
> > +
> > + if (unlikely(digit > 9)) {
> > + invalid = 1;
> > + }
> > + }
> > +
> > + if (unlikely(!sgnb)) {
> > + sgnb = (!get_nibble(b, 0)) ? 1 : -1;
> > + }
>
> Doesn't bcd_get_sgn() returning 0 indicate a badly encoded sign
> point. In which case you should set invalid, rather than making up a
> sign according to this apparently arbitrary scheme.
>
> > +
> > + if (ps) {
> > + bcd_put_digit(&ret, (sgnb == 1) ? 0xC : 0xD, 1);
> > + } else {
> > + bcd_put_digit(&ret, (sgnb == 1) ? 0x3 : 0x7, 1);
> > + }
> > + bcd_put_digit(&ret, get_nibble(b, 1), 0);
> > +
> > + if (!eq_flag) {
> > + cr = (sgnb == 1) ? 1 << CRF_GT : 1 << CRF_LT;
> > + } else {
> > + cr = 1 << CRF_EQ;
> > + }
> > +
> > + if (ox_flag) {
> > + cr |= 1 << CRF_SO;
> > + }
> > +
> > + if (unlikely(invalid)) {
> > + cr = 1 << CRF_SO;
> > + }
> > +
> > + *r = ret;
> > +
> > + return cr;
> > +}
> > +
> > void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
> > {
> > int i;
> > diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
> > index 9192f8f..62c44f0 100644
> > --- a/target-ppc/translate/vmx-impl.inc.c
> > +++ b/target-ppc/translate/vmx-impl.inc.c
> > @@ -918,6 +918,7 @@ GEN_BCD(bcdsub)
> > GEN_BCD2(bcdcfn)
> > GEN_BCD3(bcdctn)
> > GEN_BCD2(bcdcfz)
> > +GEN_BCD2(bcdctz)
> >
> > static void gen_xpnd04_1(DisasContext *ctx)
> > {
> > @@ -927,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx)
> > case 2:
> > break; /* bcdcfsq. */
> > case 4:
> > - break; /* bcdctz. */
> > + gen_bcdctz(ctx);
> > + break;
> > case 5:
> > gen_bcdctn(ctx);
> > break;
> > @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx)
> > case 2:
> > break; /* bcdcfsq. */
> > case 4:
> > - break; /* bcdctz. */
> > + gen_bcdctz(ctx);
> > + break;
> > case 6:
> > gen_bcdcfz(ctx);
> > break;
>
> --
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction
2016-10-27 1:47 ` David Gibson
2016-10-27 18:29 ` joserz
@ 2016-10-27 20:08 ` joserz
2016-10-31 2:50 ` David Gibson
1 sibling, 1 reply; 16+ messages in thread
From: joserz @ 2016-10-27 20:08 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, bharata
On Thu, Oct 27, 2016 at 12:47:34PM +1100, David Gibson wrote:
> On Wed, Oct 26, 2016 at 11:18:58AM -0200, Jose Ricardo Ziviani wrote:
> > bcdctz. converts from BCD to Zoned numeric format. Zoned format uses
> > a byte to represent a digit where the most significant nibble is 0x3
> > or 0xf, depending on the preferred signal.
> >
> > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> > ---
> > target-ppc/helper.h | 1 +
> > target-ppc/int_helper.c | 54 +++++++++++++++++++++++++++++++++++++
> > target-ppc/translate/vmx-impl.inc.c | 7 +++--
> > 3 files changed, 60 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> > index f460635..3b928b8 100644
> > --- a/target-ppc/helper.h
> > +++ b/target-ppc/helper.h
> > @@ -372,6 +372,7 @@ DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
> > DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
> > DEF_HELPER_2(bcdctn, i32, avr, avr)
> > DEF_HELPER_3(bcdcfz, i32, avr, avr, i32)
> > +DEF_HELPER_3(bcdctz, i32, avr, avr, i32)
> >
> > DEF_HELPER_2(xsadddp, void, env, i32)
> > DEF_HELPER_2(xssubdp, void, env, i32)
> > diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> > index 8cbbdfc..e0a84bb 100644
> > --- a/target-ppc/int_helper.c
> > +++ b/target-ppc/int_helper.c
> > @@ -2768,6 +2768,60 @@ uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> > *r = ret;
> > return cr;
> > }
> > +
> > +uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> > +{
> > + int i;
> > + int j;
> > + int cr = 0;
> > + int invalid = 0;
> > + uint8_t digit = 0;
> > + int sgnb = bcd_get_sgn(b);
> > + int zone_lead = (ps) ? 0xF0 : 0x30;
> > + ppc_avr_t ret = { .u64 = { 0, 0 } };
> > +
> > + int eq_flag = (b->u64[HI_IDX] == 0) && ((b->u64[LO_IDX] >> 4) == 0);
> > + int ox_flag = (b->u64[HI_IDX] != 0);
>
> This doesn't look right. The zoned format can accomodate 16 digits.
> The low word of the packed format can accomodate 15 digits + sign. So
> it should be possible to convert a packed for with a value in the low
> nibble of the upper word.
David, it seems that zoned format can accommodate 15 digits + sign at most. Suppose I have this 16 bytes zoned number: f9f9f9f9f9f9f9f9f9f9f9f9f9f9f9xx. Converting it to packed bcd will give 999999999999999x. Do you agree? can I keep that ox_flag logic?
>
> > + for (i = 31, j = 16; i > 1; i -= 2, j--) {
>
> Again not seeing any point for the reverse order.
>
> > + digit = get_nibble(b, j);
> > + ret.u8[BCD_DIG_BYTE(i)] = zone_lead + digit;
> > +
> > + if (unlikely(digit > 9)) {
> > + invalid = 1;
> > + }
> > + }
> > +
> > + if (unlikely(!sgnb)) {
> > + sgnb = (!get_nibble(b, 0)) ? 1 : -1;
> > + }
>
> Doesn't bcd_get_sgn() returning 0 indicate a badly encoded sign
> point. In which case you should set invalid, rather than making up a
> sign according to this apparently arbitrary scheme.
>
> > +
> > + if (ps) {
> > + bcd_put_digit(&ret, (sgnb == 1) ? 0xC : 0xD, 1);
> > + } else {
> > + bcd_put_digit(&ret, (sgnb == 1) ? 0x3 : 0x7, 1);
> > + }
> > + bcd_put_digit(&ret, get_nibble(b, 1), 0);
> > +
> > + if (!eq_flag) {
> > + cr = (sgnb == 1) ? 1 << CRF_GT : 1 << CRF_LT;
> > + } else {
> > + cr = 1 << CRF_EQ;
> > + }
> > +
> > + if (ox_flag) {
> > + cr |= 1 << CRF_SO;
> > + }
> > +
> > + if (unlikely(invalid)) {
> > + cr = 1 << CRF_SO;
> > + }
> > +
> > + *r = ret;
> > +
> > + return cr;
> > +}
> > +
> > void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
> > {
> > int i;
> > diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
> > index 9192f8f..62c44f0 100644
> > --- a/target-ppc/translate/vmx-impl.inc.c
> > +++ b/target-ppc/translate/vmx-impl.inc.c
> > @@ -918,6 +918,7 @@ GEN_BCD(bcdsub)
> > GEN_BCD2(bcdcfn)
> > GEN_BCD3(bcdctn)
> > GEN_BCD2(bcdcfz)
> > +GEN_BCD2(bcdctz)
> >
> > static void gen_xpnd04_1(DisasContext *ctx)
> > {
> > @@ -927,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx)
> > case 2:
> > break; /* bcdcfsq. */
> > case 4:
> > - break; /* bcdctz. */
> > + gen_bcdctz(ctx);
> > + break;
> > case 5:
> > gen_bcdctn(ctx);
> > break;
> > @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx)
> > case 2:
> > break; /* bcdcfsq. */
> > case 4:
> > - break; /* bcdctz. */
> > + gen_bcdctz(ctx);
> > + break;
> > case 6:
> > gen_bcdcfz(ctx);
> > break;
>
> --
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction
2016-10-27 16:13 ` joserz
@ 2016-10-31 0:16 ` David Gibson
0 siblings, 0 replies; 16+ messages in thread
From: David Gibson @ 2016-10-31 0:16 UTC (permalink / raw)
To: joserz; +Cc: qemu-ppc, qemu-devel, nikunj, bharata
[-- Attachment #1: Type: text/plain, Size: 6468 bytes --]
On Thu, Oct 27, 2016 at 02:13:13PM -0200, joserz@linux.vnet.ibm.com wrote:
> On Thu, Oct 27, 2016 at 12:20:17PM +1100, David Gibson wrote:
> > On Wed, Oct 26, 2016 at 11:18:56AM -0200, Jose Ricardo Ziviani wrote:
> > > bcdctn. converts from BCD to National numeric format. National format
> > > uses a byte to represent a digit where the most significant nibble is
> > > always 0x3 and the least sign. nibbles is the digit itself.
> > >
> > > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> > > ---
> > > target-ppc/helper.h | 1 +
> > > target-ppc/int_helper.c | 46 +++++++++++++++++++++++++++++++++++++
> > > target-ppc/translate/vmx-impl.inc.c | 24 ++++++++++++++++++-
> > > 3 files changed, 70 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> > > index d30ec60..92eaaf0 100644
> > > --- a/target-ppc/helper.h
> > > +++ b/target-ppc/helper.h
> > > @@ -370,6 +370,7 @@ DEF_HELPER_4(vpermxor, void, avr, avr, avr, avr)
> > > DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32)
> > > DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
> > > DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
> > > +DEF_HELPER_2(bcdctn, i32, avr, avr)
> > >
> > > DEF_HELPER_2(xsadddp, void, env, i32)
> > > DEF_HELPER_2(xssubdp, void, env, i32)
> > > diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> > > index 494c74e..cffe82c 100644
> > > --- a/target-ppc/int_helper.c
> > > +++ b/target-ppc/int_helper.c
> > > @@ -2494,6 +2494,17 @@ static uint8_t get_national_digit(ppc_avr_t *reg, int n)
> > > #endif
> > > }
> > >
> > > +static void set_national_digit(ppc_avr_t *reg, uint8_t val, int n)
> > > +{
> > > +#if defined(HOST_WORDS_BIGENDIAN)
> > > + reg->u16[8 - n] &= 0;
> > > + reg->u16[8 - n] |= val;
> >
> > The &= always sets the value to 0, so you might as well just use a
> > plain assignment in place of the &=, |=.
>
> OK! I'll fix it in v2.
>
> >
> > > +#else
> > > + reg->u16[n] &= 0;
> > > + reg->u16[n] |= val;
> > > +#endif
> > > +}
> > > +
> > > static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b)
> > > {
> > > int i;
> > > @@ -2667,6 +2678,41 @@ uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> > > return cr;
> > > }
> > >
> > > +uint32_t helper_bcdctn(ppc_avr_t *r, ppc_avr_t *b)
> > > +{
> > > + int i;
> > > + int cr = 0;
> > > + int invalid = 0;
> > > + int sgnb = bcd_get_sgn(b);
> > > + ppc_avr_t ret = { .u64 = { 0, 0 } };
> > > +
> > > + int eq_flag = (b->u64[HI_IDX] == 0) && ((b->u64[LO_IDX] >> 4) == 0);
> > > + int ox_flag = (b->u64[HI_IDX] != 0) || ((b->u64[LO_IDX] >> 8) != 0);
> >
> > This looks wrong. You're shifing the low half right 8 bits == 2
> > nybbles == 1 digit + sign. So this will set the overflow flag if your
> > input is a number of >1 digit. I think you want >>32, so it only sets
> > overflow if the input exceeds 7 decimal digits + sign.
>
> OK! I'll fix it in v2.
>
> >
> > > + for (i = 1; i < 8; i++) {
> > > + set_national_digit(&ret, 0x30 + bcd_get_digit(b, i, &invalid), i);
> > > + }
> > > + set_national_digit(&ret, (sgnb == -1) ? NATIONAL_NEG : NATIONAL_PLUS, 0);
> > > +
> > > + if (!eq_flag) {
> > > + cr = (sgnb == -1) ? 1 << CRF_LT : 1 << CRF_GT;
> > > + } else {
> > > + cr = 1 << CRF_EQ;
> > > + }
> > > +
> > > + if (ox_flag) {
> > > + cr |= 1 << CRF_SO;
> > > + }
> > > +
> > > + if (unlikely(invalid)) {
> > > + cr = 1 << CRF_SO;
> > > + }
> > > +
> > > + *r = ret;
> > > +
> > > + return cr;
> > > +}
> > > +
> > > void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
> > > {
> > > int i;
> > > diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c
> > > index 2abdcac..4364881 100644
> > > --- a/target-ppc/translate/vmx-impl.inc.c
> > > +++ b/target-ppc/translate/vmx-impl.inc.c
> > > @@ -894,9 +894,29 @@ static void gen_##op(DisasContext *ctx) \
> > > tcg_temp_free_i32(ps); \
> > > }
> > >
> > > +#define GEN_BCD3(op) \
> > > +static void gen_##op(DisasContext *ctx) \
> > > +{ \
> > > + TCGv_ptr rb, rd; \
> > > + \
> > > + if (unlikely(!ctx->altivec_enabled)) { \
> > > + gen_exception(ctx, POWERPC_EXCP_VPU); \
> > > + return; \
> > > + } \
> > > + \
> > > + rb = gen_avr_ptr(rB(ctx->opcode)); \
> > > + rd = gen_avr_ptr(rD(ctx->opcode)); \
> > > + \
> > > + gen_helper_##op(cpu_crf[6], rd, rb); \
> > > + \
> > > + tcg_temp_free_ptr(rb); \
> > > + tcg_temp_free_ptr(rd); \
> > > +}
> > > GEN_BCD(bcdadd)
> > > GEN_BCD(bcdsub)
> > > GEN_BCD2(bcdcfn)
> > > +GEN_BCD3(bcdctn)
> > >
> > > static void gen_xpnd04_1(DisasContext *ctx)
> > > {
> > > @@ -908,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx)
> > > case 4:
> > > break; /* bcdctz. */
> > > case 5:
> > > - break; /* bcdctn. */
> > > + gen_bcdctn(ctx);
> > > + break;
> > > case 6:
> > > break; /* bcdcfz. */
> > > case 7:
> >
> > Uh.. doesn't adding bcdctn to this make it identical to gen_xpnd04_2?
>
> Actually this opcode is invalid for bcdctn as per ISA3.0.
>
> Do you think that I should throw something like a SIGILL for this particular case?
Yes, if the opcode is invalid, you absolutely should be throwing an
invalid instruction exception.
> >
> > > @@ -1024,3 +1045,4 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE,
> > > #undef GEN_VAFORM_PAIRED
> > >
> > > #undef GEN_BCD2
> > > +#undef GEN_BCD3
> >
>
>
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction
2016-10-27 20:08 ` [Qemu-devel] [Qemu-ppc] " joserz
@ 2016-10-31 2:50 ` David Gibson
0 siblings, 0 replies; 16+ messages in thread
From: David Gibson @ 2016-10-31 2:50 UTC (permalink / raw)
To: joserz; +Cc: qemu-ppc, qemu-devel, bharata
[-- Attachment #1: Type: text/plain, Size: 3138 bytes --]
On Thu, Oct 27, 2016 at 06:08:20PM -0200, joserz@linux.vnet.ibm.com wrote:
> On Thu, Oct 27, 2016 at 12:47:34PM +1100, David Gibson wrote:
> > On Wed, Oct 26, 2016 at 11:18:58AM -0200, Jose Ricardo Ziviani wrote:
> > > bcdctz. converts from BCD to Zoned numeric format. Zoned format uses
> > > a byte to represent a digit where the most significant nibble is 0x3
> > > or 0xf, depending on the preferred signal.
> > >
> > > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> > > ---
> > > target-ppc/helper.h | 1 +
> > > target-ppc/int_helper.c | 54 +++++++++++++++++++++++++++++++++++++
> > > target-ppc/translate/vmx-impl.inc.c | 7 +++--
> > > 3 files changed, 60 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> > > index f460635..3b928b8 100644
> > > --- a/target-ppc/helper.h
> > > +++ b/target-ppc/helper.h
> > > @@ -372,6 +372,7 @@ DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32)
> > > DEF_HELPER_3(bcdcfn, i32, avr, avr, i32)
> > > DEF_HELPER_2(bcdctn, i32, avr, avr)
> > > DEF_HELPER_3(bcdcfz, i32, avr, avr, i32)
> > > +DEF_HELPER_3(bcdctz, i32, avr, avr, i32)
> > >
> > > DEF_HELPER_2(xsadddp, void, env, i32)
> > > DEF_HELPER_2(xssubdp, void, env, i32)
> > > diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> > > index 8cbbdfc..e0a84bb 100644
> > > --- a/target-ppc/int_helper.c
> > > +++ b/target-ppc/int_helper.c
> > > @@ -2768,6 +2768,60 @@ uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> > > *r = ret;
> > > return cr;
> > > }
> > > +
> > > +uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps)
> > > +{
> > > + int i;
> > > + int j;
> > > + int cr = 0;
> > > + int invalid = 0;
> > > + uint8_t digit = 0;
> > > + int sgnb = bcd_get_sgn(b);
> > > + int zone_lead = (ps) ? 0xF0 : 0x30;
> > > + ppc_avr_t ret = { .u64 = { 0, 0 } };
> > > +
> > > + int eq_flag = (b->u64[HI_IDX] == 0) && ((b->u64[LO_IDX] >> 4) == 0);
> > > + int ox_flag = (b->u64[HI_IDX] != 0);
> >
> > This doesn't look right. The zoned format can accomodate 16 digits.
> > The low word of the packed format can accomodate 15 digits + sign. So
> > it should be possible to convert a packed for with a value in the low
> > nibble of the upper word.
>
> David, it seems that zoned format can accommodate 15 digits + sign
> at most. Suppose I have this 16 bytes zoned number:
> f9f9f9f9f9f9f9f9f9f9f9f9f9f9f9xx. Converting it to packed bcd will
> give 999999999999999x. Do you agree? can I keep that ox_flag logic?
Uh.. no. AFAICT from the description of the instruction the last byte
of a zoned number contains *both* a sign nibble *and* the least
significant digit of the number. So zoned format can represent 16
digits + sign.
But IIRC you changed this in v2, so maybe you realised that already.
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-10-31 2:53 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-26 13:18 [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I Jose Ricardo Ziviani
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction Jose Ricardo Ziviani
2016-10-27 1:05 ` David Gibson
2016-10-27 15:29 ` joserz
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction Jose Ricardo Ziviani
2016-10-27 1:20 ` David Gibson
2016-10-27 16:13 ` joserz
2016-10-31 0:16 ` David Gibson
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction Jose Ricardo Ziviani
2016-10-27 1:35 ` David Gibson
2016-10-27 18:02 ` joserz
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction Jose Ricardo Ziviani
2016-10-27 1:47 ` David Gibson
2016-10-27 18:29 ` joserz
2016-10-27 20:08 ` [Qemu-devel] [Qemu-ppc] " joserz
2016-10-31 2:50 ` David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.