[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