Re: [PATCH] omp-low.c split

[Thomas Schwinge <thomas@codesourcery.com>]

Hi!

On Fri, 9 Dec 2016 14:08:21 +0100, Martin Jambor <mjambor@suse.cz> wrote:
> this is the promised attempt at splitting omp-low.c [...]

Yay!  \o/

I have not yet had a chance to review/test this patch, but I plan to.

A few initial comments from the "bike shed departement"; I understand in
GCC sources it will not be easy to rename stuff (such as files) later, so
we should get the names agreed upon early:

Generally, I agree with your division of "omp-low.c" parts.

>   - move everything that is part of pass_oacc_device_lower,
>     pass_omp_device_lower and pass_omp_target_link to a new file
>     omp-device.h,

Should we call this file "omp-offload.c", as offloading is what this
deals with, is the term we agreed to generally use (as far as I can
tell)?

>   - move all pre-lowering gridification stuff to a new file
>     omp-grid.c.  [...]

Is that code generic enough to not call this file "omp-hsa.c" or similar?

>   - I moved stuff that was used from all over the place to a new file
>     omp-general.c (unless it would mean exposing omp_region or
>     omp_context types).

I'd have called that simply "omp.c".

> I am opened to suggestions what to do differently, names of the file
> are for example of course subject to discussion, and I absolutely
> welcome any review and checking, for one I am not going to pretend I
> understand the stuff I put into omp-device.c.  If however there is
> consensus that we should do something like this, I would like to ask
> the community to freeze omp-low.c file until this gets committed, I
> hope you understand that I am afraid of any conflicts.

I very much understand...  :-| When I had worked on the very same thing
months ago, and my changes went without review/approval for a long time,
I had spent numerous hours on keeping my patch up to date.  So, I'm happy
to see that this is now near approval!  (Even though I don't understand
what's different now from when I worked on the same thing back then...)

I hope to have time later today to review/test your actual patch.


Grüße
 Thomas