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 | ||
|---|---|---|
| 4163–4164 | Restore it back, please. | |
| 4177 | Revert back | |
| 4183–4195 | 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 | ||
| 346 | If you expect 'llvm::Function*' here, why you can't change it to 'llvm::Function*'? | |
| 363 | Remove, not required | |
| 449 | Comment for 'true' value | |
| 558 | 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 | ||
|---|---|---|
| 343 | 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; | |
| 343 |
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–133 | 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