[Beignet] [PATCH] LibOcl: Fix float convert to long/ulong bug.

[Yang, Rong R]

> -----Original Message-----
> From: Song, Ruiling
> Sent: Wednesday, October 21, 2015 10:12
> To: Yang, Rong R; beignet at lists.freedesktop.org
> Cc: Yang, Rong R
> Subject: RE: [Beignet] [PATCH] LibOcl: Fix float convert to long/ulong bug.
> 
> 
> 
> > -----Original Message-----
> > From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf
> > Of Yang Rong
> > Sent: Tuesday, October 20, 2015 4:39 PM
> > To: beignet at lists.freedesktop.org
> > Cc: Yang, Rong R
> > Subject: [Beignet] [PATCH] LibOcl: Fix float convert to long/ulong bug.
> >
> > If the float overflow, convert to long/ulong is undef. So must use
> > long/ulong's max and min value as return value.
> > Also refine long to other integer type sat convert. Use to statement
> > to avoid generate if/else/endif.
> >
> > Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> > ---
> >  backend/src/libocl/script/ocl_convert.sh | 24
> > +++++++++++++++++++++---
> >  1 file changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/backend/src/libocl/script/ocl_convert.sh
> > b/backend/src/libocl/script/ocl_convert.sh
> > index 4f720fe..432bd41 100755
> > --- a/backend/src/libocl/script/ocl_convert.sh
> > +++ b/backend/src/libocl/script/ocl_convert.sh
> > @@ -161,7 +161,8 @@ else
> >      echo '
> >  #define DEF(DSTTYPE, SRCTYPE, MIN, MAX) \  OVERLOADABLE DSTTYPE
> > convert_ ## DSTTYPE ## _sat(SRCTYPE x) { \
> > -  return x >= MAX ? (DSTTYPE)MAX : x <= MIN ? (DSTTYPE)MIN : x; \
> > +  x = x >= MAX ? MAX : x; \
> > +  return x <= MIN ? (DSTTYPE)MIN : (DSTTYPE)x; \
> >  }
> >  '
> >  fi
> > @@ -173,8 +174,25 @@ DEF(short, long, -32768, 32767);  DEF(ushort,
> > long, 0, 65535);  DEF(int, long, -0x7fffffff-1, 0x7fffffff);
> > DEF(uint, long, 0, 0xffffffffu); -DEF(long, float,
> > -9.223372036854776e+18f, 9.223372036854776e+18f); -DEF(ulong, float,
> > 0, 1.8446744073709552e+19f);
> > +#undef DEF
> > +'
> > +
> > +if [ $1"a" = "-pa" ]; then
> > +    echo "
> > +#define DEF(DSTTYPE, SRCTYPE, SRC_MIN, SRC_MAX, DST_MIN,
> DST_MAX) \
> > +OVERLOADABLE DSTTYPE convert_ ## DSTTYPE ## _sat(SRCTYPE x);"
> > +else
> > +    echo '
> > +#define DEF(DSTTYPE, SRCTYPE, SRC_MIN, SRC_MAX, DST_MIN,
> DST_MAX) \
> > +OVERLOADABLE DSTTYPE convert_ ## DSTTYPE ## _sat(SRCTYPE x) { \
> > +  return x >= SRC_MAX ? DST_MAX : x <= SRC_MIN ? (DSTTYPE)DST_MIN :
> > (DSTTYPE)x; \
> 
> Why don't you use two sentences here？suppose this kind of writing would
> generate if-else blocks?
> Other parts LGTM.
It is hard to use two sentences. Think about two methods:

x = x >= SRC_MAX ? DST_MAX : x;
return x <= SRC_ MIN? (DSTTYPE) DST_MIN: (DSTTYPE)x;
The second sentences may convert SRC_MAX to DSTTYPE  and return, because SRC_MAX is overflow, so the value may be undefined.

Another methods:
DSTTYPE y = x >= SRC_MAX ? DST_MAX: (DSTTYPE)x; 
return x <= SRC_MIN ? (DSTTYPE) DST_MIN : (DSTTYPE)y;
But in the first sentences, when convert x to y, the x may underflow, so the y also may be undefined.

Don't you have other suggestion?