Hi! Just some suggestion related to terminology.
On Tue, 19 Nov 2013 13:58:29 +0400, Ilya Tocar <tocarip.in...@gmail.com> wrote: > On 14 Nov 11:27, Richard Biener wrote: > > > + /* Set when symbol needs to be dumped for lto/offloading. */ > > > + unsigned need_dump : 1; > > > + > > > > That's very non-descriptive. What's "offloading"? But yes, something > > like this is what I was asking for. > > I've changed it into: > Set when symbol needs to be dumped into LTO bytecode for LTO, > or in pragma omp target case, for separate compilation targeting > a different architecture. Can we in fact agree to use the term "offload" to mean exactly that? We'll need this in other contexts, too, such as for configuring the "secondary" lto1 (which is, in fact, the one that will be processing the main GCC's "offloaded" code)? I'm happy to go looking for a proper section in GCC's (internal?) manual to document what "offloading" means in GCC's context, and I'm likewise happy to hear if there's any better term existing for describing basically the process of »separate compilation targeting a different architecture«? (I'm not a native speaker.) By the way, in this context, I like saing "offloading" better than "acceleration", because while we strive for acceleration, offloading is what we technically do. > diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c > index c3a8967..53cd250 100644 > --- a/gcc/cgraphunit.c > +++ b/gcc/cgraphunit.c > @@ -2019,7 +2019,18 @@ ipa_passes (void) > passes->all_lto_gen_passes); > > if (!in_lto_p) > - ipa_write_summaries (); > + { > + if (flag_openmp) The following comment applies to several more instances in this patch: after the front end's parsing stage, we should now basically everywhere treat flag_openacc and flag_openmp the same. The idea is that all the existing OpenMP omp_* infrastructure in the middle end and following is now applicable to not only OpenMP but also OpenACC and any other "acceleration" mechanisms. Again, is there a better term to use instead of "acceleration" for describing the union of Cilk+, OpenACC, OpenMP, and similar techniques? Also, would it then make sense to define a flag à la: #define flag_acceleration (flag_openacc | flag_openmp) ..., and begin using that everywhere after the front ends where flag_openmp is currently used? > +/* Select what needs to be dumped. In lto case dump everything. > + In omp target case only dump stuff makrked with attribute. */ > +void > +select_what_to_dump (bool is_omp) Likewise, while this obviously (and unsurprisingly) continues with the existing convention, I'd suggest that in the future we name such things more generically: is_offload or is_acceleration (or, again, any better term that is generically applicable). Or, going by what I've been told before by Jakub and Nathan in <http://news.gmane.org/find-root.php?message_id=%3C523AC6FF.6030007%40acm.org%3E>: »Names are sticky.«, should we instead continue to name all these new things omp_*, too, and declare that the "omp" tag is an artifact of history? (But it certainly is an obstacle to anyone new to the code. I realize that people are a bit tired of refactoring, which recently has been applied in other contexts, and has caused some "churn". But still, I'd personally go ahread and rename the current omp_* middle end bits to any new tag that we agree on, just to make things more clear to everyone new to the code. But, I don't insist on that (by now I manage to internally map omp_* to acceleration_* or similar), so you long-time contributors of course get to have the finaly say about this. I'm only commenting from still a new contributor's point of view.) > diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h > index 797e92e..f4c46db 100644 > --- a/gcc/lto-streamer.h > +++ b/gcc/lto-streamer.h > @@ -139,6 +139,11 @@ along with GCC; see the file COPYING3. If not see > name for the functions and static_initializers. For other types of > sections a '.' and the section type are appended. */ > #define LTO_SECTION_NAME_PREFIX ".gnu.lto_" > +#define OMP_SECTION_NAME_PREFIX ".gnu.target_lto_" Also here, the "target" tag is confusing in that "target" typically has a different meaning in the compiler context -- while this one here is used for implementing OpenMP's target construct, what it technically does should be described by .gnu.offload_gimple_ or somthing similar. (Note that also the LTO term is no longer really applicable, as we're not neccessarily doing link-time optimization here, but instead rather just use the GIMPLE dumping ("offloading") infrastructure that happens to already be implemented in GCC's LTO support. Again, all this doesn't matter to anyone who's already versed with the code, but it does matter to people who are new, and still have to learn all these fine details. (Of which there are many, many more. Yes, I know, life's tough, and I'm used to that, but still.) ;-) And I'm certainly not asking anyone to spend their own time with this refactoring work, but I'd just like to inquire the general permission, the option of allowing me (or anyone else, of course) to do this later on. Also, if you again say: »Names are sticky.«, then that's fine with me, too. Grüße, Thomas
pgpUcHjcwFdXd.pgp
Description: PGP signature