[PATCH] [OPENMP] Initial codegen for '#pragma omp parallel'
Alexey Bataev
a.bataev at hotmail.com
Fri Mar 7 03:56:04 PST 2014
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:45
@@ +44,3 @@
+public:
+ enum OpenMPLocationFlags {
+ /// \brief Use tramponline for internal microtask.
----------------
hfinkel at anl.gov wrote:
> This should have some overall comment explaining what this is the location of.
Ok, I'll add comment
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:48
@@ +47,3 @@
+ OMP_IDENT_IMD = 0x01,
+ /// \brief Use c-stile ident structure.
+ OMP_IDENT_KMPC = 0x02,
----------------
hfinkel at anl.gov wrote:
> c-style?
Yes, all these comments are taken from http://llvm.org/svn/llvm-project/openmp/trunk/runtime/src/kmp.h
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:72
@@ +71,3 @@
+private:
+ enum Ident_TFields {
+ Ident_T_Reserved_1,
----------------
hfinkel at anl.gov wrote:
> This needs to be documented (including the fields too).
Ok, will be commented.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:80
@@ +79,3 @@
+ CodeGenModule &CGM;
+ llvm::Constant *DefaultOpenMPPSource;
+ llvm::StructType *Ident_TTy;
----------------
hfinkel at anl.gov wrote:
> What's this?
Default position used for initialization of all ident_t objects. I'll described it.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:28
@@ +27,3 @@
+CGOpenMPRuntime::CGOpenMPRuntime(CodeGenModule &CGM) : CGM(CGM) {
+ DefaultOpenMPPSource = CGM.GetAddrOfConstantCString(";unknown;unknown;0;0;;");
+ DefaultOpenMPPSource =
----------------
hfinkel at anl.gov wrote:
> Please document the meaning of this string.
Ok.
================
Comment at: lib/Sema/SemaOpenMP.cpp:675
@@ +674,3 @@
+ { std::make_pair("global_tid", KmpInt32PtrTy),
+ std::make_pair("bound_tid", KmpInt32PtrTy),
+ std::make_pair(StringRef(), QualType()) // __context with shared vars
----------------
hfinkel at anl.gov wrote:
> Could these names conflict with user-provided names? (Should they be in the implementation namespace?)
Yes, that's possible. I'll rename them.
http://llvm-reviews.chandlerc.com/D2883
More information about the cfe-commits
mailing list