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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
