[PATCH 1/3] geometric/floatn.inc: Add vec8 and vec16 types

Looks good to me.

Do we have a test case for this somewhere?

–Aaron

This looks fine to me. Do we have any test cases for this?

–Aaron

Things like this are the main thing the OpenCL conformance tests are good at

Do we have a reason to do this expansion to float8/float16 types?

The CL 1.2 and 2.0 specs say that all of the geometric functions operate on float, float2, float3, and float4 data types (and the double equivalents), but make no mention of float8/float16 being supported (which seems a bit silly given that almost everything else supports 8/16 element vectors if they support them at all).

–Aaron

Looks good to me.

Do we have a test case for this somewhere?

--Aaron

Things like this are the main thing the OpenCL conformance tests are
good at

Yeah, that's something that I don't have access to... I'll assume that
someone's been running this against the conformance tests and that the
tests pass.

It just helps to note (and builds confidence in correctness) that new
built-in implementations have been tested against piglit, opencv, the CL
conformance tests, webcl conformance, etc. just to note that the
implementation has been tested on something other than the local dev's
machine.

--Aaron

Do we have a reason to do this expansion to float8/float16 types?

No, this was a mistake. Sorry for the noise.

-Tom

>
> Looks good to me.
>
> Do we have a test case for this somewhere?
>
> --Aaron
>
> Things like this are the main thing the OpenCL conformance tests are
> good at
>
>
Yeah, that's something that I don't have access to... I'll assume that
someone's been running this against the conformance tests and that the
tests pass.

It just helps to note (and builds confidence in correctness) that new
built-in implementations have been tested against piglit, opencv, the CL
conformance tests, webcl conformance, etc. just to note that the
implementation has been tested on something other than the local dev's
machine.

I usually test against piglit, OpenCV, and a subset of the conformance
tests before I send patches. I can add a note in the commit message
that these tests pass conformance.

-Tom

>
> >
> > Looks good to me.
> >
> > Do we have a test case for this somewhere?
> >
> > --Aaron
> >
> > Things like this are the main thing the OpenCL conformance tests are
> > good at
> >
> >
> Yeah, that's something that I don't have access to... I'll assume that
> someone's been running this against the conformance tests and that the
> tests pass.
>
> It just helps to note (and builds confidence in correctness) that new
> built-in implementations have been tested against piglit, opencv, the CL
> conformance tests, webcl conformance, etc. just to note that the
> implementation has been tested on something other than the local dev's
> machine.
>

I usually test against piglit, OpenCV, and a subset of the conformance
tests before I send patches. I can add a note in the commit message
that these tests pass conformance.

-Tom

I almost always check piglit for new test cases, but since OpenCV has a
tendency to lock my machine up (at least in the past), I don't often run
new patches against that.

If you've tested against opencv/conformance, there's a good chance I won't
look there and a note would be helpful :slight_smile:

It's not a requirement by any means, but sometimes it'll save me time if I
know there's other existing test cases that have been run. Mostly because
if I see new built-ins and piglit doesn't have tests for that function, my
first instinct is usually to write tests before finishing my review of the
implementation.

--Aaron