Hi! On Fri, 08 Apr 2016 11:36:03 +0200, I wrote: > On Thu, 10 Dec 2015 09:08:35 +0100, Jakub Jelinek <ja...@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?), and a omp-offload.c also fell out of that (with more content to be moved into there, I suspect). We could split up omp-offload.c even further, but I don't know if that's really feasible. Currently in there: offload tables stuff, OpenACC loops stuff and pass_oacc_device_lower, pass_omp_target_link; separated by ^L in this one file. > And possibly some kind of omp-simd.c, and omp-checking.c, and so > on, if feasible. (I have not yet looked in detail.) Not yet looked into these. 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 -- is 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. > > > >I'd suggest to do this shortly before GCC 6 is released, [...] Here is a first variant of such a patch. I will continue to maintain this, and intend to send (incremental?) patches on top of that one, but intend to eventually commit all changes as one big commit, to avoid too much "source code churn" (as much as that's possible...). Some more comments, to help review: The attached 0001-Split-up-gcc-omp-low.c.patch.xz is a Git "--color --word-diff --ignore-space-change" patch, purely meant for manual review; I'm intentionally ;-) not attaching a "patch-applyable" patch at this point, to minimize the risk of other people starting to work on this in parallel with my ongoing changes, which no doubt would result in a ton of patch merge conflicts. Yet, I'm of course very open to any kind of suggestions; please submit these as a "verbal patch". I will of course submit a patch in any other format that you'd like for review. This already survived "light" C/C++/Fortran --enable-offload-targets=nvptx-none,x86_64-intelmicemul-linux-gnu,hsa testing (no HSA execution testing, though), and also survived a "big" bootstrap build. 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? All lines with "//OMP" tags in them will eventually be removed; these are just to help review (hence the --word-diff patch), and to solicit comments, in the case of "//OMPTODO". Some of the OMPTODOs are for myself (clean up #include directives), but for the others, I'd like to hear any comments that you have. I guess you can just ignore any "//OMPCUT" tags (and I'll remove them at one point, and clean up the whitespace). (In the new files) these mean that in the file where the surrounding stuff is from, there has been other stuff that either remained in the original file (omp-low.c), or has been moved to other files. In omp-low.c and omp-low.h, a "//OMPLOWH" tag means that this line has been moved to omp-low.h, and likewise: "//OMPEXP" to omp-expand.c, "//OMPEXPH" to omp-expand.h, "//OMPOFF" to omp-offload.c, and "//OMPOFFH" to omp-offload.h. 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
0001-Split-up-gcc-omp-low.c.patch.xz
Description: application/xz
