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

Attachment: pgpUcHjcwFdXd.pgp
Description: PGP signature

Reply via email to