[Openmp-dev] [PATCH] [Revisedx2] Initial cmake support

Hal Finkel hfinkel at anl.gov
Mon Jun 2 10:17:41 PDT 2014


----- Original Message -----
> From: "Andrey Bokhanko" <andreybokhanko at gmail.com>
> To: "Jack Howarth" <howarth.mailing.lists at gmail.com>
> Cc: openmp-dev at dcs-maillist2.engr.illinois.edu, "David Chisnall" <dc552 at cam.ac.uk>
> Sent: Monday, June 2, 2014 11:59:57 AM
> Subject: Re: [Openmp-dev] [PATCH] [Revisedx2] Initial cmake support
> 
> 
> 
> 
> Jack,
> 
> 
> The approach we (Intel) use is standard for clang community. No
> changes in how we prepare / submit patches will speed-up code review
> process. Limiting factor here is small number of clang code owners /
> their limited throughput (which is understandable given how many
> things they carry on their shoulders).
> 
> 
> We tried to extend number of code reviewers for our patches, but as
> the mail thread you linked tells, not many people has enough
> authority to approve patches for clang. At least yet -- hopefully,
> this will change in the future.

Just to be clear (because I feel this is important), it is not a matter of authority, it is a matter of expertise. There are not many regular contributors with the expertise and background that would allow them to feel comfortable approving the patches. And, yes, this will improve as time goes on (and, if nothing else, based on current trends, the list of relevant people will increasingly come to include Alexey, etc.).

 -Hal

> 
> 
> There is hope, though -- Richard Smith, who is one of clang code
> owners, approved us to commit Sema (parsing + semantic analysis)
> parts of our patches in "review after commit" fashion. Given that
> these pathces comprise 50% of all code, this should speed up
> upstreaming a lot.
> 
> 
> Yours,
> Andrey
> 
> 
> 
> On Mon, Jun 2, 2014 at 8:14 PM, Jack Howarth <
> howarth.mailing.lists at gmail.com > wrote:
> 
> 
> 
> Andrey,
> Also, note that when I say a single set of patches, I don't mean a
> single patch but number individual patches submitted as a complete
> patch set. After many years of carefully monitoring merges in FSF
> gcc (and helping mitigate the breakage from them on the darwin
> targets since Apple abandoned gcc), it has become clear that there
> are certain social pressures in the review process that a unified
> patch set creates. When a complete set of patches are submitted and
> say 90% of them are quickly reviewed, approved and committed, this
> results in a social pressure for the remaining reviewers to
> accelerate their work so as to not be seen as retarding the merge.
> Jack
> 
> 
> 
> 
> 
> 
> 
> On Mon, Jun 2, 2014 at 11:48 AM, Jack Howarth <
> howarth.mailing.lists at gmail.com > wrote:
> 
> 
> 
> Andrey,
> Reading through the the thread at
> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140519/106158.html
> , I can understand the sensitivities here on the topic of reviews.
> IMHO, the process of integrating clang-omp and openmp into the
> standard llvm/compiler-rt/clang build would go much smoother if the
> merge of clang-omp changes were sent up stream as a cohesive set of
> patches to merge the branch like FSF gcc does. I know this will set
> the hair on edge for some of the llvm developers, but when a merge
> is submitted as a single set of patches, the upstream developers are
> forced to take the review process far more seriously. Especially, if
> the reviews are coming in slowly, submitting these patches upstream
> in a piecemeal approach will only aggravate the problem of timely
> reviews.
> Jack
> 
> 
> 
> 
> 
> On Mon, Jun 2, 2014 at 11:17 AM, Andrey Bokhanko <
> andreybokhanko at gmail.com > wrote:
> 
> 
> 
> 
> 
> 
> Alp,
> 
> With all respect, a few of assertions you made are simply *not true*.
> 
> 
> 
> 
> 
> 
> On Mon, Jun 2, 2014 at 6:02 PM, Alp Toker < alp at nuanti.com > wrote:
> 
> 
> It should be made clear that the current OpenMP runtime CMake build
> system has been in development for some time, including on-list
> discussions in the LLVM community that go back weeks following all
> the best practices we have. The only thing that changed is that C.
> Bergstrom graciously provided the sign-off we needed to integrate
> Jack's work late last week.
> 
> 
> 
> What "discussions... that go back weeks" you are speaking about?!
> 
> Jack started his "On Improving the Build System revisited" thread on
> May 30. This is four days ago, not weeks.
> 
> And since when "all the best practices" include introducing a new
> build system without getting project architect's consent? --
> especially after explicitly asked to do so, a message that you
> conveniently ignored.
> 
> 
> 
> 
> So it's a mischaracterisation to say this happened over the weekend.
> Even if it did that would be on the long side compared to timescales
> seen on llvm-commits.
> 
> 
> What timescales you are speaking about?!
> 
> 
> For reference, we wait for *weeks* for our OpenMP in clang patches to
> be reviewed! And we commit them *only* after explicit consent of one
> of clang code owners -- even if we already got code review from
> someone else.
> 
> 
> 
> In general it's a good idea to participate in on-list discussions and
> give a heads up if you see people discussing features you have plans
> for. Is there anything else in the pipeline?
> 
> 
> That's *exactly* what we did back in March.
> 
> http://lists.cs.uiuc.edu/pipermail/openmp-dev/2014-March/000055.html
> 
> 
> Yours,
> Andrey
> 
> 
> 
> _______________________________________________
> Openmp-dev mailing list
> Openmp-dev at dcs-maillist2.engr.illinois.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/openmp-dev
> 
> 
> 
> 
> 
> _______________________________________________
> Openmp-dev mailing list
> Openmp-dev at dcs-maillist2.engr.illinois.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/openmp-dev
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory



More information about the Openmp-dev mailing list