sfantao added a comment.
Hi Alexey,
Thanks again for the review!
================
Comment at: lib/CodeGen/CGOpenMPRuntimeCommon.h:1
@@ +1,2 @@
+//===---- CGOpenMPRuntimeCommon.h - Helpers for OpenMP Runtimes Codegen
----==//
+//
----------------
ABataev wrote:
> 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.
Alright. I will add what is required as protected members of CGOpenMPRuntime.
For now I didn't do that for any of the classes given that we are not doing any
actual codegen.
================
Comment at: lib/CodeGen/CGOpenMPRuntimeCommon.h:267-268
@@ +266,4 @@
+
+LValue emitLoadOfPointerLValue(CodeGenFunction &CGF, Address PtrAddr,
+ QualType Ty);
+
----------------
ABataev wrote:
> I think it is better to make this function a member of CodeGenFunction.
Ok, once we start moving things to `CGOpenMPRntime` runtime, we add this to
CodeGenFunction accordingly.
================
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);
+
----------------
ABataev wrote:
> If you need this one, make it a virtual member of CGOpenMPRuntime
Ok. I'll do that once we post the codegen patches.
================
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.");
----------------
ABataev wrote:
> I think it must be checked during creation of NVPTX specific OpenMPRuntime
> instance.
Ok, I did that. Also I added a new diagnostic in the compiler invocation so
that the user gets a message instead of a stack dump. Let me know if you agree.
http://reviews.llvm.org/D16784
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
