[Libclc-dev] [PATCH] integer: Update integer limits to comply with spec

[Aaron Watry via Libclc-dev]

On Thu, Sep 10, 2015 at 12:10 PM, Tom Stellard <tom at stellard.net> wrote:

> On Thu, Sep 10, 2015 at 10:14:34AM -0500, Aaron Watry via Libclc-dev wrote:
> > The values for the char/short/integer/long minimums were declared with
> their actual values, not the definitions from the CL spec (v1.1).  As a
> result, (-2147483648) was actually being treated as a long by the
> compiler, not an int, which caused issues when trying to add/subtract that
> value from a vector.
> >
> > Update the definitions to use the values declared by the spec, and also
> add explicit casts for the char/short minimums so that the compiler
> actually treats them as shorts/chars. Without those casts, they actually
> end up stored as integers.
> >
> > The compiler can sign extend the values if it needs to conver the
> char->short or short->int.
> >
> > Reported-by: Moritz Pflanzer <moritz.pflanzer14 at imperial.ac.uk>
> > CC: Moritz Pflanzer <moritz.pflanzer14 at imperial.ac.uk>
> > Signed-off-by: Aaron Watry <awatry at gmail.com>
> > ---
> >  generic/include/clc/integer/definitions.h | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/generic/include/clc/integer/definitions.h
> b/generic/include/clc/integer/definitions.h
> > index a407974..505002f 100644
> > --- a/generic/include/clc/integer/definitions.h
> > +++ b/generic/include/clc/integer/definitions.h
> > @@ -1,14 +1,14 @@
> >  #define CHAR_BIT 8
> >  #define INT_MAX 2147483647
> > -#define INT_MIN -2147483648
> > +#define INT_MIN (-2147483647 - 1)
>
> Is there some reason why
>
> #define INT_MIN ((int)-2147483648)
>
> won't work?
>
> I also don't understand how subtracting one guarantees the result will
> be an integer and not long.
>
>
You're right that ((int)-2147483648) would work as well, which is somewhat
similar to what I did with the char/short minimums in this patch (forcibly
casting the result). The only reason that I went with the value that I did
for INT_MIN was that the spec seems pretty specific:

Section 6.11.3 of the 1.1 Spec:
"The macro names given in the following list must use the values specified.
The values shall all
be constant expressions suitable for use in #if preprocessing directives."

<snip>
#define INT_MIN (-2147483647 – 1)

The only reason that I put an explicit cast in for the char/short values
was because they would not actually work correctly for charN/shortN casts
without them, and it seems to be the intention behind the spec that the
MIN/MAX of a given type should be stored as that type.

I don't personally care that much what we end up with, as the result after
preprocessing should be identical...  as long as the new unit tests pass,
I'm good.

--Aaron


> -Tom
>
> >  #define LONG_MAX  0x7fffffffffffffffL
> > -#define LONG_MIN -0x8000000000000000L
> > +#define LONG_MIN (-0x7fffffffffffffffL - 1)
> > +#define CHAR_MAX SCHAR_MAX
> > +#define CHAR_MIN SCHAR_MIN
> >  #define SCHAR_MAX 127
> > -#define SCHAR_MIN -128
> > -#define CHAR_MAX 127
> > -#define CHAR_MIN -128
> > +#define SCHAR_MIN ((char)(-127 - 1))
> >  #define SHRT_MAX 32767
> > -#define SHRT_MIN -32768
> > +#define SHRT_MIN ((short)(-32767 - 1))
> >  #define UCHAR_MAX 255
> >  #define USHRT_MAX 65535
> >  #define UINT_MAX 0xffffffff
> > --
> > 2.1.4
> >
> > _______________________________________________
> > Libclc-dev mailing list
> > Libclc-dev at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/libclc-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libclc-dev/attachments/20150910/49aef8b1/attachment.html>