[Libclc-dev] [PATCH] Add missing convert_*_sat functions

[Aaron Watry]

You are correct.  You can use _sat for converting to integer types,
but not for converting to floating types:

6.2.3.3 Out-of-Range Behavior and Saturated Conversions
...
Conversions to integer type may opt to convert using the optional
saturated mode by appending
the _sat modifier to the conversion function name. When in saturated
mode, values that are
outside the representable range shall clamp to the nearest
representable value in the destination
format. (NaN should be converted to 0).

Conversions to floating-point type shall conform to IEEE-754 rounding
rules. The _sat
modifier may not be used for conversions to floating-point formats.


In that case, feel free to remove the entire #ifdef block for
saturated conversions with floating types as destinations.

--Aaron

On Tue, Aug 13, 2013 at 9:53 AM, Tom Stellard <tom at stellard.net> wrote:
> On Mon, Aug 12, 2013 at 07:53:01PM -0500, Aaron Watry wrote:
>> On Mon, Aug 12, 2013 at 5:11 PM, Tom Stellard <tom at stellard.net> wrote:
>> > From: Tom Stellard <thomas.stellard at amd.com>
>> >
>> > ---
>> >  generic/lib/convert.cl | 48 ++++++++++++++++++++++++++----------------------
>> >  1 file changed, 26 insertions(+), 22 deletions(-)
>> >
>> > diff --git a/generic/lib/convert.cl b/generic/lib/convert.cl
>> > index c2ac9be..1b98d51 100644
>> > --- a/generic/lib/convert.cl
>> > +++ b/generic/lib/convert.cl
>> > @@ -72,32 +72,36 @@
>> >
>> >  CONVERT_ID_TO()
>> >
>> > -_CLC_OVERLOAD _CLC_DEF char convert_char_sat(long l) {
>> > -  return l > 127 ? 127 : l < -128 ? -128 : l;
>> > -}
>> > -
>> > -_CLC_OVERLOAD _CLC_DEF uchar convert_uchar_sat(ulong l) {
>> > -  return l > 255U ? 255U : l;
>> > -}
>> > -
>> > -_CLC_OVERLOAD _CLC_DEF short convert_short_sat(long l) {
>> > -  return l > 32767 ? 32767 : l < -32768 ? -32768 : l;
>> > -}
>> > +#define SAT_IMPL(l, min, max)  ((l) > (max) ? (max) : (l) < (min) ? (min) : (l));
>> >
>> > -_CLC_OVERLOAD _CLC_DEF ushort convert_ushort_sat(ulong l) {
>> > -  return l > 65535U ? 65535U : l;
>> > +#define CONVERT_SAT(FROM_TYPE, TO_TYPE, MIN, MAX) \
>> > +  _CLC_OVERLOAD _CLC_DEF TO_TYPE convert_##TO_TYPE##_sat(FROM_TYPE l) { \
>> > +  return (TO_TYPE)SAT_IMPL(l, ((FROM_TYPE)(MIN)), ((FROM_TYPE)(MAX))); \
>> >  }
>> >
>> > -_CLC_OVERLOAD _CLC_DEF int convert_int_sat(long l) {
>> > -  return l > ((1L<<31)-1) ? ((1L<<31L)-1) : l < -(1L<<31) ? -(1L<<31) : l;
>> > -}
>> > -
>> > -_CLC_OVERLOAD _CLC_DEF uint convert_uint_sat(ulong l) {
>> > -  return l > ((1UL<<32)-1) ? ((1UL<<32)-1) : l;
>> > -}
>> > +#define CONVERT_SAT_FROM(FROM_TYPE) \
>> > +  CONVERT_SAT(FROM_TYPE, char, CHAR_MIN, CHAR_MAX) \
>> > +  CONVERT_SAT(FROM_TYPE, uchar, 0, UCHAR_MAX) \
>> > +  CONVERT_SAT(FROM_TYPE, short, SHRT_MIN, SHRT_MAX) \
>> > +  CONVERT_SAT(FROM_TYPE, ushort, 0, USHRT_MAX) \
>> > +  CONVERT_SAT(FROM_TYPE, int, INT_MIN, INT_MAX) \
>> > +  CONVERT_SAT(FROM_TYPE, uint, 0, UINT_MAX) \
>> > +  CONVERT_SAT(FROM_TYPE, long, LONG_MIN, LONG_MAX) \
>> > +  CONVERT_SAT(FROM_TYPE, ulong, 0, ULONG_MAX)
>> > +
>> > +CONVERT_SAT_FROM(long);
>> > +CONVERT_SAT_FROM(ulong);
>> > +CONVERT_SAT_FROM(int);
>> > +CONVERT_SAT_FROM(uint);
>> > +CONVERT_SAT_FROM(short);
>> > +CONVERT_SAT_FROM(ushort);
>> > +CONVERT_SAT_FROM(char);
>> > +CONVERT_SAT_FROM(uchar);
>> > +CONVERT_SAT_FROM(float);
>> > +#ifdef cl_khr_fp64
>> > +CONVERT_SAT_FROM(double);
>> > +#endif
>> >
>> > -CONVERT_ID(long, long, _sat)
>> > -CONVERT_ID(ulong, ulong, _sat)
>> >  #ifdef cl_khr_fp64
>> >  CONVERT_ID(double, float, _sat)
>> >  CONVERT_ID(double, double, _sat)
>>
>> It got cut-off by git, but while I was looking through this change
>> set, I thought that this #ifdef looked suspect.
>> Shouldn't the true branch also have the following:
>> CONVERT_ID(float, float, _sat)
>>
>> It's present in the #else branch, just not the first branch.
>>
>
>
> Actually, I think all the code inside this #ifdef should be removed,
> because you the OpenCL spec says you cant use the _sat modifier for
> conversions to floating-point formats.
>
> -Tom
>
>
>> For this patch:
>> Reviewed-by: Aaron Watry <awatry at gmail.com>
>>
>> > --
>> > 1.7.11.4
>> >
>> >
>> > _______________________________________________
>> > Libclc-dev mailing list
>> > Libclc-dev at pcc.me.uk
>> > http://www.pcc.me.uk/cgi-bin/mailman/listinfo/libclc-dev