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

[Cownie, James H]

>From a runtime point of view we need to preserve backwards binary compatibility, so we can't change the interface to the current interface function to introduce a count (because that old code won't set it).

I would therefore prefer to do this

1) Change the prototype for the existing function so that it becomes a function with no arguments. (Effectively your #1). Since no existing code passes arguments, this is fine.

2) If/when we decide that it is useful to pass extra information, we design that as a new function. I can't see a need for that in a cache-coherent system, and in a non-coherent system the interface needs to be more complicated than just a set of pointers (because you also need to know how big the object at the target of the pointer is). So this gets complicated, but, since we're not going to implement it until we need it and understand the problem better, that’s fine. 

I'm avoiding your #2 because I don't believe it is sufficient.

-- Jim

James Cownie <james.h.cownie at intel.com>
SSG/DPD/TCAR (Technical Computing, Analyzers and Runtimes)
Tel: +44 117 9071438

-----Original Message-----
From: Hal Finkel [mailto:hfinkel at anl.gov] 
Sent: Tuesday, November 18, 2014 10:29 PM
To: reviews+D6292+public+0f122151bdc64d55 at reviews.llvm.org
Cc: cfe-commits at cs.uiuc.edu; a bataev; fraggamuffin at gmail.com; ejstotzer at gmail.com; rjmccall at gmail.com; Cownie, James H
Subject: Re: [PATCH] [OPENMP] Codegen for "omp flush" directive.

[+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
---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.