Re: [PATCH, og9] Port OpenACC profiling interface to OG9

[Thomas Schwinge <thomas@codesourcery.com>]

Hi Kwok!

On Mon, 17 Jun 2019 14:27:46 +0100, Kwok Cheung Yeung <kcy@codesourcery.com> wrote:
> This is a straightforward port of the OpenACC profiling interface from 
> OG8 to OG9, with a few tweaks

Thanks for looking into this.

> to compensate for patches that have not 
> been carried over to OG9.

Conceptually ACK.  (I have not reviewed what's missing/got dropped.)

> I have also bundled in the profiling hooks and 
> documentation updates that have been added since the original patch in OG8.

ACK.

> Okay to push to openacc-gcc-9-branch?

What you probably didn't know (sorry!) is that trunk r271346,
<http://mid.mail-archive.com/yxfplfz4anys.fsf@hertz.schwinge.homeip.net>,
contains some changes that are not in the og8 code: code and
documentation improvements, TODOs resolved, etc.  It also doesn't contain
the actual 'acc_register_library' implementation, because that one needs
further work, as discussed before.

I think what would be best, is the following approach:

  - First, backport trunk r271346 to og9.

    That might not be completely trivial, because as you know, og9
    contains a number of "feature" commits that need to include changes
    related to the profiling code.  It's the question in which order to
    sequence patches -- whether these changes related to the profiling
    code are part of the individual "feature" commits, or they're part of
    a later "profiling" commit.  As og9 has been published with "feature"
    commits not containing any profiling bits, all these things will have
    to be in a later "profiling" commit.

    However, for upstreaming this into trunk later on, it will be
    beneficial to have the backported "profiling" commit as similar as
    possible to trunk r271346, so we shall live with some short-lived
    inconsistencies, maybe even a few testuite regressions, which then
    get resolved when you...

  - Second, on top of that, add the pieces of functionality (from the
    og8) version that are missing from the trunk r271346 backport.  Of
    course, remove all changes that would worsen the state, compared to
    what trunk r271346 already contains.

So, conceptually: a first commit to backport trunk r271346, then a second
commit containing a merger of a (temporary) revert of the first commit
followed by applying the patch you just posted.  Then, remove from the
second commit all changes that worsen the state, compared to what trunk
r271346 already contains.

Or, in other words: split the patch you just posted into two, where the
first one is as close as possible to a backport of trunk r271346, then
all other changes in a second commit.

Will that work?

I'll be happy to help review these changes, especially whether something
should be part of the second commit, or get dropped.


Grüße
 Thomas