[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