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: [gomp4.1] Doacross library implementation


On 10/08/2015 05:48 AM, Torvald Riegel wrote:
On Thu, 2015-09-24 at 20:32 +0200, Jakub Jelinek wrote:
Torvald, can you please have a look at it, if I got all the atomics / memory
models right?

More detailed comments below, but in general, I'd really suggest to add
more code comments for the synchronization parts.  In the end, the level
of detail of documentation of libgomp is your decision, but, for
example, the lack of comments in synchronization code in glibc has made
maintaining this code and fixing issues in it very costly.  It has also
been hard to understand for many.

My suggestion would be both to (1) document the high-level, abstract
synchronization scheme and (2) how that scheme is implemented.  The
first point is important in my experience because typically, the
high-level scheme and the actual thinking behind it (or, IOW, the intent
of the original author) is much harder to reconstruct in case of
concurrent code than it is for sequential code; you can't just simply
follow the program along line by line, but have to consider
interleavings.

I couldn't agree more. After having spent the last month trying to make sense of libgomp/task.c, I can honestly say that we need better internal documentation. I know this isn't Jakub's fault, as Richard started the non-documenting party, but clearly defined descriptions, functions, and implementation go a long way. APIs and abstractions also make things a _lot_ easier to follow.

It could also be that I'm very new to runtime work, specifically parallel runtime work, but it was hard to understand. I think I finally have a firm grasp on it (I hope), but it did take me until early this week. Consequently, I took it upon myself to documenting big pieces of task.c this week. I assume anyone not jakub/rth coming after me will benefit from it. So yeah, my upcoming patch will have some variables renamed, many more functions with better descriptions (or descriptions at all, etc), and a clearly defined API.

Maybe my brain is small; but this stuff is hard.  Every little bit helps :).

p.s. Ironically, it seems that the longer I spend looking at this code, the less I feel I need to comment because things are now "obvious", which perhaps is an indication that either putting newbies on the projects is a good thing, or documenting things early is good practice.

Aldy


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