JonChesterfield added inline comments.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.cpp:175
+
+void CGOpenMPRuntimeAMDGCN::emitSPMDKernelWrapper(
+ const OMPExecutableDirective &D, StringRef ParentName,
----------------
The nvptx emitSPMDKernelWrapper does nothing and the amdgcn one appends some
metadata.
How about 'nvptx::generateMetadata(...)' that does nothing and
'amdgcn::generateMetadata(...)` that does this stuff, called from the end of
emitSPMDKernel?
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.cpp:197
+
+/// Emit structure descriptor for a kernel
+void CGOpenMPRuntimeAMDGCN::emitStructureKernelDesc(
----------------
This metadata generation could be split out from the other changes.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.h:49
+
+ /// Allocate global variable for TransferMedium
+ llvm::GlobalVariable *
----------------
I'm not convinced by this abstraction. It looks like amdgcn and nvptx want
almost exactly the same variable in each case. The difference appears to be
that nvptx uses internal linkage and amdgcn uses weak + externally initialized,
in which case we're better off with
`bool nvptx::needsExternalInitialization() {return false;}`
`bool amdgpu::needsExternalInitialization() {return true;}`
Or, if the inline ternary is unappealing, amdgcn::NewGlobalVariable(...) that
passes the arguments to llvm::GlobalVariable while setting the two fields that
differ between the two.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.h:175
+ /// Emit outlined function specialized for the Fork-Join
+ /// programming model for applicable target directives on the NVPTX device.
----------------
Please put this back to the previous location so we can see whether it changed
in the diff
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86097/new/
https://reviews.llvm.org/D86097
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
