[PATCH 1/2] Implement fmin using __builtin_fmin

This ensures correct handling of NaN / INF.

This has been tested with piglit, OpenCV, and the ocl conformance tests.

This ensures correct handling of NaN / INF.

This has been tested with piglit, OpenCV, and the ocl conformance tests.

This ensures correct handling of NaN / INF.

This has been tested with piglit, OpenCV, and the ocl conformance tests.
---
generic/include/clc/math/fmin.h | 5 +----
generic/lib/math/fmin.cl | 13 +++++++++----
generic/lib/math/fmin.inc | 18 ++++++++++++++++++
3 files changed, 28 insertions(+), 8 deletions(-)
create mode 100644 generic/lib/math/fmin.inc

diff --git a/generic/include/clc/math/fmin.h b/generic/include/clc/math/fmin.h
index 5588ba9..d45f572 100644
--- a/generic/include/clc/math/fmin.h
+++ b/generic/include/clc/math/fmin.h
@@ -1,8 +1,5 @@
-#undef fmin
-#define fmin __clc_fmin
-
#define __CLC_BODY <clc/math/binary_decl.inc>
-#define __CLC_FUNCTION __clc_fmin
+#define __CLC_FUNCTION fmin

Wasn’t this done to avoid name collisions with the standard math library?

Otherwise: LGTM.

Jeroen

This ensures correct handling of NaN / INF.

This has been tested with piglit, OpenCV, and the ocl conformance tests.
---
generic/include/clc/math/fmax.h | 5 +----
generic/lib/math/fmax.cl | 13 +++++++++----
generic/lib/math/fmax.inc | 18 ++++++++++++++++++
3 files changed, 28 insertions(+), 8 deletions(-)
create mode 100644 generic/lib/math/fmax.inc

diff --git a/generic/include/clc/math/fmax.h b/generic/include/clc/math/fmax.h
index d6956af..71ee859 100644
--- a/generic/include/clc/math/fmax.h
+++ b/generic/include/clc/math/fmax.h
@@ -1,8 +1,5 @@
-#undef fmax
-#define fmax __clc_fmax
-
#define __CLC_BODY <clc/math/binary_decl.inc>
-#define __CLC_FUNCTION __clc_fmax
+#define __CLC_FUNCTION fmax

Same comment as the fmin patch.

Otherwise: LGTM.

Jeroen

>
> This ensures correct handling of NaN / INF.
>
> This has been tested with piglit, OpenCV, and the ocl conformance tests.
> ---
> generic/include/clc/math/fmin.h | 5 +----
> generic/lib/math/fmin.cl | 13 +++++++++----
> generic/lib/math/fmin.inc | 18 ++++++++++++++++++
> 3 files changed, 28 insertions(+), 8 deletions(-)
> create mode 100644 generic/lib/math/fmin.inc
>
> diff --git a/generic/include/clc/math/fmin.h b/generic/include/clc/math/fmin.h
> index 5588ba9..d45f572 100644
> --- a/generic/include/clc/math/fmin.h
> +++ b/generic/include/clc/math/fmin.h
> @@ -1,8 +1,5 @@
> -#undef fmin
> -#define fmin __clc_fmin
> -
> #define __CLC_BODY <clc/math/binary_decl.inc>
> -#define __CLC_FUNCTION __clc_fmin
> +#define __CLC_FUNCTION fmin

Wasn’t this done to avoid name collisions with the standard math library?

We are passing -fno-builtin to clang, which as far as I understand
prevents this problem.

-Tom

This ensures correct handling of NaN / INF.

This has been tested with piglit, OpenCV, and the ocl conformance tests.
---
generic/include/clc/math/fmin.h | 5 +----
generic/lib/math/fmin.cl | 13 +++++++++----
generic/lib/math/fmin.inc | 18 ++++++++++++++++++
3 files changed, 28 insertions(+), 8 deletions(-)
create mode 100644 generic/lib/math/fmin.inc

diff --git a/generic/include/clc/math/fmin.h b/generic/include/clc/math/fmin.h
index 5588ba9..d45f572 100644
--- a/generic/include/clc/math/fmin.h
+++ b/generic/include/clc/math/fmin.h
@@ -1,8 +1,5 @@
-#undef fmin
-#define fmin __clc_fmin
-
#define __CLC_BODY <clc/math/binary_decl.inc>
-#define __CLC_FUNCTION __clc_fmin
+#define __CLC_FUNCTION fmin

Wasn’t this done to avoid name collisions with the standard math library?

We are passing -fno-builtin to clang, which as far as I understand
prevents this problem.

I had to read up on no-buitin; this seems to correspond with my understanding of this flag. So, LGTM.

Jeroen