This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: OpenACC (gomp-4_0-branch) patch review


On 10/16/2015 11:44 AM, Thomas Schwinge wrote:
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".

I'll just comment on this briefly. In general I'll try to review anything I think I can figure out and which doesn't have a super-active maintainer, but some areas have folks who clearly know the code better. The whole gomp area is one of those, and when in doubt, I like to err on the side of allowing (in this case) Jakub a chance to take a look.

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?

It might be good to start with a relatively high-level overview of the current approach, also documenting which parts of OpenACC the changes implement and which ones they don't.

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"?

Depends on how "somewhat non-perfect" is defined - you might want to elaborate. Code should follow the coding standards, add documentation, testcases, etc., those are minimum requirements for everyone. In cases where something is implemented in a way that has a clearly superiour alternative, I think it is reasonable to ask for it to be changed (the builtin folding fell into this category for me). In terms of the current general approach towards implementing OpenACC I don't intend to give you a hard time, especially since past patch review from Jakub pointed in this direction and it would be unreasonable to second-guess this choice now.

On the bright side I think most of omp-low.c could be described as "somewhat non-perfect", so you'd be following existing practice.


Bernd


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]