[PATCH] D33198: Header file to help forcibly link GPURuntime
Tobias Grosser via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon May 22 20:28:32 PDT 2017
grosser added a comment.
Hi Sanjay,
this looks overall good. However, there is one question open:
================
Comment at: include/polly/Support/LinkGPURuntime.h:18
+
+extern "C"
+{
----------------
singam-sanjay wrote:
> Although the Julia (the external tool) compiled without this construct, would it be better to enclose the #include line inside this extern "C" since it specifies that the code within should be treated as C code ?
>
> e.g. Lines like the following, in GPUJIT.c, which are allowed in the default C dialect used by GCC, would fail when compiled as C++ ( although it didn't this time. Why was that the case ? )
>
>
> - PollyGPUFunction *Function = malloc(sizeof(PollyGPUFunction)); ([[ https://github.com/llvm-mirror/polly/blob/99a2e0c6bb2713e700c33cec48a9e8dd0461f9f6/tools/GPURuntime/GPUJIT.c#L467 | 1 ]] , [[ https://github.com/llvm-mirror/polly/blob/99a2e0c6bb2713e700c33cec48a9e8dd0461f9f6/tools/GPURuntime/GPUJIT.c#L1192 | 2 ]])
> - [[ https://github.com/llvm-mirror/polly/blob/99a2e0c6bb2713e700c33cec48a9e8dd0461f9f6/tools/GPURuntime/GPUJIT.c#L606 | PollyGPUDevicePtr *DevData = malloc(sizeof(PollyGPUDevicePtr)); ]]
>
>
Why are you including the full .c file? You should probably introduce a GPUJIT.h header file and only include this one. Like this there is also no need to make CUDA or OpenCL includes available when linking.
Repository:
rL LLVM
https://reviews.llvm.org/D33198
More information about the llvm-commits
mailing list