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