[PATCH] D16784: [OpenMP] Reorganize code to allow specialized code generation for different devices.

Alexey Bataev via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 2 23:20:28 PST 2016


ABataev added inline comments.

================
Comment at: lib/CodeGen/CGOpenMPRuntimeCommon.h:1
@@ +1,2 @@
+//===---- CGOpenMPRuntimeCommon.h - Helpers for OpenMP Runtimes Codegen ----==//
+//
----------------
I don't think we need this new file and new namespace. If some (currently) internal classes are needed, they must be exposed via CGOpenMPRuntime class. Derived class will be able to use these classes.
Also, do not expose classes if they are not required right now.

================
Comment at: lib/CodeGen/CGOpenMPRuntimeCommon.h:267-268
@@ +266,4 @@
+
+LValue emitLoadOfPointerLValue(CodeGenFunction &CGF, Address PtrAddr,
+                                      QualType Ty);
+
----------------
I think it is better to make this function a member of CodeGenFunction.

================
Comment at: lib/CodeGen/CGOpenMPRuntimeCommon.h:270-279
@@ +269,12 @@
+
+/// \brief Emits code for OpenMP 'if' clause using specified \a CodeGen
+/// function. Here is the logic:
+/// if (Cond) {
+///   ThenGen();
+/// } else {
+///   ElseGen();
+/// }
+void emitOMPIfClause(CodeGenFunction &CGF, const Expr *Cond,
+                            const RegionCodeGenTy &ThenGen,
+                            const RegionCodeGenTy &ElseGen);
+
----------------
If you need this one, make it a virtual member of CGOpenMPRuntime

================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:24
@@ +23,3 @@
+  explicit CGOpenMPRuntimeNVPTX(CodeGenModule &CGM) : CGOpenMPRuntime(CGM) {
+    if (!CGM.getLangOpts().OpenMPIsDevice)
+      llvm_unreachable("OpenMP NVPTX is only prepared to deal with device code.");
----------------
I think it must be checked during creation of NVPTX specific OpenMPRuntime instance.


http://reviews.llvm.org/D16784





More information about the cfe-commits mailing list