[Beignet] [BUG REPORT] nested if-else not work for 64bit integer

[Xing, Homer]

The piglit test case is already a unit test case ... 
-----Original Message-----
From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com] 
Sent: Friday, August 16, 2013 3:32 PM
To: Xing, Homer
Cc: beignet at lists.freedesktop.org
Subject: Re: [Beignet] [BUG REPORT] nested if-else not work for 64bit integer

Use the following patch, you can get a unit test case for this bug. 

>From 66a0fe1bee293c5b2ab54c060c76c63cc1bcd378 Mon Sep 17 00:00:00 2001
From: Zhigang Gong <zhigang.gong at linux.intel.com>
Date: Fri, 16 Aug 2013 15:28:52 +0800
Subject: [PATCH] Utests: enable long/ulong for abs_diff test case.

Signed-off-by: Zhigang Gong <zhigang.gong at linux.intel.com>
---
 utests/compiler_abs_diff.cpp |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/utests/compiler_abs_diff.cpp b/utests/compiler_abs_diff.cpp index 384a654..7491aac 100644
--- a/utests/compiler_abs_diff.cpp
+++ b/utests/compiler_abs_diff.cpp
@@ -193,10 +193,13 @@ template <typename T, typename U> static void compiler_abs_diff_with_type(void)  typedef unsigned char uchar;  typedef unsigned short ushort;  typedef unsigned int uint;
+typedef unsigned long ulong;
 ABS_TEST_DIFF_TYPE(int, uint)
+ABS_TEST_DIFF_TYPE(long, ulong)
 ABS_TEST_DIFF_TYPE(short, ushort)
 ABS_TEST_DIFF_TYPE(char, uchar)
 ABS_TEST_DIFF_TYPE(uint, uint)
+ABS_TEST_DIFF_TYPE(ulong, ulong)
 ABS_TEST_DIFF_TYPE(ushort, ushort)
 ABS_TEST_DIFF_TYPE(uchar, uchar)
 
@@ -222,6 +225,26 @@ ABS_TEST_DIFF_TYPE(uint4, uint4)  ABS_TEST_DIFF_TYPE(uint8, uint8)  ABS_TEST_DIFF_TYPE(uint16, uint16)
 
+typedef cl_vec<long, 2> long2;
+typedef cl_vec<long, 3> long3;
+typedef cl_vec<long, 4> long4;
+typedef cl_vec<long, 8> long8;
+typedef cl_vec<long, 16> long16;
+typedef cl_vec<unsigned long, 2> ulong2; typedef cl_vec<unsigned long, 
+3> ulong3; typedef cl_vec<unsigned long, 4> ulong4; typedef 
+cl_vec<unsigned long, 8> ulong8; typedef cl_vec<unsigned long, 16> 
+ulong16; ABS_TEST_DIFF_TYPE(long2, ulong2) ABS_TEST_DIFF_TYPE(long3, 
+ulong3) ABS_TEST_DIFF_TYPE(long4, ulong4) ABS_TEST_DIFF_TYPE(long8, 
+ulong8) ABS_TEST_DIFF_TYPE(long16, ulong16) ABS_TEST_DIFF_TYPE(ulong2, 
+ulong2) ABS_TEST_DIFF_TYPE(ulong3, ulong3) ABS_TEST_DIFF_TYPE(ulong4, 
+ulong4) ABS_TEST_DIFF_TYPE(ulong8, ulong8) ABS_TEST_DIFF_TYPE(ulong16, 
+ulong16)
 
 typedef cl_vec<char, 2> char2;
 typedef cl_vec<char, 3> char3;
--
1.7.9.5



On Fri, Aug 16, 2013 at 03:10:38PM +0800, Zhigang Gong wrote:
> Thanks for the patch and bug report. Could you take this bug?
> 
> On Fri, Aug 16, 2013 at 09:21:43AM +0800, Homer Hsing wrote:
> > 64bit version "abs_diff()" uses two-level-nested if-else.
> > GEN_IR is correct. But GEN_ASM may be wrong.
> > 
> > To reproduce the bug:
> >   first apply patch "add empty 64bit-integer version built-in functions".
> >   then apply this patch.
> >   then in piglit, run
> > bin/cl-program-tester generated_tests/cl/builtin/int/builtin-long-abs_diff-1.0.generated.cl
> > 
> > ---
> >  backend/src/ocl_stdlib.tmpl.h | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/backend/src/ocl_stdlib.tmpl.h b/backend/src/ocl_stdlib.tmpl.h
> > index 00d6fda..bcb8f1a 100644
> > --- a/backend/src/ocl_stdlib.tmpl.h
> > +++ b/backend/src/ocl_stdlib.tmpl.h
> > @@ -507,10 +507,13 @@ INLINE_OVERLOADABLE uint abs_diff (int x, int y) {
> >  }
> >  
> >  INLINE_OVERLOADABLE ulong abs_diff (long x, long y) {
> > -  return 0;
> > +    if ((x >= 0 && y >= 0) || (x <= 0 && y <= 0))
> > +        return abs(x - y);
> > +    return abs(x) + abs(y);
> >  }
> > +
> >  INLINE_OVERLOADABLE ulong abs_diff (ulong x, ulong y) {
> > -  return 0;
> > +    return y > x ? y - x : x - y;
> >  }
> >  
> >  /////////////////////////////////////////////////////////////////////////////
> > -- 
> > 1.8.1.2
> > 
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet