OpenACC (gomp-4_0-branch) patch review (was: Merge from gomp-4_1-branch to trunk)

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Tue, 13 Oct 2015 21:12:14 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
> I've bootstrapped/regtested on x86_64-linux and i686-linux following
> merge from gomp-4_1-branch to trunk, which brings in most of the OpenMP 4.5
> support for C and C++

With nvptx offloading, I'm seeing the following regressions (on trunk):

    PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/asyncwait-1.c -DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/asyncwait-1.c -DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 execution test

    PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/data-2.c -DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/data-2.c -DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 execution test

    PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/data-3.c -DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/data-3.c -DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 execution test

>From a quick look, it might have something to do with changes that affect
handling of the OpenACC async clause.  I guess you're not set up for
nvptx offloading testing; I'll try to figure it out.


More generally, "as you've committed first", the burden of merging your
gomp-4_1-branch merge (trunk r228777) with our gomp-4_0-branch changes
will now be upon us.  From a quick assessment, this certainly doesn't
look trivial.  But well, that's how it is; one of us has to swallow the
bitter pill...

But: working on getting our changes into trunk, for example, when we make
an effort to extract from gomp-4_0-branch self-contained, individual
patches, but it then takes weeks to get commit approval or review
comments, I don't see how that's going to work for the thousands of lines
patches that we're still to submit?  I mean, GCC development stage 1 is
going to end in just a few weeks, I suppose?

At the GNU Tools Cauldron 2013 we've repeatedly been asked (by you, by
Jeff Law, and others) a) to tightly integrate our OpenACC/nvptx
offloading changes with the existing OMP code, and b) to do our
development in public so that you have a chance to review our changes
early, so they can then be merged without much effort -- we've tried our
best for a) given our understanding of the existing OMP code, and our
changes have incrementally been committed to gomp-4_0-branch over the
course of a lot of months now, but I'm not convinced that much of that
has actually been reviewed so far?  How are we going to tackle this?

I understand that you're a very busy person, with lots of
responsibilities, and I also understand that often you have a rather
"rigid" idea of how changes should/shouldn't be done (which generally is
a good thing, of course!), but if we're apparently not making much
progress with our merge into trunk -- and, as you know yourself, juggling
with (that is, rebasing on top of trunk changes, and so on) thousands of
lines patches is no fun! -- would it perhaps make sense to officially
appoint somebody else to review OMP changes in addition to you?  And,
also, allow for "somewhat non-perfect" changes to be committed, and later
address the "warts"?  (Allowing for incremental progress, while keeping
GCC test results clean, and all that, of course!)  Lately, Bernd has
stepped up a few times to review OMP patches (many thanks, Bernd!), but
also he sometimes stated that even though a patch appears fine to him,
he'd like "Jakub to have a look".

Please note that my concern here is not to accuse people, but it's to
find a way to improve the review situation/process.

There's still a bit of clean-up and development going on on
gomp-4_0-branch, but what should be the strategy to get it merged into
trunk?  Instead of us extracting and submitting individual changes (which
we certainly can do, but which is a huge time-sink if they're then not
handled quickly), would you maybe prefer to do a "whole-branch" review?


GrÃÃe
 Thomas

Attachment: signature.asc
Description: PGP signature