[Libclc-dev] [PATCH 3/4] Do not include clc_nextafter header globally

[Jeroen Ketema via Libclc-dev]

> On 8 Oct 2017, at 20:11, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> 
> On Sun, 2017-10-08 at 12:22 +0200, Jeroen Ketema via Libclc-dev wrote:
>>> On 8 Oct 2017, at 10:55, Jan Vesely via Libclc-dev <libclc-dev at lists.llvm.org> wrote:
>>> 
>>> Drop unused clc/math/clc_nextafter.h header
>>> 
>>> Signed-off-by: Jan Vesely <jan.vesely at 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  

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libclc-dev/attachments/20171008/52ee21df/attachment-0001.html>