[Piglit] [PATCH 2/2] cl: Change data types of char/short buffers in integer limits tests

[Jan Vesely]

On Thu, 2015-09-17 at 10:34 -0500, Aaron Watry wrote:
> On Wed, Sep 16, 2015 at 7:33 PM, Jan Vesely <jan.vesely at rutgers.edu>
> wrote:
> 
> > On Wed, 2015-09-16 at 17:18 -0500, Aaron Watry wrote:
> > > On Tue, Sep 15, 2015 at 9:42 AM, Jan Vesely <
> > > jan.vesely at rutgers.edu>
> > > wrote:
> > > 
> > > > 
> > > > 
> > > > On Tue, Sep 15, 2015 at 7:28 AM, Jan Vesely <
> > > > jan.vesely at rutgers.edu
> > > > > 
> > > > wrote:
> > > > 
> > > > > On Thu, 2015-09-10 at 10:12 -0500, Aaron Watry wrote:
> > > > > > The char/short return buffers were declared as ints.
> > > > > > 
> > > > > > Signed-off-by: Aaron Watry <awatry at gmail.com>
> > > > > 
> > > > > Reviewed-by: Jan Vesely <jan.vesely at rutgers.edu>
> > > > > For both patches.
> > > > > 
> > > > > though, I agree with Serge that a spec reference would be
> > > > > nice.
> > > > > 
> > > > 
> > > > PS: don't we need to test this for *_MAX too? I'd expect at
> > > > least
> > > > char and
> > > > short to have the same problem.
> > > > 
> > > 
> > > 
> > > Yes, it seems that the _MAX values are also broken because the
> > > values
> > > are
> > > upgraded to ints by llvm.
> > 
> > I checked the c99 specs, section 6.4.4.1 says that the type decimal
> > constants without suffix is the first of int, long int, long long
> > int,
> > in which the value can be represented.
> > So I guess llvm is doing the right thing here.
> > 
> > 
> Yeah, I believe it...  It's just sad that the CL spec doesn't take
> into
> account that the main usage of [CHAR|SHORT]_[MIN|MAX] will be for
> comparisons that possibly include values of those types (or something
> smaller than an int).
> 
> I'm updating the patches to send the _MAX values for each type
> through a
> vector/scalar round trip.
> 
> While I'm at it, do we have any consensus on how the CHAR_BIT macro
> should
> be defined?  The value of CHAR_BIT is the number of bits in a byte,
> but I'm
> not really sure if it makes sense to cast it to char as well
> (probably
> doesn't hurt anything), or if we should leave it as an int.

I did a bit more digging, and it turns out that both c99 and OpenCL
require that "The values shall all be constant expressions suitable for
use in #if preprocessing directives." (section 5.2.4.2.1 and 6.12.3).
type casts don't work with preprocessor, so I think adding them is
against the specs.


Jan



> 
> 
> > Jan
> > 
> > > 
> > > 
> > > > 
> > > > > Jan
> > > > > 
> > > > > > ---
> > > > > >  tests/cl/program/execute/int-definitions.cl | 8 ++++----
> > > > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/tests/cl/program/execute/int-definitions.cl
> > > > > > b/tests/cl/program/execute/int-definitions.cl
> > > > > > index 3d8ee63..a438fe4 100644
> > > > > > --- a/tests/cl/program/execute/int-definitions.cl
> > > > > > +++ b/tests/cl/program/execute/int-definitions.cl
> > > > > > @@ -12,12 +12,12 @@ global_size: 1 0 0
> > > > > >  [test]
> > > > > >  name: Char Definitions
> > > > > >  kernel_name: test_char
> > > > > > -arg_out: 0 buffer int[6] 8 127 -128 127 -128 255
> > > > > > +arg_out: 0 buffer char[6] 8 127 -128 127 -128 255
> > > > > > 
> > > > > >  [test]
> > > > > >  name: Short Definitions
> > > > > >  kernel_name: test_short
> > > > > > -arg_out: 0 buffer int[3] 32767 -32768 65535
> > > > > > +arg_out: 0 buffer short[3] 32767 -32768 65535
> > > > > > 
> > > > > >  [test]
> > > > > >  name: Int Definitions
> > > > > > @@ -32,7 +32,7 @@ arg_out: 0 buffer long[3]
> > > > > > 9223372036854775807
> > > > > > \
> > > > > >                            18446744073709551615
> > > > > >  !*/
> > > > > > 
> > > > > > -kernel void test_char(global int* out) {
> > > > > > +kernel void test_char(global char* out) {
> > > > > >    int i = 0;
> > > > > >    out[i++] = CHAR_BIT;
> > > > > >    out[i++] = CHAR_MAX;
> > > > > > @@ -42,7 +42,7 @@ kernel void test_char(global int* out) {
> > > > > >    out[i++] = UCHAR_MAX;
> > > > > >  }
> > > > > > 
> > > > > > -kernel void test_short(global int* out) {
> > > > > > +kernel void test_short(global short* out) {
> > > > > >    int i = 0;
> > > > > >    out[i++] = SHRT_MAX;
> > > > > >    out[i++] = (SHRT_MIN - (short2)(0)).s0;
> > > > > 
> > > > 
> > > > 
> > 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150917/3d91fd31/attachment.sig>