LinkGPURuntime.h defines and creates a structure ForceGPURuntimeLinking which creates an artificial dependency to functions defined in GPUJIT.c. The presence of this structure ensures that these functions are a part of the compiled object/library files including it.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Otherwise, this looks good.
include/polly/Support/LinkGPURuntime.h | ||
---|---|---|
1 ↗ | (On Diff #99004) | The FOLDER tag looks surprising. |
I've uploaded only a proof-of-concept version. @grosser @Meinersbur Need your suggestions with code style, location of the file, etc. Questions inlined.
CMakeLists.txt | ||
---|---|---|
209 ↗ | (On Diff #99004) | This line makes available the files under tools/GPURuntime. This is for a case when a program within the LLVM tree (opt, bugpoint) would require ForceGPURuntimeLinking.h. It isn't useful when the file is to be accessed by an external entitiy, e.g. Julia, since the line(s) that'll include <polly_src>/tools will have to be added to the compile commands of the tool. |
include/polly/Support/LinkGPURuntime.h | ||
1 ↗ | (On Diff #99004) | Which folder would you recommend placing this file ? It's currently in include/polly/Support. |
18 ↗ | (On Diff #99004) | 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 ? )
|
34 ↗ | (On Diff #99004) | Should I typecast the nullptr (like in the line below) or leave it as is ? |
include/polly/Support/LinkGPURuntime.h | ||
---|---|---|
34 ↗ | (On Diff #99004) | I've removed all pointer and integer type casts. |
43 ↗ | (On Diff #99134) | I'm now using a static structure variable instead a function call to force-link, like LinkAllIR.h. This change would make it simpler to force-link by only requiring a #include line for LinkGPURuntime.h . |
@grosser Could you please review this patch ? Julia has accepted my PR to introduce the USE_POLLY_ACC option and uses LinkGPURuntime.h to Link GPURuntime into libjulia.so. Please leave your comments to get this patch working ASAP, before someone tries out USE_POLLY_ACC:=1.
Hi Sanjay,
this looks overall good. However, there is one question open:
include/polly/Support/LinkGPURuntime.h | ||
---|---|---|
18 ↗ | (On Diff #99004) | 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. |
include/polly/Support/LinkGPURuntime.h | ||
---|---|---|
18 ↗ | (On Diff #99004) | I did it this way having Julia in mind. If I had to #include just the GPUJIT.h, I'd have to link libGPURuntime.so to libjulia.so, which would require the rpath to libGPURuntime.so's location. But, the reviewers weren't happy with explicitly adding the rpath. So, I included GPUJIT.c and called every function to ensure an equivalent of linking to GPURuntime happend when I included LinkGPURuntime.h. I'm now working to see if I could do away with GPUJIT.c and make libGPURuntime.so available by copying it to locations specified in RPATH of libjulia.so. |
Not pulling in entire GPUJIT.c anymore. Has to be made available by linking to libGPURuntime.