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

[Hal Finkel]

[+Jim]

----- Original Message -----
> From: "John McCall" <rjmccall at gmail.com>
> To: "a bataev" <a.bataev at hotmail.com>, fraggamuffin at gmail.com, ejstotzer at gmail.com, rjmccall at gmail.com
> Cc: cfe-commits at cs.uiuc.edu
> Sent: Tuesday, November 18, 2014 4:24:10 PM
> Subject: Re: [PATCH] [OPENMP] Codegen for "omp flush" directive.
> 
> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory