I'd appreciate it if you described the rationale for changes like this
somewhere in the code. I'm happy to take your word for it that this is
correct, but it will help later people looking at this code if they can see why
you're using one function instead of the other.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:640
@@ -642,3 +639,3 @@
OpenMPLocationFlags Flags) {
- // Build call __kmpc_barrier(loc, thread_id)
+ // Build call __kmpc_cancel_barrier(loc, thread_id);
llvm::Value *Args[] = {EmitOpenMPUpdateLocation(CGF, Loc, Flags),
----------------
Does this runtime function always exist, or is it contingent on having a
sufficiently new OpenMP runtime? If the latter, you should introduce the
concept of a target OpenMP runtime version, patterned on the target Objective-C
runtime version.
Also, please describe in a comment why we're using this function rather than
the other, since apparently both "work" in some sense.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:96
@@ +95,3 @@
+ /// \brief Implicit barrier in 'for' directive.
+ OMP_IDENT_BARRIER_IMPL_FOR = 0x40,
+ /// \brief Implicit barrier in 'sections' directive.
----------------
Is this supposed to be the same value as OMP_IDENT_BARRIER_IMPL? If so, is
there a better way to define these constants that makes their structure more
obvious?
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:238
@@ +237,3 @@
+ virtual void EmitOMPBarrierCall(CodeGenFunction &CGF, SourceLocation Loc,
+ OpenMPLocationFlags Flags);
+
----------------
Don't make two functions with the same name that differ only in an enum vs.
bool argument.
http://reviews.llvm.org/D6447
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
