[PATCH] [OPENMP] Codegen for 'if' clause

hfinkel at anl.gov hfinkel at anl.gov
Wed Oct 8 07:40:13 PDT 2014


LGTM, thanks!

================
Comment at: include/clang/AST/StmtOpenMP.h:132
@@ +131,3 @@
+  /// \brief Gets single clause of the specified kind \a K associated with the
+  /// current directive iff there is only one clause of this kind.
+  const OMPClause *getSingleClause(OpenMPClauseKind K) const;
----------------
Make sure this gets the improved comment text from D5145.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:141
@@ +140,3 @@
+    auto LVal = OMPRegionInfo->getThreadIDVariableLValue(CGF);
+    auto RVal = CGF.EmitLoadOfLValue(LVal, SourceLocation());
+    LVal = CGF.MakeNaturalAlignAddrLValue(RVal.getScalarVal(),
----------------
Why not use Loc as the source location?

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:144
@@ +143,3 @@
+                                          ThreadIDVar->getType());
+    ThreadID = CGF.EmitLoadOfLValue(LVal, SourceLocation()).getScalarVal();
+    // If value loaded in entry block, use it everywhere in function.
----------------
And here too.

I'm under the impression that it is better to have a bunch of code pointing to the same source location than to have code generated with no source locations.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:264
@@ +263,3 @@
+// as "kmp_int32 *gtid");
+// otherwise, if we're not inside parallel region, but in regular serial code
+// region, get thread ID by calling kmp_int32 kmpc_global_thread_num(ident_t
----------------
Capitalize O in otherwise. -- Move to previous line. 

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:268
@@ +267,3 @@
+// temp.
+//
+llvm::Value *CGOpenMPRuntime::EmitThreadIDAddress(CodeGenFunction &CGF,
----------------
Remove empty comment line.

http://reviews.llvm.org/D4716






More information about the cfe-commits mailing list