This patch adds base support for codegen of the target directive on the NVPTX device.
Details
- Reviewers
ABataev hfinkel - Commits
- rG5c309e475dcf: [OpenMP] Base support for target directive codegen on NVPTX device.
rGac563708abd0: [OpenMP] Base support for target directive codegen on NVPTX device.
rG5e1493b560da: [OpenMP] Base support for target directive codegen on NVPTX device.
rGc61744c26b81: [OpenMP] Base support for target directive codegen on NVPTX device.
rC264018: [OpenMP] Base support for target directive codegen on NVPTX device.
rC263783: [OpenMP] Base support for target directive codegen on NVPTX device.
rC263587: [OpenMP] Base support for target directive codegen on NVPTX device.
rC263552: [OpenMP] Base support for target directive codegen on NVPTX device.
rL263783: [OpenMP] Base support for target directive codegen on NVPTX device.
rL263587: [OpenMP] Base support for target directive codegen on NVPTX device.
rL263552: [OpenMP] Base support for target directive codegen on NVPTX device.
Diff Detail
Event Timeline
lib/CodeGen/CGOpenMPRuntime.cpp | ||
---|---|---|
4111–4112 | Restore it back, please. | |
4125 | Revert back | |
4131–4143 | What is the purpose of these massive changes? | |
lib/CodeGen/CGOpenMPRuntime.h | ||
52–56 | No way!!! Revert it back, please. No need to expose all these stuff in header file | |
294 | Please, gather all 'protected' members in a single 'protected' section | |
lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp | ||
351 | If you expect 'llvm::Function*' here, why you can't change it to 'llvm::Function*'? | |
368 | Remove, not required | |
454 | Comment for 'true' value | |
563 | What about exceptions? Do you plan to support them? If yes, add tests for classes with constructors/destructors and exceptions | |
lib/CodeGen/CGOpenMPRuntimeNVPTX.h | ||
137 | Please, gather all private members in single 'private' section, protected to single 'protected' and public to single 'public' | |
143–148 | No way!!! This must go to .cpp file! | |
175–209 | Move all implementations to .cpp file | |
224–233 | Implementation must be in .cpp | |
236–238 | Again, move it to .cpp | |
245–279 | You can leave just declaration here, as this class is used by reference always. Definition must be moved to .cpp |
lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp | ||
---|---|---|
348 | This function will also handle global variables from the 'declare target' construct in the future. void CGOpenMPRuntimeNVPTX::createOffloadEntry(llvm::Constant *ID, llvm::Constant *Addr, uint64_t Size) { auto *F = dyn_cast<llvm::Function>(Addr); // TODO: Add support for global variables on the device after declare target // support. if (!F) return; | |
348 |
This backend does not support exceptions. We plan to send a separate patch that will selectively deactivate exceptions in target regions generated for the GPU. |
lib/CodeGen/CGOpenMPRuntime.h | ||
---|---|---|
69 | Missed 'virtual' | |
lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp | ||
27 | Use 'llvm::None' instead of '{}' | |
36 | Use 'llvm::None' instead of '{}' | |
45 | Use 'llvm::None' instead of '{}' | |
53 | Remove '{}', not required | |
90 | Just 'namespace', remove 'anonymous' | |
104 | Use '.arrangeNullaryFunction()' instead | |
235 | Use 'llvm::None' as the second arg | |
lib/CodeGen/CGOpenMPRuntimeNVPTX.h | ||
26 | All functions must start with a lower case letter |
lib/CodeGen/CGOpenMPRuntime.h | ||
---|---|---|
69 | Do we need 'virtual'? This function needs to be called by the NVPTX implementation (in emitTargetOutlinedFunction) but doesn't need to be overridden. |
lib/CodeGen/CGOpenMPRuntime.h | ||
---|---|---|
69 | I we don't need it in 'CGOpenMPRuntime', then why it is here? |
lib/CodeGen/CGOpenMPRuntime.h | ||
---|---|---|
69 | Alexey, both CGOpenMPRuntime and CGOpenMPRuntimeNVPTX call emitTargetOutlinedFunctionHelper(). CGOpenMPRuntime::emitTargetOutlinedFunction() sets up the 'codegen' lambda and then calls CGOpenMPRuntime::emitTargetOutlinedFunctionHelper() to do the work. In the same way, CGOpenMPRuntimeNVPTX::emitTargetOutlinedFunction() sets up the 'codegen' lambda for the GPU and then calls CGOpenMPRuntime::emitTargetOutlinedFunctionHelper(). Does that make sense? |
No way!!! Revert it back, please. No need to expose all these stuff in header file