From patchwork Mon Feb 25 15:45:40 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Petar Jovanovic X-Patchwork-Id: 222948 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id A41EB2C02BF for ; Tue, 26 Feb 2013 03:01:18 +1100 (EST) Received: from localhost ([::1]:53480 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UA0UM-0003zd-QL for incoming@patchwork.ozlabs.org; Mon, 25 Feb 2013 11:01:14 -0500 Received: from eggs.gnu.org ([208.118.235.92]:32885) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UA0UB-0003rV-HW for qemu-devel@nongnu.org; Mon, 25 Feb 2013 11:01:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UA0U8-00049H-N9 for qemu-devel@nongnu.org; Mon, 25 Feb 2013 11:01:03 -0500 Received: from mail.rt-rk.ftn.uns.ac.rs ([147.91.177.140]:53851 helo=mail.rt-rk.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UA0H7-00085m-Vu for qemu-devel@nongnu.org; Mon, 25 Feb 2013 10:47:34 -0500 Received: from mail.rt-rk.com (mail.localdomain [127.0.0.1]) by mail.rt-rk.com (Postfix) with SMTP id 6599125BB42 for ; Mon, 25 Feb 2013 16:47:30 +0100 (CET) X-Virus-Scanned: amavisd-new at rt-rk.com From: Petar Jovanovic To: qemu-devel@nongnu.org Date: Mon, 25 Feb 2013 16:45:40 +0100 Message-Id: <1361807140-60543-1-git-send-email-petar.jovanovic@rt-rk.com> X-Mailer: git-send-email 1.7.9.5 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 147.91.177.140 Cc: petar.jovanovic@imgtec.com, aurelien@aurel32.net Subject: [Qemu-devel] [PATCH] target-mips: fix DSP overflow macro and affected routines X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Petar Jovanovic The previous implementation incorrectly used same macro to detect overflow for addition and subtraction. This patch makes distinction between these two, and creates separate macros. The affected routines are changed accordingly. This change also includes additions to the existing tests for SUBQ_S_PH and SUBQ_S_W that would trigger the fixed issue, and it removes dead code from the test file. The last test case in subq_s_w.c is a bug found/reported/ isolated by Klaus Peichl from Dolby. Signed-off-by: Petar Jovanovic --- target-mips/dsp_helper.c | 90 ++++++++++++++++++--------------- tests/tcg/mips/mips32-dsp/subq_s_ph.c | 22 +++++++- tests/tcg/mips/mips32-dsp/subq_s_w.c | 36 +++++++++---- 3 files changed, 94 insertions(+), 54 deletions(-) diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c index 841f47b..ffa9396 100644 --- a/target-mips/dsp_helper.c +++ b/target-mips/dsp_helper.c @@ -44,7 +44,8 @@ typedef union { /*** MIPS DSP internal functions begin ***/ #define MIPSDSP_ABS(x) (((x) >= 0) ? x : -x) -#define MIPSDSP_OVERFLOW(a, b, c, d) (!(!((a ^ b ^ -1) & (a ^ c) & d))) +#define MIPSDSP_OVERFLOW_ADD(a, b, c, d) (~(a ^ b) & (a ^ c) & d) +#define MIPSDSP_OVERFLOW_SUB(a, b, c, d) ((a ^ b) & (a ^ c) & d) static inline void set_DSPControl_overflow_flag(uint32_t flag, int position, CPUMIPSState *env) @@ -142,7 +143,7 @@ static inline int16_t mipsdsp_add_i16(int16_t a, int16_t b, CPUMIPSState *env) tempI = a + b; - if (MIPSDSP_OVERFLOW(a, b, tempI, 0x8000)) { + if (MIPSDSP_OVERFLOW_ADD(a, b, tempI, 0x8000)) { set_DSPControl_overflow_flag(1, 20, env); } @@ -156,7 +157,7 @@ static inline int16_t mipsdsp_sat_add_i16(int16_t a, int16_t b, tempS = a + b; - if (MIPSDSP_OVERFLOW(a, b, tempS, 0x8000)) { + if (MIPSDSP_OVERFLOW_ADD(a, b, tempS, 0x8000)) { if (a > 0) { tempS = 0x7FFF; } else { @@ -175,7 +176,7 @@ static inline int32_t mipsdsp_sat_add_i32(int32_t a, int32_t b, tempI = a + b; - if (MIPSDSP_OVERFLOW(a, b, tempI, 0x80000000)) { + if (MIPSDSP_OVERFLOW_ADD(a, b, tempI, 0x80000000)) { if (a > 0) { tempI = 0x7FFFFFFF; } else { @@ -858,7 +859,7 @@ static inline uint16_t mipsdsp_sub_i16(int16_t a, int16_t b, CPUMIPSState *env) int16_t temp; temp = a - b; - if (MIPSDSP_OVERFLOW(a, -b, temp, 0x8000)) { + if (MIPSDSP_OVERFLOW_SUB(a, b, temp, 0x8000)) { set_DSPControl_overflow_flag(1, 20, env); } @@ -871,8 +872,8 @@ static inline uint16_t mipsdsp_sat16_sub(int16_t a, int16_t b, int16_t temp; temp = a - b; - if (MIPSDSP_OVERFLOW(a, -b, temp, 0x8000)) { - if (a > 0) { + if (MIPSDSP_OVERFLOW_SUB(a, b, temp, 0x8000)) { + if (a >= 0) { temp = 0x7FFF; } else { temp = 0x8000; @@ -889,8 +890,8 @@ static inline uint32_t mipsdsp_sat32_sub(int32_t a, int32_t b, int32_t temp; temp = a - b; - if (MIPSDSP_OVERFLOW(a, -b, temp, 0x80000000)) { - if (a > 0) { + if (MIPSDSP_OVERFLOW_SUB(a, b, temp, 0x80000000)) { + if (a >= 0) { temp = 0x7FFFFFFF; } else { temp = 0x80000000; @@ -1004,7 +1005,7 @@ static inline uint32_t mipsdsp_sub32(int32_t a, int32_t b, CPUMIPSState *env) int32_t temp; temp = a - b; - if (MIPSDSP_OVERFLOW(a, -b, temp, 0x80000000)) { + if (MIPSDSP_OVERFLOW_SUB(a, b, temp, 0x80000000)) { set_DSPControl_overflow_flag(1, 20, env); } @@ -1017,7 +1018,7 @@ static inline int32_t mipsdsp_add_i32(int32_t a, int32_t b, CPUMIPSState *env) temp = a + b; - if (MIPSDSP_OVERFLOW(a, b, temp, 0x80000000)) { + if (MIPSDSP_OVERFLOW_ADD(a, b, temp, 0x80000000)) { set_DSPControl_overflow_flag(1, 20, env); } @@ -2488,37 +2489,42 @@ DP_QH(dpsq_s_w_qh, 0, 1); #endif #define DP_L_W(name, is_add) \ -void helper_##name(uint32_t ac, target_ulong rs, target_ulong rt, \ - CPUMIPSState *env) \ -{ \ - int32_t temp63; \ - int64_t dotp, acc; \ - uint64_t temp; \ - \ - dotp = mipsdsp_mul_q31_q31(ac, rs, rt, env); \ - acc = ((uint64_t)env->active_tc.HI[ac] << 32) | \ - ((uint64_t)env->active_tc.LO[ac] & MIPSDSP_LLO); \ - if (!is_add) { \ - dotp = -dotp; \ - } \ - \ - temp = acc + dotp; \ - if (MIPSDSP_OVERFLOW((uint64_t)acc, (uint64_t)dotp, temp, \ - (0x01ull << 63))) { \ - temp63 = (temp >> 63) & 0x01; \ - if (temp63 == 1) { \ - temp = (0x01ull << 63) - 1; \ - } else { \ - temp = 0x01ull << 63; \ - } \ - \ - set_DSPControl_overflow_flag(1, 16 + ac, env); \ - } \ - \ - env->active_tc.HI[ac] = (target_long)(int32_t) \ - ((temp & MIPSDSP_LHI) >> 32); \ - env->active_tc.LO[ac] = (target_long)(int32_t) \ - (temp & MIPSDSP_LLO); \ +void helper_##name(uint32_t ac, target_ulong rs, target_ulong rt, \ + CPUMIPSState *env) \ +{ \ + int32_t temp63; \ + int64_t dotp, acc; \ + uint64_t temp; \ + bool overflow; \ + \ + dotp = mipsdsp_mul_q31_q31(ac, rs, rt, env); \ + acc = ((uint64_t)env->active_tc.HI[ac] << 32) | \ + ((uint64_t)env->active_tc.LO[ac] & MIPSDSP_LLO); \ + if (is_add) { \ + temp = acc + dotp; \ + overflow = MIPSDSP_OVERFLOW_ADD((uint64_t)acc, (uint64_t)dotp, \ + temp, (0x01ull << 63)); \ + } else { \ + temp = acc - dotp; \ + overflow = MIPSDSP_OVERFLOW_SUB((uint64_t)acc, (uint64_t)dotp, \ + temp, (0x01ull << 63)); \ + } \ + \ + if (overflow) { \ + temp63 = (temp >> 63) & 0x01; \ + if (temp63 == 1) { \ + temp = (0x01ull << 63) - 1; \ + } else { \ + temp = 0x01ull << 63; \ + } \ + \ + set_DSPControl_overflow_flag(1, 16 + ac, env); \ + } \ + \ + env->active_tc.HI[ac] = (target_long)(int32_t) \ + ((temp & MIPSDSP_LHI) >> 32); \ + env->active_tc.LO[ac] = (target_long)(int32_t) \ + (temp & MIPSDSP_LLO); \ } DP_L_W(dpaq_sa_l_w, 1); diff --git a/tests/tcg/mips/mips32-dsp/subq_s_ph.c b/tests/tcg/mips/mips32-dsp/subq_s_ph.c index 8e36dad..64c89eb 100644 --- a/tests/tcg/mips/mips32-dsp/subq_s_ph.c +++ b/tests/tcg/mips/mips32-dsp/subq_s_ph.c @@ -12,7 +12,8 @@ int main() resultdsp = 0x01; __asm - ("subq_s.ph %0, %2, %3\n\t" + ("wrdsp $0\n\t" + "subq_s.ph %0, %2, %3\n\t" "rddsp %1\n\t" : "=r"(rd), "=r"(dsp) : "r"(rs), "r"(rt) @@ -27,7 +28,24 @@ int main() resultdsp = 0x01; __asm - ("subq_s.ph %0, %2, %3\n\t" + ("wrdsp $0\n\t" + "subq_s.ph %0, %2, %3\n\t" + "rddsp %1\n\t" + : "=r"(rd), "=r"(dsp) + : "r"(rs), "r"(rt) + ); + dsp = (dsp >> 20) & 0x01; + assert(dsp == resultdsp); + assert(rd == result); + + rs = 0x12340000; + rt = 0x87658000; + result = 0x7FFF7FFF; + resultdsp = 0x01; + + __asm + ("wrdsp $0\n\t" + "subq_s.ph %0, %2, %3\n\t" "rddsp %1\n\t" : "=r"(rd), "=r"(dsp) : "r"(rs), "r"(rt) diff --git a/tests/tcg/mips/mips32-dsp/subq_s_w.c b/tests/tcg/mips/mips32-dsp/subq_s_w.c index 09022e9..9d456a9 100644 --- a/tests/tcg/mips/mips32-dsp/subq_s_w.c +++ b/tests/tcg/mips/mips32-dsp/subq_s_w.c @@ -12,7 +12,8 @@ int main() resultdsp = 0x01; __asm - ("subq_s.w %0, %2, %3\n\t" + ("wrdsp $0\n\t" + "subq_s.w %0, %2, %3\n\t" "rddsp %1\n\t" : "=r"(rd), "=r"(dsp) : "r"(rs), "r"(rt) @@ -24,10 +25,11 @@ int main() rs = 0x66666; rt = 0x55555; result = 0x11111; - resultdsp = 0x01; + resultdsp = 0x0; __asm - ("subq_s.w %0, %2, %3\n\t" + ("wrdsp $0\n\t" + "subq_s.w %0, %2, %3\n\t" "rddsp %1\n\t" : "=r"(rd), "=r"(dsp) : "r"(rs), "r"(rt) @@ -36,23 +38,37 @@ int main() assert(dsp == resultdsp); assert(rd == result); - -#if 0 - rs = 0x35555555; - rt = 0xf5555555; - result = 0x80000000; + rs = 0x0; + rt = 0x80000000; + result = 0x7FFFFFFF; resultdsp = 0x01; __asm - ("subq_s.w %0, %2, %3\n\t" + ("wrdsp $0\n\t" + "subq_s.w %0, %2, %3\n\t" "rddsp %1\n\t" : "=r"(rd), "=r"(dsp) : "r"(rs), "r"(rt) ); + dsp = (dsp >> 20) & 0x01; + assert(dsp == resultdsp); + assert(rd == result); + + rs = 0x80000000; + rt = 0x80000000; + result = 0; + resultdsp = 0x00; + __asm + ("wrdsp $0\n\t" + "subq_s.w %0, %2, %3\n\t" + "rddsp %1\n\t" + : "=r"(rd), "=r"(dsp) + : "r"(rs), "r"(rt) + ); dsp = (dsp >> 20) & 0x01; assert(dsp == resultdsp); assert(rd == result); -#endif + return 0; }