OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

target/ppc: Fix bcdsub. emulation when result overflows

Browse Source 
The commit d03b174a83 (target/ppc: simplify bcdadd/sub functions)
meant to simplify some of the code but it inadvertently altered the
way the CR6 field is set after the operation has overflowed.

The CR6 bits are set based on the *unbounded* result of the operation,
so we need to look at the result before returning from bcd_add_mag,
otherwise we will look at 0 when it overflows.

Consider the following subtraction:

v0 = 0x9999999999999999999999999999999c (maximum positive BCD value)
v1 = 0x0000000000000000000000000000001d (negative one BCD value)
bcdsub. v0,v0,v1,0

The Power ISA 2.07B says:
If the unbounded result is greater than zero, do the following.
  If PS=0, the sign code of the result is set to 0b1100.
  If PS=1, the sign code of the result is set to 0b1111.
  If the operation overflows, CR field 6 is set to 0b0101. Otherwise,
  CR field 6 is set to 0b0100.

POWER9 hardware:
vr0 = 0x0000000000000000000000000000000c (positive zero BCD value)
cr6 = 0b0101 (0x5) (positive, overflow)

QEMU:
vr0 = 0x0000000000000000000000000000000c (positive zero BCD value)
cr6 = 0b0011 (0x3) (zero, overflow) <--- wrong

This patch reverts the part of d03b174a83 that introduced the
problem and adds a test-case to avoid further regressions:

before:
$ make run-tcg-tests-ppc64le-linux-user
(...)
  TEST    bcdsub on ppc64le
bcdsub: qemu/tests/tcg/ppc64le/bcdsub.c:58: test_bcdsub_gt:
Assertion `(cr >> 4) == ((1 << 2) | (1 << 0))' failed.

Fixes: d03b174a83 (target/ppc: simplify bcdadd/sub functions)
Reported-by: Paul Clarke <pc@us.ibm.com>
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
Message-Id: <20210222194035.2723056-1-farosas@linux.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This commit is contained in:
Fabiano Rosas 2021-02-22 16:40:35 -03:00 committed by David Gibson
parent a4ee352fe0
commit 936fda4d77
5 changed files with 171 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
target/ppc/int_helper.c
View File

@ -2175,14 +2175,17 @@ static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b)
return 0;
}
static void bcd_add_mag(ppc_avr_t *t, ppc_avr_t *a, ppc_avr_t *b, int *invalid,
static int bcd_add_mag(ppc_avr_t *t, ppc_avr_t *a, ppc_avr_t *b, int *invalid,
int *overflow)
{
int carry = 0;
int i;
int is_zero = 1;
for (i = 1; i <= 31; i++) {
uint8_t digit = bcd_get_digit(a, i, invalid) +
bcd_get_digit(b, i, invalid) + carry;
is_zero &= (digit == 0);
if (digit > 9) {
carry = 1;
digit -= 10;
@ -2194,6 +2197,7 @@ static void bcd_add_mag(ppc_avr_t *t, ppc_avr_t *a, ppc_avr_t *b, int *invalid,
}
*overflow = carry;
return is_zero;
}
static void bcd_sub_mag(ppc_avr_t *t, ppc_avr_t *a, ppc_avr_t *b, int *invalid,
@ -2225,14 +2229,15 @@ uint32_t helper_bcdadd(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, uint32_t ps)
int sgnb = bcd_get_sgn(b);
int invalid = (sgna == 0) || (sgnb == 0);
int overflow = 0;
int zero = 0;
uint32_t cr = 0;
ppc_avr_t result = { .u64 = { 0, 0 } };
if (!invalid) {
if (sgna == sgnb) {
result.VsrB(BCD_DIG_BYTE(0)) = bcd_preferred_sgn(sgna, ps);
bcd_add_mag(&result, a, b, &invalid, &overflow);
cr = bcd_cmp_zero(&result);
zero = bcd_add_mag(&result, a, b, &invalid, &overflow);
cr = (sgna > 0) ? CRF_GT : CRF_LT;
} else {
int magnitude = bcd_cmp_mag(a, b);
if (magnitude > 0) {
@ -2255,6 +2260,8 @@ uint32_t helper_bcdadd(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, uint32_t ps)
cr = CRF_SO;
} else if (overflow) {
cr |= CRF_SO;
} else if (zero) {
cr |= CRF_EQ;
}
*r = result;

6
tests/tcg/configure.sh
View File

@ -251,6 +251,12 @@ for target in $target_list; do
echo "CROSS_CC_HAS_ARMV8_MTE=y" >> $config_target_mak
fi
;;
ppc*)
if do_compiler "$target_compiler" $target_compiler_cflags \
-mpower8-vector -o $TMPE $TMPC; then
echo "CROSS_CC_HAS_POWER8_VECTOR=y" >> $config_target_mak
fi
;;
esac
enabled_cross_compilers="$enabled_cross_compilers $target_compiler"

13
tests/tcg/ppc64/Makefile.target Normal file
View File

@ -0,0 +1,13 @@
# -*- Mode: makefile -*-
#
# ppc64 specific tweaks
VPATH += $(SRC_PATH)/tests/tcg/ppc64
VPATH += $(SRC_PATH)/tests/tcg/ppc64le
ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_POWER8_VECTOR),)
PPC64_TESTS=bcdsub
endif
bcdsub: CFLAGS += -mpower8-vector
TESTS += $(PPC64_TESTS)

12
tests/tcg/ppc64le/Makefile.target Normal file
View File

@ -0,0 +1,12 @@
# -*- Mode: makefile -*-
#
# ppc64le specific tweaks
VPATH += $(SRC_PATH)/tests/tcg/ppc64le
ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_POWER8_VECTOR),)
PPC64LE_TESTS=bcdsub
endif
bcdsub: CFLAGS += -mpower8-vector
TESTS += $(PPC64LE_TESTS)

130
tests/tcg/ppc64le/bcdsub.c Normal file
View File

@ -0,0 +1,130 @@
#include <assert.h>
#include <unistd.h>
#include <signal.h>
#define CRF_LT (1 << 3)
#define CRF_GT (1 << 2)
#define CRF_EQ (1 << 1)
#define CRF_SO (1 << 0)
#define UNDEF 0
#define BCDSUB(vra, vrb, ps) \
asm ("bcdsub. %1,%2,%3,%4;" \
"mfocrf %0,0b10;" \
: "=r" (cr), "=v" (vrt) \
: "v" (vra), "v" (vrb), "i" (ps) \
: );
#define TEST(vra, vrb, ps, exp_res, exp_cr6) \
do { \
__int128 vrt = 0; \
int cr = 0; \
BCDSUB(vra, vrb, ps); \
if (exp_res) \
assert(vrt == exp_res); \
assert((cr >> 4) == exp_cr6); \
} while (0)
/*
* Unbounded result is equal to zero:
* sign = (PS) ? 0b1111 : 0b1100
* CR6 = 0b0010
*/
void test_bcdsub_eq(void)
{
__int128 a, b;
/* maximum positive BCD value */
a = b = (((__int128) 0x9999999999999999) << 64 | 0x999999999999999c);
TEST(a, b, 0, 0xc, CRF_EQ);
TEST(a, b, 1, 0xf, CRF_EQ);
}
/*
* Unbounded result is greater than zero:
* sign = (PS) ? 0b1111 : 0b1100
* CR6 = (overflow) ? 0b0101 : 0b0100
*/
void test_bcdsub_gt(void)
{
__int128 a, b, c;
/* maximum positive BCD value */
a = (((__int128) 0x9999999999999999) << 64 | 0x999999999999999c);
/* negative one BCD value */
b = (__int128) 0x1d;
TEST(a, b, 0, 0xc, (CRF_GT | CRF_SO));
TEST(a, b, 1, 0xf, (CRF_GT | CRF_SO));
c = (((__int128) 0x9999999999999999) << 64 | 0x999999999999998c);
TEST(c, b, 0, a, CRF_GT);
TEST(c, b, 1, (a | 0x3), CRF_GT);
}
/*
* Unbounded result is less than zero:
* sign = 0b1101
* CR6 = (overflow) ? 0b1001 : 0b1000
*/
void test_bcdsub_lt(void)
{
__int128 a, b;
/* positive zero BCD value */
a = (__int128) 0xc;
/* positive one BCD value */
b = (__int128) 0x1c;
TEST(a, b, 0, 0x1d, CRF_LT);
TEST(a, b, 1, 0x1d, CRF_LT);
/* maximum negative BCD value */
a = (((__int128) 0x9999999999999999) << 64 | 0x999999999999999d);
/* positive one BCD value */
b = (__int128) 0x1c;
TEST(a, b, 0, 0xd, (CRF_LT | CRF_SO));
TEST(a, b, 1, 0xd, (CRF_LT | CRF_SO));
}
void test_bcdsub_invalid(void)
{
__int128 a, b;
/* positive one BCD value */
a = (__int128) 0x1c;
b = 0xf00;
TEST(a, b, 0, UNDEF, CRF_SO);
TEST(a, b, 1, UNDEF, CRF_SO);
TEST(b, a, 0, UNDEF, CRF_SO);
TEST(b, a, 1, UNDEF, CRF_SO);
a = 0xbad;
TEST(a, b, 0, UNDEF, CRF_SO);
TEST(a, b, 1, UNDEF, CRF_SO);
}
int main(void)
{
struct sigaction action;
action.sa_handler = _exit;
sigaction(SIGABRT, &action, NULL);
test_bcdsub_eq();
test_bcdsub_gt();
test_bcdsub_lt();
test_bcdsub_invalid();
return 0;
}