[+Jim] ----- Original Message ----- > From: "John McCall" <[email protected]> > To: "a bataev" <[email protected]>, [email protected], > [email protected], [email protected] > Cc: [email protected] > 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
-- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
