Splitting up gcc/omp-low.c?

[Martin Jambor]

Hi,

On Wed, May 25, 2016 at 08:03:41AM +0200, Thomas Schwinge wrote:
> Hi!
> 
> Ping.
> 
> Given that we conceptually agreed about this task, but apparently nobody
> is now interested in reviewing my proposed changes (and tells me how
> they'd like me to submit the patch for review), should I maybe just
> execute the steps?

I would suggest that you re-post the patch in a new thread (and in an
orinary non-fancy format), the details are now buried in this big
thred and might easily be forgotten.

I would also strongly suggest that you post the Changelog in the email
body, even if you compress and attach the patch itself.  Frankly, if
the changes are just mechanical movements, only the Changelog is what
I'd like to see and perhaps comment on.

Martin

> 
> On Wed, 18 May 2016 13:42:37 +0200, Thomas Schwinge <thomas@codesourcery.com> wrote:
> > Ping.
> > 
> > On Wed, 11 May 2016 15:44:14 +0200, I wrote:
> > > Ping.
> > > 
> > > On Tue, 03 May 2016 11:34:39 +0200, I wrote:
> > > > On Wed, 13 Apr 2016 18:01:09 +0200, I wrote:
> > > > > On Fri, 08 Apr 2016 11:36:03 +0200, I wrote:
> > > > > > On Thu, 10 Dec 2015 09:08:35 +0100, Jakub Jelinek <jakub@redhat.com> wrote:
> > > > > > > On Wed, Dec 09, 2015 at 06:23:22PM +0100, Bernd Schmidt wrote:
> > > > > > > > On 12/09/2015 05:24 PM, Thomas Schwinge wrote:
> > > > > > > > >how about we split up gcc/omp-low.c into several
> > > > > > > > >files?  Would it make sense (I have not yet looked in detail) to do so
> > > > > > > > >along the borders of the several passes defined therein?
> > > > > 
> > > > > > > > I suspect a split along the ompexp/omplow boundary would be quite easy to
> > > > > > > > achieve.
> > > > > 
> > > > > That was indeed the first one that I tackled, omp-expand.c (spelled out
> > > > > "expand" instead of "exp" to avoid confusion as "exp" might also be short
> > > > > for "expression"; OK?) [...]
> > > > 
> > > > That's the one I'd suggest to pursue next, now that GCC 6.1 has been
> > > > released.  How would you like me to submit the patch for review?  (It's
> > > > huge, obviously.)
> > > > 
> > > > A few high-level comments, and questions that remain to be answered:
> > > > 
> > > > > Stuff that does not relate to OMP lowering, I did not move stuff out of
> > > > > omp-low.c (into a new omp.c, or omp-misc.c, for example) so far, but
> > > > > instead just left all that in omp-low.c.  We'll see how far we get.
> > > > > 
> > > > > One thing I noticed is that there sometimes is more than one suitable
> > > > > place to put stuff: omp-low.c and omp-expand.c categorize by compiler
> > > > > passes, and omp-offload.c -- at least in part -- [would be] about the orthogonal
> > > > > "offloading" category.  For example, see the OMPTODO "struct oacc_loop
> > > > > and enum oacc_loop_flags" in gcc/omp-offload.h.  We'll see how that goes.
> > > > 
> > > > > Some more comments, to help review:
> > > > 
> > > > > As I don't know how this is usually done: is it appropriate to remove
> > > > > "Contributed by Diego Novillo" from omp-low.c (he does get mentioned for
> > > > > his OpenMP work in gcc/doc/contrib.texi; a ton of other people have been
> > > > > contributing a ton of other stuff since omp-low.c has been created), or
> > > > > does this line stay in omp-low.c, or do I even duplicate it into the new
> > > > > files?
> > > > > 
> > > > > I tried not to re-order stuff when moving.  But: we may actually want to
> > > > > reorder stuff, to put it into a more sensible order.  Any suggestions?
> > > > 
> > > > > I had to export a small number of functions (see the prototypes not moved
> > > > > but added to the header files).
> > > > > 
> > > > > Because it's also used in omp-expand.c, I moved the one-line static
> > > > > inline is_reference function from omp-low.c to omp-low.h, and renamed it
> > > > > to omp_is_reference because of the very generic name.  Similar functions
> > > > > stay in omp-low.c however, so they're no longer defined next to each
> > > > > other.  OK, or does this need a different solution?
> 
> 
> Grüße
>  Thomas