[PATCH] [OPENMP] Codegen for "omp flush" directive.

Alexey Bataev a.bataev at hotmail.com
Wed Nov 19 01:32:12 PST 2014


John, thanks for the review.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:667
@@ +666,3 @@
+  // Build call void __kmpc_flush(ident_t *loc, ...)
+  // List of variables is ignored by libiomp5 runtime, no need to generate it.
+  llvm::Value *Args[] = {EmitOpenMPUpdateLocation(CGF, Loc)};
----------------
rjmccall wrote:
> To what extent does OpenMP have an ABI?  Like, okay, I accept that the current OpenMP runtime does nothing with the list of variables.  Do we not care that a future OpenMP runtime might want to do something?  Are we implicitly hard-coding an assumption that the compiler will always ship with some exact version of the runtime?
> 
> The runtime spec even calls this out as being useless because the varargs aren't terminated.  So why is this even specified as a variadic function?
> 
> Please fix this.  At the very least, either:
> 1.  Accept that this runtime function will never try to do a more restricted flush, and therefore make this a non-varargs function,
> 2.  Specify some way to say how many variables there are.  The best way to do this, for a runtime function where we don't really care about the convenience of the caller, is to pass the count as a non-varargs argument.
> 
> If you're doing #2 — and I think you probably should — and you don't want to do the work of materializing the addresses, then you should also specify that passing no addresses requests a full memory fence.
John, as a temporary solution I can add an int32 0 as a second parameter to this function call and add a comment that this must be fixed once the runtime function is changed and add a note that full mem fence is requested here.

http://reviews.llvm.org/D6292






More information about the cfe-commits mailing list