[PATCH 1/4] math/binary_decl.inc: Do not declare mixed float/double functions

fmin/fmax only need vector/scalar mix

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>

Drop unused clc/math/clc_nextafter.h header

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>

using clang builtin results in external library call

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>

LGTM.

-- Jeroen

LGTM.

-- Jeroen

Drop unused clc/math/clc_nextafter.h header

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
---
amdgpu/lib/math/nextafter.cl | 1 +
generic/include/clc/clc.h | 5 -----
generic/include/clc/math/clc_nextafter.h | 11 -----------
generic/include/clc/math/gentype.inc | 2 ++
4 files changed, 3 insertions(+), 16 deletions(-)
delete mode 100644 generic/include/clc/math/clc_nextafter.h

diff --git a/amdgpu/lib/math/nextafter.cl b/amdgpu/lib/math/nextafter.cl
index 6aee0a0..5b4521d 100644
--- a/amdgpu/lib/math/nextafter.cl
+++ b/amdgpu/lib/math/nextafter.cl
@@ -1,5 +1,6 @@
#include <clc/clc.h>
#include "../lib/clcmacro.h"
+#include <math/clc_nextafter.h>

_CLC_DEFINE_BINARY_BUILTIN(float, nextafter, __clc_nextafter, float, float)

diff --git a/generic/include/clc/clc.h b/generic/include/clc/clc.h
index adaab90..3701336 100644
--- a/generic/include/clc/clc.h
+++ b/generic/include/clc/clc.h
@@ -264,9 +264,4 @@
#include <clc/image/image_defines.h>
#include <clc/image/image.h>

-/* libclc internal defintions */
-#ifdef __CLC_INTERNAL
-#include <math/clc_nextafter.h>
-#endif
-
#pragma OPENCL EXTENSION all : disable
diff --git a/generic/include/clc/math/clc_nextafter.h b/generic/include/clc/math/clc_nextafter.h
deleted file mode 100644
index 81c8f36..0000000
--- a/generic/include/clc/math/clc_nextafter.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#define __CLC_BODY <clc/math/binary_decl.inc>
-
-#define __CLC_FUNCTION nextafter
-#include <clc/math/gentype.inc>
-#undef __CLC_FUNCTION
-
-#define __CLC_FUNCTION __clc_nextafter
-#include <clc/math/gentype.inc>
-#undef __CLC_FUNCTION
-
-#undef __CLC_BODY
diff --git a/generic/include/clc/math/gentype.inc b/generic/include/clc/math/gentype.inc
index e6ffad1..954cd00 100644
--- a/generic/include/clc/math/gentype.inc
+++ b/generic/include/clc/math/gentype.inc
@@ -54,6 +54,8 @@

#ifndef __FLOAT_ONLY
#ifdef cl_khr_fp64
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable
+
#define __CLC_SCALAR_GENTYPE double
#define __CLC_FPSIZE 64

I this this pragma being enabled in the header files in other places. I’m somewhat
confused: why is this necessary? Why shouldn’t this only occur in the source files
that implement fp64 functionality?

Jeroen

>
> Drop unused clc/math/clc_nextafter.h header
>
> Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
> ---
> amdgpu/lib/math/nextafter.cl | 1 +
> generic/include/clc/clc.h | 5 -----
> generic/include/clc/math/clc_nextafter.h | 11 -----------
> generic/include/clc/math/gentype.inc | 2 ++
> 4 files changed, 3 insertions(+), 16 deletions(-)
> delete mode 100644 generic/include/clc/math/clc_nextafter.h
>
> diff --git a/amdgpu/lib/math/nextafter.cl b/amdgpu/lib/math/nextafter.cl
> index 6aee0a0..5b4521d 100644
> --- a/amdgpu/lib/math/nextafter.cl
> +++ b/amdgpu/lib/math/nextafter.cl
> @@ -1,5 +1,6 @@
> #include <clc/clc.h>
> #include "../lib/clcmacro.h"
> +#include <math/clc_nextafter.h>
>
> _CLC_DEFINE_BINARY_BUILTIN(float, nextafter, __clc_nextafter, float, float)
>
> diff --git a/generic/include/clc/clc.h b/generic/include/clc/clc.h
> index adaab90..3701336 100644
> --- a/generic/include/clc/clc.h
> +++ b/generic/include/clc/clc.h
> @@ -264,9 +264,4 @@
> #include <clc/image/image_defines.h>
> #include <clc/image/image.h>
>
> -/* libclc internal defintions */
> -#ifdef __CLC_INTERNAL
> -#include <math/clc_nextafter.h>
> -#endif
> -
> #pragma OPENCL EXTENSION all : disable
> diff --git a/generic/include/clc/math/clc_nextafter.h b/generic/include/clc/math/clc_nextafter.h
> deleted file mode 100644
> index 81c8f36..0000000
> --- a/generic/include/clc/math/clc_nextafter.h
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -#define __CLC_BODY <clc/math/binary_decl.inc>
> -
> -#define __CLC_FUNCTION nextafter
> -#include <clc/math/gentype.inc>
> -#undef __CLC_FUNCTION
> -
> -#define __CLC_FUNCTION __clc_nextafter
> -#include <clc/math/gentype.inc>
> -#undef __CLC_FUNCTION
> -
> -#undef __CLC_BODY
> diff --git a/generic/include/clc/math/gentype.inc b/generic/include/clc/math/gentype.inc
> index e6ffad1..954cd00 100644
> --- a/generic/include/clc/math/gentype.inc
> +++ b/generic/include/clc/math/gentype.inc
> @@ -54,6 +54,8 @@
>
> #ifndef __FLOAT_ONLY
> #ifdef cl_khr_fp64
> +#pragma OPENCL EXTENSION cl_khr_fp64 : enable
> +
> #define __CLC_SCALAR_GENTYPE double
> #define __CLC_FPSIZE 64

I this this pragma being enabled in the header files in other places. I’m somewhat
confused: why is this necessary? Why shouldn’t this only occur in the source files
that implement fp64 functionality?

it needs to be enabled every time double type is used. It is true that
clc.h enables it globally (and then disables at the end of the header).
However, this patch moves math/clc_nextafter.h outside of the main
clc.h header. It thus does not benefit from clc.h global extension
enables.

without this change, clc_nextafter.h would need to add:
#ifdef cl_khr_fp64
#pragma OPENCL EXTENSION cl_khr_fp64 : enable
#endif

I consider moving extension enables to type.inc files preferable so the
'user' files don't have to think about it.

Jan

Drop unused clc/math/clc_nextafter.h header

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>

amdgpu/lib/math/nextafter.cl | 1 +
generic/include/clc/clc.h | 5 -----
generic/include/clc/math/clc_nextafter.h | 11 -----------
generic/include/clc/math/gentype.inc | 2 ++
4 files changed, 3 insertions(+), 16 deletions(-)
delete mode 100644 generic/include/clc/math/clc_nextafter.h

diff --git a/amdgpu/lib/math/nextafter.cl b/amdgpu/lib/math/nextafter.cl
index 6aee0a0…5b4521d 100644
— a/amdgpu/lib/math/nextafter.cl
+++ b/amdgpu/lib/math/nextafter.cl
@@ -1,5 +1,6 @@
#include <clc/clc.h>
#include “…/lib/clcmacro.h”
+#include <math/clc_nextafter.h>

_CLC_DEFINE_BINARY_BUILTIN(float, nextafter, __clc_nextafter, float, float)

diff --git a/generic/include/clc/clc.h b/generic/include/clc/clc.h
index adaab90…3701336 100644
— a/generic/include/clc/clc.h
+++ b/generic/include/clc/clc.h
@@ -264,9 +264,4 @@
#include <clc/image/image_defines.h>
#include <clc/image/image.h>

-/* libclc internal defintions */
-#ifdef __CLC_INTERNAL
-#include <math/clc_nextafter.h>
-#endif

#pragma OPENCL EXTENSION all : disable
diff --git a/generic/include/clc/math/clc_nextafter.h b/generic/include/clc/math/clc_nextafter.h
deleted file mode 100644
index 81c8f36…0000000
— a/generic/include/clc/math/clc_nextafter.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#define __CLC_BODY <clc/math/binary_decl.inc>

-#define __CLC_FUNCTION nextafter
-#include <clc/math/gentype.inc>
-#undef __CLC_FUNCTION

-#define __CLC_FUNCTION __clc_nextafter
-#include <clc/math/gentype.inc>
-#undef __CLC_FUNCTION

-#undef __CLC_BODY
diff --git a/generic/include/clc/math/gentype.inc b/generic/include/clc/math/gentype.inc
index e6ffad1…954cd00 100644
— a/generic/include/clc/math/gentype.inc
+++ b/generic/include/clc/math/gentype.inc
@@ -54,6 +54,8 @@

#ifndef __FLOAT_ONLY
#ifdef cl_khr_fp64
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable
+
#define __CLC_SCALAR_GENTYPE double
#define __CLC_FPSIZE 64

I this this pragma being enabled in the header files in other places. I’m somewhat
confused: why is this necessary? Why shouldn’t this only occur in the source files
that implement fp64 functionality?

it needs to be enabled every time double type is used. It is true that
clc.h enables it globally (and then disables at the end of the header).
However, this patch moves math/clc_nextafter.h outside of the main
clc.h header. It thus does not benefit from clc.h global extension
enables.

without this change, clc_nextafter.h would need to add:
#ifdef cl_khr_fp64
#pragma OPENCL EXTENSION cl_khr_fp64 : enable
#endif

I consider moving extension enables to type.inc files preferable so the
‘user’ files don’t have to think about it.

I would say that from a strict standards point of view, the pragma should
not occur in the headers at all, and only in the implementation. If a user
wants to use doubles, he/she should add the pragma to his/her own code.
However, from a usability point-of-view. I totally understand having the
pragma in the headers.

I’m fine with it either way. I just think it’s good we realise we’re making an
explicit choice here.

Jeroen

>=20
>>>=20
>>> Drop unused clc/math/clc_nextafter.h header
>>>=20
>>> Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
>>> ---
>>> amdgpu/lib/math/nextafter.cl | 1 +
>>> generic/include/clc/clc.h | 5 -----
>>> generic/include/clc/math/clc_nextafter.h | 11 -----------
>>> generic/include/clc/math/gentype.inc | 2 ++
>>> 4 files changed, 3 insertions(+), 16 deletions(-)
>>> delete mode 100644 generic/include/clc/math/clc_nextafter.h
>>>=20
>>> diff --git a/amdgpu/lib/math/nextafter.cl =
b/amdgpu/lib/math/nextafter.cl
>>> index 6aee0a0..5b4521d 100644
>>> --- a/amdgpu/lib/math/nextafter.cl
>>> +++ b/amdgpu/lib/math/nextafter.cl
>>> @@ -1,5 +1,6 @@
>>> #include <clc/clc.h>
>>> #include "../lib/clcmacro.h"
>>> +#include <math/clc_nextafter.h>
>>>=20
>>> _CLC_DEFINE_BINARY_BUILTIN(float, nextafter, __clc_nextafter, float, =
float)
>>>=20
>>> diff --git a/generic/include/clc/clc.h b/generic/include/clc/clc.h
>>> index adaab90..3701336 100644
>>> --- a/generic/include/clc/clc.h
>>> +++ b/generic/include/clc/clc.h
>>> @@ -264,9 +264,4 @@
>>> #include <clc/image/image_defines.h>
>>> #include <clc/image/image.h>
>>>=20
>>> -/* libclc internal defintions */
>>> -#ifdef __CLC_INTERNAL
>>> -#include <math/clc_nextafter.h>
>>> -#endif
>>> -
>>> #pragma OPENCL EXTENSION all : disable
>>> diff --git a/generic/include/clc/math/clc_nextafter.h =
b/generic/include/clc/math/clc_nextafter.h
>>> deleted file mode 100644
>>> index 81c8f36..0000000
>>> --- a/generic/include/clc/math/clc_nextafter.h
>>> +++ /dev/null
>>> @@ -1,11 +0,0 @@
>>> -#define __CLC_BODY <clc/math/binary_decl.inc>
>>> -
>>> -#define __CLC_FUNCTION nextafter
>>> -#include <clc/math/gentype.inc>
>>> -#undef __CLC_FUNCTION
>>> -
>>> -#define __CLC_FUNCTION __clc_nextafter
>>> -#include <clc/math/gentype.inc>
>>> -#undef __CLC_FUNCTION
>>> -
>>> -#undef __CLC_BODY
>>> diff --git a/generic/include/clc/math/gentype.inc =
b/generic/include/clc/math/gentype.inc
>>> index e6ffad1..954cd00 100644
>>> --- a/generic/include/clc/math/gentype.inc
>>> +++ b/generic/include/clc/math/gentype.inc
>>> @@ -54,6 +54,8 @@
>>>=20
>>> #ifndef __FLOAT_ONLY
>>> #ifdef cl_khr_fp64
>>> +#pragma OPENCL EXTENSION cl_khr_fp64 : enable
>>> +
>>> #define __CLC_SCALAR_GENTYPE double
>>> #define __CLC_FPSIZE 64
>>=20
>> I this this pragma being enabled in the header files in other places. =
I=E2=80=99m somewhat
>> confused: why is this necessary? Why shouldn=E2=80=99t this only =
occur in the source files
>> that implement fp64 functionality?
>=20
> it needs to be enabled every time double type is used. It is true that
> clc.h enables it globally (and then disables at the end of the =
header).
> However, this patch moves math/clc_nextafter.h outside of the main
> clc.h header. It thus does not benefit from clc.h global extension
> enables.
>=20
> without this change, clc_nextafter.h would need to add:
> #ifdef cl_khr_fp64
> #pragma OPENCL EXTENSION cl_khr_fp64 : enable
> #endif
>=20
> I consider moving extension enables to type.inc files preferable so =
the
> 'user' files don't have to think about it.

I would say that from a strict standards point of view, the pragma =
should
not occur in the headers at all, and only in the implementation. If a =
user
wants to use doubles, he/she should add the pragma to his/her own code.
However, from a usability point-of-view. I totally understand having the
pragma in the headers.

This does not impact user applications. clc.h includes
#pragma OPENCL EXTENSION all : disable at the end, so any code needs to
reenable cl_khr_fp64 if it wants to use double type, even if it
includes clc.h.

clang does not distinguish between libclc or other code so the same
rules about types and extensions apply to us; all .cl files reenable
cl_khr_fp64 if they implement builtin using double type.

Jan

I=E2=80=99m fine with it either way. I just think it=E2=80=99s good we =
realise we=E2=80=99re making an
explicit choice here.

Jeroen =20

>=20
> Jan
>=20
>>=20
>> Jeroen
>>=20
>>>=20
>>> --=20
>>> 2.13.6
>>>=20
>>> _______________________________________________
>>> Libclc-dev mailing list
>>> Libclc-dev@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/libclc-dev
>>=20
>> _______________________________________________
>> Libclc-dev mailing list
>> Libclc-dev@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/libclc-dev
>=20
> --=20
> Jan Vesely <jan.vesely@rutgers.edu <mailto:jan.vesely@rutgers.edu>>

[SNIP apple mail noise]

=20

=20
Drop unused clc/math/clc_nextafter.h header
=20
Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
---
amdgpu/lib/math/nextafter.cl | 1 +
generic/include/clc/clc.h | 5 -----
generic/include/clc/math/clc_nextafter.h | 11 -----------
generic/include/clc/math/gentype.inc | 2 ++
4 files changed, 3 insertions(+), 16 deletions(-)
delete mode 100644 generic/include/clc/math/clc_nextafter.h
=20
diff --git a/amdgpu/lib/math/nextafter.cl =

b/amdgpu/lib/math/nextafter.cl

index 6aee0a0..5b4521d 100644
--- a/amdgpu/lib/math/nextafter.cl
+++ b/amdgpu/lib/math/nextafter.cl
@@ -1,5 +1,6 @@
#include <clc/clc.h>
#include "../lib/clcmacro.h"
+#include <math/clc_nextafter.h>
=20
_CLC_DEFINE_BINARY_BUILTIN(float, nextafter, __clc_nextafter, float, =

float)

=20
diff --git a/generic/include/clc/clc.h b/generic/include/clc/clc.h
index adaab90..3701336 100644
--- a/generic/include/clc/clc.h
+++ b/generic/include/clc/clc.h
@@ -264,9 +264,4 @@
#include <clc/image/image_defines.h>
#include <clc/image/image.h>
=20
-/* libclc internal defintions */
-#ifdef __CLC_INTERNAL
-#include <math/clc_nextafter.h>
-#endif
-
#pragma OPENCL EXTENSION all : disable
diff --git a/generic/include/clc/math/clc_nextafter.h =

b/generic/include/clc/math/clc_nextafter.h

deleted file mode 100644
index 81c8f36..0000000
--- a/generic/include/clc/math/clc_nextafter.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#define __CLC_BODY <clc/math/binary_decl.inc>
-
-#define __CLC_FUNCTION nextafter
-#include <clc/math/gentype.inc>
-#undef __CLC_FUNCTION
-
-#define __CLC_FUNCTION __clc_nextafter
-#include <clc/math/gentype.inc>
-#undef __CLC_FUNCTION
-
-#undef __CLC_BODY
diff --git a/generic/include/clc/math/gentype.inc =

b/generic/include/clc/math/gentype.inc

index e6ffad1..954cd00 100644
--- a/generic/include/clc/math/gentype.inc
+++ b/generic/include/clc/math/gentype.inc
@@ -54,6 +54,8 @@
=20
#ifndef __FLOAT_ONLY
#ifdef cl_khr_fp64
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable
+
#define __CLC_SCALAR_GENTYPE double
#define __CLC_FPSIZE 64

=20
I this this pragma being enabled in the header files in other places. =

I=E2=80=99m somewhat

confused: why is this necessary? Why shouldn=E2=80=99t this only =

occur in the source files

that implement fp64 functionality?

=20
it needs to be enabled every time double type is used. It is true that
clc.h enables it globally (and then disables at the end of the =

header).

However, this patch moves math/clc_nextafter.h outside of the main
clc.h header. It thus does not benefit from clc.h global extension
enables.
=20
without this change, clc_nextafter.h would need to add:
#ifdef cl_khr_fp64
#pragma OPENCL EXTENSION cl_khr_fp64 : enable
#endif
=20
I consider moving extension enables to type.inc files preferable so =

the

'user' files don't have to think about it.

I would say that from a strict standards point of view, the pragma =
should
not occur in the headers at all, and only in the implementation. If a =
user
wants to use doubles, he/she should add the pragma to his/her own code.
However, from a usability point-of-view. I totally understand having the
pragma in the headers.

This does not impact user applications. clc.h includes
#pragma OPENCL EXTENSION all : disable at the end, so any code needs to
reenable cl_khr_fp64 if it wants to use double type, even if it
includes clc.h.

clang does not distinguish between libclc or other code so the same
rules about types and extensions apply to us; all .cl files reenable
cl_khr_fp64 if they implement builtin using double type.

You’re right. I’m being stupid. Sorry for the noise.

Jeroen

LGTM

-- Jeroen

LGTM

-- Jeroen