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

[Aaron Watry via Libclc-dev]

On Fri, Sep 11, 2015 at 1:28 PM, Tom Stellard <tom at stellard.net> wrote:

> On Thu, Sep 10, 2015 at 08:28:02PM -0500, Aaron Watry wrote:
> > 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.
> >
>
> I don't mind using the values from the spec.  The only change I would
> request
> is that we cast INT_MIN to int like we do for char and short.  I think the
> compiler is still allowed to evaluate that expression to long if it wants
> to.
>
>
Works for me.  v2 incoming.


> -Tom
>
> > --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/20150911/23690cb5/attachment.html>