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 | ||
---|---|---|
3790 | Revert back | |
3796–3808 | What is the purpose of these massive changes? | |
3918–3919 | Restore it back, please. | |
lib/CodeGen/CGOpenMPRuntime.h | ||
50–54 | No way!!! Revert it back, please. No need to expose all these stuff in header file | |
383 | Please, gather all 'protected' members in a single 'protected' section | |
lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp | ||
30 | If you expect 'llvm::Function*' here, why you can't change it to 'llvm::Function*'? | |
47 | Remove, not required | |
133 | Comment for 'true' value | |
242 | What about exceptions? Do you plan to support them? If yes, add tests for classes with constructors/destructors and exceptions | |
lib/CodeGen/CGOpenMPRuntimeNVPTX.h | ||
30 | Please, gather all private members in single 'private' section, protected to single 'protected' and public to single 'public' | |
36–41 | No way!!! This must go to .cpp file! | |
68–102 | Move all implementations to .cpp file | |
117–126 | Implementation must be in .cpp | |
129–131 | Again, move it to .cpp | |
138–172 | You can leave just declaration here, as this class is used by reference always. Definition must be moved to .cpp |
lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp | ||
---|---|---|
27 | 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; | |
27 |
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 | ||
---|---|---|
67 | Missed 'virtual' | |
lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp | ||
26 | Use 'llvm::None' instead of '{}' | |
35 | Use 'llvm::None' instead of '{}' | |
44 | Use 'llvm::None' instead of '{}' | |
52 | Remove '{}', not required | |
89 | Just 'namespace', remove 'anonymous' | |
103 | Use '.arrangeNullaryFunction()' instead | |
234 | 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 | ||
---|---|---|
67 | 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 | ||
---|---|---|
67 | I we don't need it in 'CGOpenMPRuntime', then why it is here? |
lib/CodeGen/CGOpenMPRuntime.h | ||
---|---|---|
67 | 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