One conceptual objection. Code-wise, seems fine.
================
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)};
----------------
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.
http://reviews.llvm.org/D6292
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
