From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58832) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RNmZX-0002jK-Ew for qemu-devel@nongnu.org; Tue, 08 Nov 2011 09:22:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RNmZS-00031p-Ff for qemu-devel@nongnu.org; Tue, 08 Nov 2011 09:22:43 -0500 Received: from mail.windriver.com ([147.11.1.11]:38923) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RNmZS-00031e-5w for qemu-devel@nongnu.org; Tue, 08 Nov 2011 09:22:38 -0500 Message-ID: <4EB93B29.9070204@windriver.com> Date: Tue, 8 Nov 2011 08:22:33 -0600 From: Jason Wessel MIME-Version: 1.0 References: <1320757252-29290-1-git-send-email-jason.wessel@windriver.com> In-Reply-To: Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] target-i386: Fix regression with maxsd SSE2 instruction List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Desnogues Cc: qemu-devel@nongnu.org On 11/08/2011 07:48 AM, Laurent Desnogues wrote: > On Tue, Nov 8, 2011 at 2:00 PM, Jason Wessel wrote: >> The maxsd instruction needs to take into account the sign of the >> numbers 64 bit numbers. This is a regression that was introduced in >> 347ac8e356 (target-i386: switch to softfloat). >> >> The case that fails is: >> >> maxsd %xmm1,%xmm0 >> >> When xmm1 = 24 and xmm0 = -100 >> >> This was found running the glib2 binding tests where it prints the message: >> /binding/transform: >> GLib-GObject-WARNING **: value "24.000000" of type `gdouble' is invalid or out of range for property `value' of type `gdouble' >> aborting... >> >> Using a signed comparison fixes the problem. >> >> Signed-off-by: Jason Wessel >> --- >> target-i386/ops_sse.h | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/target-i386/ops_sse.h b/target-i386/ops_sse.h >> index aa41d25..bcc0ed9 100644 >> --- a/target-i386/ops_sse.h >> +++ b/target-i386/ops_sse.h >> @@ -584,8 +584,8 @@ void helper_ ## name ## sd (Reg *d, Reg *s)\ >> #define FPU_SUB(size, a, b) float ## size ## _sub(a, b, &env->sse_status) >> #define FPU_MUL(size, a, b) float ## size ## _mul(a, b, &env->sse_status) >> #define FPU_DIV(size, a, b) float ## size ## _div(a, b, &env->sse_status) >> -#define FPU_MIN(size, a, b) (a) < (b) ? (a) : (b) >> -#define FPU_MAX(size, a, b) (a) > (b) ? (a) : (b) > Isn't maxsd a floating-point instruction? If so, shouldn't > FPU_{MIN,MAX} use softfloat operations? You are correct. It should be: +#define FPU_MIN(size, a, b) float ## size ## _lt(a, b, &env->sse_status) ? (a) : (b) +#define FPU_MAX(size, a, b) float ## size ## _lt(b, a, &env->sse_status) ? (a) : (b) Jason. > > > Laurent > >> +#define FPU_MIN(size, a, b) (int ## size ## _t)(a) < (int ## size ## _t)(b) ? (a) : (b) >> +#define FPU_MAX(size, a, b) (int ## size ## _t)(a) > (int ## size ## _t)(b) ? (a) : (b) >> #define FPU_SQRT(size, a, b) float ## size ## _sqrt(b, &env->sse_status) >> >> SSE_HELPER_S(add, FPU_ADD) >> -- >> 1.7.1 >> >> >>